Page MenuHomePhabricator

ParsoidHandler::getParsedBody() needs error handling for json_decode
Closed, ResolvedPublic

Description

ParsoidHandler.php contains the following code:

protected function getParsedBody(): array {
        $request = $this->getRequest();
        list( $contentType ) = explode( ';', $request->getHeader( 'Content-Type' )[0] ?? '', 2 );
        switch ( $contentType ) {
                case 'application/x-www-form-urlencoded':
                case 'multipart/form-data':
                        return $request->getPostParams();
                case 'application/json':
                        return json_decode( $request->getBody()->getContents(), true );
                default:
                        throw new HttpException( 'Unsupported Media Type', 415 );
        }
}

About 20 minutes ago, someone did a pybal restart, causing some connections to fail or be dropped. This then caused an error spike in this code, exposing a lack of error handling:

[f67d4c4c-299b-4a55-8636-4613c9b86fa6] /w/rest.php/zh.wikipedia.org/v3/transform/pagebundle/to/pagebundle/Running_Man   TypeError from line 181 of /srv/mediawiki/php-1.36.0-wmf.1/vendor/wikimedia/parsoid/extension/src/Rest/Handler/ParsoidHandler.php: Return value of MWParsoid\Rest\Handler\ParsoidHandler::getParsedBody() must be of the type array, null returned

If $request->getBody()->getContents() returns nothing, or even just returns a string that is invalid JSON, json_decode() will return null. The code doesn't check for this, and uncritically returns the result of json_decode(). This then causes a TypeError, because the method is typed to return an array, but null is not an array. Another way this can fail is if the REST endpoint returns something that is valid JSON, but decodes to something that is not an array (although this is quite rare).

Event Timeline

I assume the TypeError results in a 500 error. What's the preferred HTTP error code for "bad input"?

Change 617164 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/services/parsoid@master] Error handling when json_decoding in ParsoidHandler::getParsedBody()

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

Change 617164 merged by jenkins-bot:
[mediawiki/services/parsoid@master] Error handling when json_decoding in ParsoidHandler::getParsedBody()

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

Arlolra claimed this task.

Change 618166 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/vendor@master] Bump wikimedia/parsoid to v0.13.0-a4

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

Change 618038 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/vendor@wmf/1.36.0-wmf.3] Bump wikimedia/parsoid to v0.13.0-a4

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

Change 618166 merged by jenkins-bot:
[mediawiki/vendor@master] Bump wikimedia/parsoid to v0.13.0-a4

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

Change 618038 merged by jenkins-bot:
[mediawiki/vendor@wmf/1.36.0-wmf.3] Bump wikimedia/parsoid to v0.13.0-a4

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