Page MenuHomePhabricator

TimedMediaHandler making bad action=raw requests
Closed, ResolvedPublic

Description

In the CentralAuth bug 39996 debug logs we're seeing requests coming through with $_SERVER['REQUEST_URI'] set to /wiki/:South+Africa+National+Anthem.ogg.af.srt?action=raw&ctype=text/x-srt

Problems here:

  1. Namespace is missing
  2. Using + instead of _
  3. Passing ctype=text/x-srt doesn't work. RawAction has a whitelist and converts it to text/x-wiki AFAIS
  4. Visiting that URL in my browser throws a "Invalid file extension found in the path info or query string." Even with proper namespace (https://commons.wikimedia.org/wiki/TimedText:South_Africa_National_Anthem.ogg.af.srt?action=raw&ctype=text/x-srt), I still see the same error.

Version: unspecified
Severity: major
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=39996

Details

Reference
bz69453

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 3:42 AM
bzimport added a project: TimedMediaHandler.
bzimport set Reference to bz69453.
bzimport added a subscriber: Unknown Object (MLST).
Legoktm created this task.Aug 13 2014, 1:32 AM

Change 154144 had a related patch set uploaded by Brian Wolff:
Fix horribly broken way TMH was generating <track> urls

https://gerrit.wikimedia.org/r/154144

(In reply to Gerrit Notification Bot from comment #1)

Change 154144 had a related patch set uploaded by Brian Wolff:
Fix horribly broken way TMH was generating <track> urls
https://gerrit.wikimedia.org/r/154144

That only covers the bad encoding, not the wrong namespace.


Wrong namespace appears to be because TMH tries to override the api with a different DB object (which is super insane), but the api still works with the local instances namespace list, so TimedText is getting translated to whatever namespace 102 is on the local wiki. On mw.org it uses the extension namespace (!)

Change 154144 merged by jenkins-bot:
Fix horribly broken way TMH was generating <track> urls

https://gerrit.wikimedia.org/r/154144

Change 154150 had a related patch set uploaded by Legoktm:
Fix horribly broken way TMH was generating <track> urls

https://gerrit.wikimedia.org/r/154150

Change 154151 had a related patch set uploaded by Legoktm:
Fix horribly broken way TMH was generating <track> urls

https://gerrit.wikimedia.org/r/154151

Change 154151 merged by jenkins-bot:
Fix horribly broken way TMH was generating <track> urls

https://gerrit.wikimedia.org/r/154151

Change 154150 merged by jenkins-bot:
Fix horribly broken way TMH was generating <track> urls

https://gerrit.wikimedia.org/r/154150

First part was backported, so the action=raw requests with bad namespaces now return 404s instead of 403s, which means MediaWiki should at least clean up any unfinished transactions.

I looked at the logs again today, and there's at least one new broken account since bawolff's patch was deployed.

The log entries we have for it are:

centralauth-bug39996.log-20140827.gz:[ts] mw1164 commonswiki: CentralAuthHooks::attemptAddUser: creating new user (USERNAME) - from: /wiki/:Citing+sources+tutorial,+part+1.ogv.en.srt?action=raw&ctype=text/x-srt
centralauth-bug39996.log-20140827.gz:[ts] mw1208 commonswiki: CentralAuthHooks::attemptAddUser: creating new user (USERNAME) - from: /w/api.php?callback=jQuery1111[...]&action=parse&page=TimedText%3ACiting_sources_tutorial%2C_part_1.ogv.en.srt&smaxage=3600&maxage=3600&format=json&_=[some number]
centralauth-bug39996.log-20140827.gz:[ts] mw1149 commonswiki: CentralAuthHooks::attemptAddUser: creating new user (USERNAME) - from: /wiki/:Citing+sources+tutorial,+part+1.ogv.zh-hant.srt?action=raw&ctype=text/x-srt
centralauth-bug39996.log-20140827.gz:[ts] mw1097 commonswiki: CentralAuthHooks::attemptAddUser: creating new user (USERNAME) - from: /wiki/:Citing+sources+tutorial,+part+1.ogv.or.srt?action=raw&ctype=text/x-srt

I removed the timestamps, but they were all the exact same second.

Is this still a problem, and still high priority?

Legoktm lowered the priority of this task from High to Normal.Mar 9 2015, 7:10 PM

The requests are now less broken, and no longer causing CentralAuth issues AFAIS, so lowering priority.

mw1075 commonswiki: CentralAuthHooks::attemptAddUser: creating new user (USERNAME) - from: /w/index.php?title=:Candidate%27s+song+take+2.ogg.en.srt&action=raw&ctype=text%2Fx-srt
Jdforrester-WMF moved this task from Untriaged to Backlog on the Multimedia board.Sep 4 2015, 6:35 PM
Restricted Application added a subscriber: Matanya. · View Herald TranscriptSep 4 2015, 6:35 PM
brion moved this task from Backlog to TimedText on the TimedMediaHandler board.Oct 23 2015, 9:25 PM

Change 289976 had a related patch set uploaded (by TheDJ):
[WIP] Rewrite discovery of TimedText tracks

https://gerrit.wikimedia.org/r/289976

TheDJ moved this task from TimedText to Doing on the TimedMediaHandler board.May 23 2016, 7:41 PM

Change 289976 merged by jenkins-bot:
Rewrite discovery of TimedText tracks

https://gerrit.wikimedia.org/r/289976

Change 314856 had a related patch set uploaded (by Paladox):
Rewrite discovery of TimedText tracks

https://gerrit.wikimedia.org/r/314856

Change 314862 had a related patch set uploaded (by Brion VIBBER):
WIP: Rewrite discovery of TimedText tracks

https://gerrit.wikimedia.org/r/314862

Change 314862 merged by jenkins-bot:
Rewrite discovery of TimedText tracks

https://gerrit.wikimedia.org/r/314862

Change 314856 abandoned by Reedy:
Rewrite discovery of TimedText tracks

Reason:
https://gerrit.wikimedia.org/r/#/c/314862/

https://gerrit.wikimedia.org/r/314856

TheDJ closed this task as Resolved.Oct 14 2016, 7:21 PM
TheDJ claimed this task.
TheDJ moved this task from Doing to Done on the TimedMediaHandler board.
TheDJ removed a project: Patch-For-Review.
TheDJ added a subscriber: TheDJ.

Well at least the urls have the right namespace now, but we still have a CORS issue, but see T122737 for that.