Page MenuHomePhabricator

Add getParsedBody to RequestInterface in the REST framework
Closed, ResolvedPublic

Description

RequestInterface should have a getParsedBody method (and a matching setParsedBody method), implemented in RequestBase. The REST framework should ensure that the parsed body is populated if the request body is JSON or form data.

Motivation: This will allow us to implement a solution for T305973 in a stright-forward way. It also improves parity with PSR-7.

Implementation notes:

  • The Router class should call setParsedBody() close to where it already calls setPathParams() on the request object.
  • The actualy parsing of the body data should be implemented on in parseBodyData() method on the Handler base class, so concrete handlers can add supported formats, or opt out of body parsing by returnign null. Handlers that have body parsing disabled by process in request body as a stream by calling RequestInterface::getBox().
  • PSR-7 requires getParsedBody to also work for form data. So requests that have application/x-www-form-urlencoded or multipart/form-data bodies, getParsedBody() must return the same thing as getPostParams(). To support this, it will be useful to add a getBodyType() method to RequestInterface(), which provides convenient access to the body's content model. The implementation of that method should follow the code currently located in Validator::validateBody().
  • RequestData should support setting the parsed body in the constructor, using the key 'parsedBody'.

Event Timeline

Change 1003028 had a related patch set uploaded (by Atieno; author: Atieno):

[mediawiki/core@master] RequestInterface: Add getParsedBody and setParsedBody

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

Change 1003411 had a related patch set uploaded (by WQuarshie; author: WQuarshie):

[mediawiki/core@master] Parsed Body

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

Some TODOs, from the top of my head

  • RequestBase::getParsedBody:
    • PSR-7 says that we should return the post params if the body type is one of the form data types. Decide if we want to move that logic out of Handler::parseBodyData. Consider how handlers can control whether or not they support form data in addition to JSON.
  • Router::execute should call a new method Router::setBodyData, which should:
    • should throw for GET, HEAD, and DELETE if there is body data. Compare the code in Use the code from Validator::validateBody.
    • should throw for POST and PUT if there is body data but no content type. Be careful not to try and loat the entire body in order to check, since it may be very large. Use the code from Validator::validateBody.
    • should not throw for POST and PUT if there is no content type but also no body
    • call Handler::parseBodyData (if appropriate)
    • call RequestInterface::setParsedBody if Handler::parseBodyData returned an array.
    • throw if the handler does not support the given content type. Note that handlers may return null from parseBodyData() even for types they support, if they want to process the request as a stream.
  • File a ticket to remove that code duplication by deprecating/replacing Validator::validateBody

Change 1003028 merged by jenkins-bot:

[mediawiki/core@master] RequestData: Introduce phpunit tests

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

Change 1003411 merged by jenkins-bot:

[mediawiki/core@master] RequestInterface: add getParsedBody and setParsedBody.

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

daniel moved this task from In Progress to Done on the MW-Interfaces-Team board.

Change 1008419 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] Rest: add a test for disabling automatic body parsing

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

Change 1008821 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] REST: Improve documentation of methods related to body parsing

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

Change 1008758 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@wmf/1.42.0-wmf.21] Rest: allow Handlers to disable body parsing.

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

Change 1008419 merged by jenkins-bot:

[mediawiki/core@master] Rest: allow Handlers to disable body parsing.

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

Change 1008758 abandoned by Daniel Kinzler:

[mediawiki/core@wmf/1.42.0-wmf.21] Rest: allow Handlers to disable body parsing.

Reason:

This can probably wait for next week's train, no need for a backport

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

Change #1008821 merged by jenkins-bot:

[mediawiki/core@master] REST: Improve documentation of methods related to body parsing

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