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

MessageDecoder deserialize only POST request #3

Closed
wants to merge 1 commit into from
Closed

MessageDecoder deserialize only POST request #3

wants to merge 1 commit into from

Conversation

crisim
Copy link
Contributor

@crisim crisim commented Aug 7, 2014

MessageDecoder should deserialize only POST request and fixed error in
CompositeSerializer

MessageDecoder should deserialize only POST request  and fixed error in
CompositeSerializer
@jgauffin
Copy link
Owner

jgauffin commented Aug 7, 2014

Bodies are allowed in all requests but HEAD according to the HTTP specification (RFC2616)

@jgauffin jgauffin closed this Aug 7, 2014
@crisim
Copy link
Contributor Author

crisim commented Aug 7, 2014

the RFC2616 states that responses to HEAD requests MUST NOT return an entity body. You've got a point not accepting the pull for another reason, we don't need to enforce the deserialization of the body in the CompositeSerializer if the content type of the request doesn't match "multipart/form-data" or "application/x-www-form-urlencoded", maybe a property or a constructor argument?

@jgauffin
Copy link
Owner

jgauffin commented Aug 7, 2014

That's what I said. "all but HEAD". So you can't just parse bodies for POST as you did in your pull request..

Regarding the decoder, the problem is rather

if (formAndFiles != null)
                    {
                        request.Form = formAndFiles.Form;
                        request.Files = formAndFiles.Files;
                    }
                    else
                        throw new HttpException(500, "Unknown decoder result: " + result);

in HttpMessageDecoder.cs.

that should instead be else if (result != null). Because as you said, null should be OK. The check is there to make sure that no deserializer thinks that it works but do not (As it returned something that the HttpMessageDecoder can't handle).

@jgauffin
Copy link
Owner

jgauffin commented Aug 7, 2014

So if you fix those two things I would gladly accept a new pull request.

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.

None yet

2 participants