Page MenuHomePhabricator

WebRequest->checkUrlExtension() should not cause logged exception on action=raw
Closed, ResolvedPublicPRODUCTION ERROR

Description

https://en.wikipedia.org/wiki/MediaWiki:Common.js?action=raw&ctype=text/javascript for example triggers this:

Forbidden

Invalid file extension found in the path info or query string.
2015-02-12 19:46:07 mw1068 enwiki: [cf13a9f1] /wiki/MediaWiki:Common.js?action=raw&ctype=text/javascript   HttpError from line 942 of /srv/mediawiki/php-1.25wmf16/includes/WebRequest.php: Invalid file extension found in the path info or query string.
#0 /srv/mediawiki/php-1.25wmf16/includes/actions/RawAction.php(61): WebRequest->checkUrlExtension()
#1 /srv/mediawiki/php-1.25wmf16/includes/actions/FormlessAction.php(43): RawAction->onView()
#2 /srv/mediawiki/php-1.25wmf16/includes/MediaWiki.php(401): FormlessAction->show()
#3 /srv/mediawiki/php-1.25wmf16/includes/MediaWiki.php(277): MediaWiki->performAction()
#4 /srv/mediawiki/php-1.25wmf16/includes/MediaWiki.php(556): MediaWiki->performRequest()
#5 /srv/mediawiki/php-1.25wmf16/includes/MediaWiki.php(420): MediaWiki->main()
#6 /srv/mediawiki/php-1.25wmf16/index.php(46): MediaWiki->run()
#7 /srv/mediawiki/w/index.php(3): include()
#8 {main}

Event Timeline

Krenair raised the priority of this task from to High.
Krenair updated the task description. (Show Details)
Krenair added a project: MediaWiki-General.
Krenair added subscribers: Krenair, Krinkle, csteipp.

Use the canonical form with index.php, this is a safety issue for old IE security bugs with type sniffing.

brion lowered the priority of this task from High to Low.Feb 12 2015, 9:22 PM

I recommend fixing RawPage to catch the exception and link to the preferred URL; this has been reported several timed over the years and seems to confuse people with the unclear failure mode.

Both locally on master, mediawiki.org and on en.wikipedia.org, I can't reproduce a user-facing exception for this type of url. And its been that way for at least 5 years I think.

It generates a Forbidden page:

Screen_Shot_2015-02-12_at_21.25.49.png (230×870 px, 22 KB)

It's technically implemented as an HttpError(Exception) with a report method to render the Forbidden page. And MWExceptionHandler logs it as it would any other exception. Perhaps we should disable logging of HttpError exceptions. Similarly as PermissionsError exceptions (though I'm not certain we aren't logging those either). Or should we log them in some cases?

I doubt this has been all over the exception logs for years... These should probably be caught somewhere. I'm not seeing PermissionsErrors exceptions come up in the log if I try to browse to Special:HideRevision on enwiki.

I recommend fixing RawPage to catch the exception and link to the preferred URL; this has been reported several timed over the years and seems to confuse people with the unclear failure mode.

If we have a preferred URL, why not have IEUrlExtension::fixUrlForIE6() return it to use the existing code path that redirects the request instead of showing any error?

If we have a preferred URL, why not have IEUrlExtension::fixUrlForIE6() return it to use the existing code path that redirects the request instead of showing any error?

Hmmmmmmm, that should work actually I think. I'll whip up an old IE VM and double-check the behavior.

It's technically implemented as an HttpError(Exception) with a report method to render the Forbidden page. And MWExceptionHandler logs it as it would any other exception. Perhaps we should disable logging of HttpError exceptions. Similarly as PermissionsError exceptions (though I'm not certain we aren't logging those either). Or should we log them in some cases?

PermissionsError, being an ErrorPageError exception, would be caught by MediaWiki::run() and handled specially there. HttpError isn't an ErrorPageError, presumably because that expects i18n messages and so on while HttpError seems to be intended to be lower-level.

Ok, looks like IEUrlExtension::fixUrlForIE6() doesn't know how to decompose the title or action or whatever from the URL... not sure how tricky it would be to retrofit on that end without introducing MW dependencies into the library code.

Krenair renamed this task from action=raw&ctype=text/javascript broken on CSS/JS pages to action=raw&ctype=text/javascript on CSS/JS pages causes exceptions to be logged.Feb 14 2015, 4:09 AM
Krinkle renamed this task from action=raw&ctype=text/javascript on CSS/JS pages causes exceptions to be logged to WebRequest->checkUrlExtension() should not cause logged exception on action=raw.Nov 2 2016, 7:17 PM

Use the canonical form with index.php, this is a safety issue for old IE security bugs with type sniffing.

Considering we've already killed support for such browsers at WMF, is this still worth even detecting and defending against?

The error is still show at the example url (which is fine, since we don't want to support those urls), and while I'm not sure what changed, it is no longer spamming the logs when opening this url so. Fixed?

mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:12 PM