Skip to content

Form Urlencoded Request Body Parser Middleware #222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

WyriHaximus
Copy link
Member

@WyriHaximus WyriHaximus commented Sep 14, 2017

Split off from #220
Requires #216

@andig
Copy link
Contributor

andig commented Sep 17, 2017

Does this middleware- same as the multipart one- not require that the entire body is received first? If yes- how is the dependency on the Buffer expressed (or is it up to the developer) and doesn't this- at least in the case of multipart- to some extend defy the purpose of async?

It's probably confusion on my side but this implementation feels radically different from what has been committed last year before rolling back with the difference being much more than just middlware stacked in?

@WyriHaximus
Copy link
Member Author

WyriHaximus commented Sep 17, 2017

@andig it does, that dependency will be expressed through the documentation in the readme. Haven't added that to this PR since we haven't came to the conclusion if this and #223 of a middleware per content type or #220 is the way to go.

I understand your confusion, when discussing this with @clue at DPC a few months ago. But #216, #218, and #220 or #222/#223 together make working form and file upload handling. Because I personally would prefer a streaming parser and the new middleware make it easy putting another parser in there, I'm writing a streaming body parser middleware once @reactphp/core settles on #220 vs #222/#223 . (So there is an option for both, plus I have all the code making it a matter of putting it together.)

@WyriHaximus
Copy link
Member Author

Closing in favour of #220 as explained here: #220 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants