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

Don't convert empty body on POST/PUT. #205

Merged
merged 2 commits into from
Jul 18, 2012

Conversation

tim-vandecasteele
Copy link
Contributor

When posting with content type 'application/json' and an empty body, multi_json crashes on the empty string.

When posting with content type 'application/json' and an empty body, multi_json crashes on the empty string.
@dblock
Copy link
Member

dblock commented Jul 18, 2012

This is great, thank you. Could you please update the CHANGELOG as part of the pull, I'll merge it.

Also, I've always been confused whether we should be using .size, .length or something else. Rails implementation of blank? has a pretty clear answer:

def blank?
   respond_to?(:empty?) ? empty? : !self
end

Although we're splitting hair, I think empty? is a more readable way to do a check here.

@tim-vandecasteele
Copy link
Contributor Author

I agree empty? would be nicer to read, sadly StringIO doesn't support empty?.

@dblock dblock merged commit aaf5eaa into ruby-grape:master Jul 18, 2012
@dblock
Copy link
Member

dblock commented Jul 18, 2012

Merged, than you.

@adamgotterer
Copy link
Contributor

With this commit I started getting "Fatal error: undefined method 'size' for #Rack::Lint::InputWrapper:0x86d92e8" under certain post circumstances while calling params. Would changing && request.body.size > 0 to && request.content_length do the same thing?

@dblock
Copy link
Member

dblock commented Jul 21, 2012

@adamgotterer => in theory this would work as well depending on how request.content_length is constructed. I am down with this change if all specs pass.

@adamgotterer
Copy link
Contributor

The current version would fail if it used the real #body_params method and not a mock. The failure case is when the body is plain text and not json. There is no #size method for a string. The change to #content_length wont break tests (I tried it earlier). But I'll spend a little time over the weekend trying to figure out a better way to test this.

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

Successfully merging this pull request may close these issues.

3 participants