Page MenuHomePhabricator

PHP Fatal from ApiMain.php: Header may not contain more than a single header, new line detected
Closed, ResolvedPublic

Description

Error

Request ID: W@JJDQpAIDUAAGrtFAEAAACM

message
PHP Fatal Error: Header may not contain more than a single header, new line detected
trace
#0 /srv/mediawiki/php-1.33.0-wmf.2/includes/WebResponse.php(72): NO_FUNCTION_GIVEN()
#1 /srv/mediawiki/php-1.33.0-wmf.2/includes/api/ApiMain.php(588): WebResponse->header(string)
#2 /srv/mediawiki/php-1.33.0-wmf.2/includes/api/ApiMain.php(538): ApiMain->handleException(ApiUsageException)
#3 /srv/mediawiki/php-1.33.0-wmf.2/includes/api/ApiMain.php(502): ApiMain->executeActionWithErrorHandling()
#4 /srv/mediawiki/php-1.33.0-wmf.2/api.php(87): ApiMain->execute()
#5 /srv/mediawiki/w/api.php(3): include(string)
#6 {main}

Impact

Certain API queries are unexpectedly aborted due to this fatal error. Unlike normal exceptions, with this fatal error, the user is not informed about what the problem was, the response is not in a format the client expects (e.g. not JSON or XML), and the HTTP status code becomes HTTP 500 (instead of HTTP 200).

That means users are unable to report it in a useful manner (no request/exception ID).

It also means that the exceptions ends up elevating the error levels for Varnish (instead of just MediaWiki) due to it becoming a fatal with HTTP 500.

Notes

This started 2 days ago on 5 November 2018 around 16:00. It has since been seen 255 times on both wmf.1 and wmf.2 for en.wikipedia.org.

The timing of the regression aligns with the following entry in SAL which might be related:

  • 16:06 anomie@deploy1001: Synchronized wmf-config/InitialiseSettings.php: Setting MCR to read-new on all wikis (T198308) (duration: 00m 55s)

Event Timeline

Krinkle created this task.Nov 7 2018, 2:58 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 7 2018, 2:58 AM
daniel added a subscriber: daniel.Nov 7 2018, 1:43 PM

As far as I can tell, this is an error raised while handling another error (a ApiUsageException). Something somewhere is writing something that contains a newline (a stack trace, perhaps) into the field that is supposed to contain an error code.

This should be addressed from two sides: a) avoid the fatal error during error handling by sanitizing the error codes, and b) do not allow things that are not error codes to be used as error codes. (a) will fix the fatal error, but doesn't help us find the original issue. (b) would cause a fatal error in the offending code, pinpointing the issue.

Anomie moved this task from Unsorted to Non-core-API stuff on the MediaWiki-API board.
Anomie added a subscriber: Anomie.

It looks like every instance of this error logged in Kibana is correlated with an error in the restbase channel, and all have root_req_uri matching /en.wikipedia.org/v1/data/lists/631495/entries/batch?csrf_token=* or /en.wikipedia.org/v1/data/lists/631496/entries/batch?csrf_token=*, which as far as I can tell has something to do with Reading List Service.

If I'm guessing right as to what that restbase URL does, this bit of code[1] seems suspicious:

$this->dieWithError( 'apierror-invalidtitle', wfEscapeWikiText( $op['title'] ) );

Due to some missing brackets,[2] it's passing wfEscapeWikiText( $op['title'] ) as the code for the error rather than as a parameter to the message. If whatever client is trying to edit the lists with IDs 631495 and 631496 is passing a value for $op['title'] that contains a newline, that would cause this error.

[1]: The same error is present here too, BTW.
[2]: That code should be $this->dieWithError( [ 'apierror-invalidtitle', wfEscapeWikiText( $op['title'] ) ] );

Change 472183 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/ReadingLists@master] Fix uses of dieWithError; first param is the whole message, second isn't its parameters

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

Change 472187 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] Ensure that no bad error codes end up in an HTTP header.

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

Change 472199 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/extensions/GlobalBlocking@master] Fix error reporting in SpecialPasswordResetOnSubmit hook function

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

Change 472183 merged by jenkins-bot:
[mediawiki/extensions/ReadingLists@master] Fix uses of dieWithError; first param is the whole message, second isn't its parameters

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

Change 472207 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/extensions/JsonConfig@master] Fix use of Status class

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

Change 472199 merged by jenkins-bot:
[mediawiki/extensions/GlobalBlocking@master] Fix error reporting in SpecialPasswordResetOnSubmit hook function

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

Change 472211 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] API: Validate API error codes

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

Change 472187 abandoned by Daniel Kinzler:
Ensure that no bad error codes end up in an HTTP header.

Reason:
use Ib2d8bd4d4a5d58af76431835ba783c148de7792a instead

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

Change 472207 merged by jenkins-bot:
[mediawiki/extensions/JsonConfig@master] Fix use of Status class

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

Change 472211 merged by jenkins-bot:
[mediawiki/core@master] API: Validate API error codes

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

Anomie closed this task as Resolved.Nov 27 2018, 7:13 PM
Anomie claimed this task.