Page MenuHomePhabricator

thumb.php should not return HTTP 200 when streaming the file fails
Closed, ResolvedPublic0 Estimate Story Points


thumb.php ignores the return status of MediaTransformOutput::streamFile() and/or FileRepo::streamFile() and returns a normal HTTP status even if the file could not be streamed, which results in Varnish caching the error. It should return 503 instead.


Related Gerrit Patches:

Event Timeline

Tgr created this task.Mar 12 2015, 7:08 PM
Tgr claimed this task.
Tgr raised the priority of this task from to Needs Triage.
Tgr updated the task description. (Show Details)
Tgr added a project: MediaWiki-File-management.
Tgr added a subscriber: Tgr.
Restricted Application added a project: Multimedia. · View Herald TranscriptMar 12 2015, 7:08 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Tgr set Security to None.
Tgr edited a custom field.

Change 196469 had a related patch set uploaded (by Gergő Tisza):
Return HTTP 500 not 200 from thumb.php when streaming fails

Tgr added a comment.EditedMar 13 2015, 12:53 AM

Per Timo's comment on the patchset, the streamFile calls in img_auth.php, SpecialUndelete#showFile, SpecialRevisiondelete#tryShowFile and SpecialUploadStash#outputLocalFile should be handled similarly. (Also, if we are going to touch all streamFile calls, it should probably be changed to return the Status object so the error messages can be more informative.) Less of a priority though, those can't get cached.

update: split to T92903

Change 196469 merged by jenkins-bot:
Return HTTP 500 not 200 from thumb.php when streaming fails

Gilles added a subscriber: Gilles.Mar 18 2015, 9:10 AM

Any idea how we could test this?

Tgr added a comment.Mar 18 2015, 10:02 PM

Just break MediaTransformOutput::streamFile() and/or FileRepo::streamFile() and make them return false. The first should affect newly created thumbnails (on a wiki using a 404 handler)- The second, I am not sure; I think it only happens on Swift (or similard shared-storage backends), but you can simulate it by calling thumb.php directly for an existing non-thumbnail file (/w/thumb.php?f=<title> should do it).

Gilles closed this task as Resolved.Mar 25 2015, 3:12 PM

I meant testing in production, but that seems a bit far-fetched seeing that locally modifying the code is needed.