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

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 3:42 AM
bzimport set Reference to bz69453.
bzimport added a subscriber: Unknown Object (MLST).

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 Medium.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

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

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

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

TheDJ claimed this task.
TheDJ moved this task from Doing to Done on the TimedMediaHandler board.
TheDJ removed a project: Patch-For-Review.
TheDJ subscribed.

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