Page MenuHomePhabricator

"thumb.php" should not throw fatal MWException when missing query parameters
Closed, ResolvedPublic




[c012c80f] /w/thumb.php?f=Screenshot.png MWException from line 60 of .../mediawiki/includes/media/ImageHandler.php: No width specified to ImageHandler::makeParamString


#0 includes/filerepo/file/File.php(929): ImageHandler->makeParamString(array)
#1 includes/filerepo/file/File.php(912): File->generateThumbName(string, array)
#2 thumb.php(253): File->thumbName(array)
#3 thumb.php(35): wfStreamThumb(array)
#4 {main}

Event Timeline

Krinkle raised the priority of this task from to Needs Triage.
Krinkle updated the task description. (Show Details)
Krinkle added a subscriber: Krinkle.
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Presumably this should throw an HTTP 400 error with some sensible text instead?

The exception is not thrown in the strict sense, it is caught and rendered via wfThumbError( 500, $e->getHTML() ). Should we display a human-readable error message instead of the backtrace? Seems helpful for debugging.

ImageHandler::makeParamString() throws MWException when the width is missing; child classes return false instead, and that is handled nicely (compare ). So there are two ways to fix this:

  1. have ImageHandler::makeParamString() return false - violates the function signature (as documented at MediaHandler::makeParamString()) but child classes do that already so this at least makes it consistent
  2. create a new exception subclass
In T88508#1016745, @Tgr wrote:

have ImageHandler::makeParamString() return false

That does not seem to be handled correctly by the methods calling makeParamString, so better not mess with it.

gerritbot added a subscriber: gerritbot.

Change 188730 had a related patch set uploaded (by Gergő Tisza):
Handle missing width nicely in thumb.php


Change 188730 merged by jenkins-bot:
Handle missing width nicely in thumb.php