Skip to content
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

text bodyParser sets object when body is empty #128

Closed
sosiouxme opened this issue Sep 18, 2015 · 3 comments
Closed

text bodyParser sets object when body is empty #128

sosiouxme opened this issue Sep 18, 2015 · 3 comments
Assignees
Labels

Comments

@sosiouxme
Copy link

Looking at:

req.body = req.body || {}

It seems that when the body is actually null/empty (as with a GET), bodyParser.text will set {} as the body. This can be confusing to later handlers. It seems to me that it should either be left null or set to the empty string.

Background on my use case: I'm implementing a proxy that transforms the request based on the user's authorization. There may or may not be a request body, but if there is, I may need to transform it and then restream it. So my request handling looks like:

app.use(bodyParser.text({'type': '*/*'}), authorizationHandler, reqJsonRestreamer)

If it's a GET request and there is no actual body, bodyParser still sets one, authorizationHandler doesn't know whether there was a real body or not, and when reqJsonRestreamer runs against {} it tries to stream a request body where there was none originally, and the request hangs.

The workaround is to have the next handler check if the body is an object or string treat the object as an empty body. But it shouldn't be necessary if bodyParser just set body accurately in this case.

@dougwilson dougwilson self-assigned this Sep 18, 2015
@dougwilson
Copy link
Contributor

Hi! Yes, the behavior you describe is by design, for better or for worse. The behavior has been changed, to be inline with what you expect, in the 2.0 branch (https://github.com/expressjs/body-parser/tree/2.x). You will have to wait for the 2.0 release (subscribe to PR #66 for updates), just add the following to your package.json: "body-parser": "expressjs/body-parser#2.x", or work-around this (you can use the type-is module's hasBody method to detect if there was really a body or not).

@sosiouxme
Copy link
Author

Got it, thanks!

@dougwilson
Copy link
Contributor

And if it helps, I 100% agree with you the way 1.x works in this regards sucks :) We had to do req.body = {} at the time long ago because other modules would look to see if req.body was falsy and then try to parse (which would hang since we read the body already) in addition to a lot of people just having that like req.body.some_property without checking if req.bdoy existed or not :(

@theganyo theganyo mentioned this issue Nov 6, 2015
9 tasks
@expressjs expressjs locked and limited conversation to collaborators Nov 6, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants