Skip to content

Request body incorrectly parsed #386

Closed
#395
Closed
Request body incorrectly parsed#386
#395
Labels
new feature
Milestone
v1.2.0

Description

@bpolaszek
bpolaszek
opened on Aug 24, 2020

Hello there,

I have implemented a Mercure Hub with ReactPHP and I'm having troubles with request bodies.

I'm currently trying to achieve this:

POST /.well-known/mercure
Content-Type: application/x-www-form-urlencoded

topic=/foo&topic=/bar&data=foobar

Indeed, the Mercure protocol specification states that a topic /foo can have alternate IRIs such as /bar. The payload to be sent is not in the PHP format topic[]=foo&topic[]=bar but topic=/foo&topic=/bar which means that PHP, by default, considers that topic is a string with the only value /bar, which is not what I want.
And so is the React\Http\Middleware\RequestBodyParserMiddleware which internally relies on PHP's parse_str function.

So, I'd like to implement my own logic to parse the body payload by myself, but:

  • It looks like ReactPHP removes any trace of the original body at some point ($request->getBody() returns an empty string), when Content-Type is application/x-www-form-urlencoded (could not reproduce with a random content type)
  • It looks like the only way to short-circuit the RequestBodyParserMiddleware to introduce my own is to set an empty value for enable_post_data_reading in php.ini, but I cannot assume people who will use my project have this tricky tweak in their configuration.

Could we either:

  • Not remove the original body of the request
  • Introduce an interface for RequestBodyParserMiddleware so that we could optionally replace it in Server's constructor
  • Introduce an array $config of options into Server's constructor so that we could pass an option to prevent parsing request bodies and let our own middlewares take care of this?

Thank you,
Ben

Activity

  1. clue commented on Sep 16, 2020

    @clue
    clue
    on Sep 16, 2020
    Member

    @bpolaszek Thanks for bringing this up!

    I agree that clearing the request body for some requests is somewhat inconsistent, so I've just filed #395 to make sure you can always read the raw request body and apply your own custom parsing rules.

    I consider this a new feature rather than a bug fix. IMO the request body is not "incorrectly parsed" because it follows PHP's default handling and I don't see anything that contradicts this.

    With the above change in place, you can use your own parser to override this behavior. If you feel this is something that more people could benefit from, perhaps share this as a middleware implementation or consider filing a PR in this repo and I'm happy to take another look!

    BTW, love your Mercure protocol implementation, keep it up! 💪

    I believe this has been answered, so I'm closing this for now. Please come back with more details if this problem persists and we can always reopen this 👍

  2. clue
    closed this as completedon Sep 16, 2020
  3. clue
    added
    new feature
    on Sep 16, 2020
  4. clue
    added this to the v1.2.0 milestone on Sep 16, 2020
  5. clue
    reopened this on Sep 16, 2020

  6. bpolaszek commented on Sep 16, 2020

    @bpolaszek
    bpolaszek
    on Sep 16, 2020
    Author

    Hello @clue,

    Thanks a lot for having worked on this!

    Regarding the Mercure hub, it highly depends on your work on clue/redis-react, clue/block-react and clue/reactphp-eventsource (among almost everything else in the reactphp ecosystem), so let's seize the opportunity for thanks and congrats! 🙏 🙏 🙏

    I'll keep an eye on the PR merge to test this ASAP.

    Thanks again,
    Ben

  7. WyriHaximus
    closed this as completedin #395on Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    new feature

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions