Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

http_body_parser tries to parse body of GET Requests #131

Open
davidmehren opened this issue Jul 29, 2015 · 15 comments
Open

http_body_parser tries to parse body of GET Requests #131

davidmehren opened this issue Jul 29, 2015 · 15 comments
Assignees

Comments

@davidmehren
Copy link

I just found out that http_body_parser tries to parse the body of every request if the header 'Content-Type' is set to 'application/json'. If the request is GET this obviously fails because GET requests dont have a body.

Stack trace of the error:
Failed to execute .getDays - FormatException: Unexpected end of input (at character 1)

^

dart:convert JsonCodec.decode
package:redstone/src/http_body_parser.dart 101:60 _parseRequestBody.

@Pacane
Copy link
Contributor

Pacane commented Oct 10, 2015

Does anyone know what the standard is for this? I know from that a GET request can actually have a body.

A message-body MUST NOT be included in a request if the specification of the request method (section 5.1.1) does not allow sending an entity-body in requests. A server SHOULD read and forward a message-body on any request; if the request method does not include defined semantics for an entity-body, then the message-body SHOULD be ignored when handling the request.

Although, nowhere in the GET specification do I see anything saying that it's forbidden to send an entity-body in such request. [As opposed to OPTIONS request where it's explicitly written.]

I wonder what others think about this. I feel something's wrong with ignoring the request, but I also feel like it's not so good to throw an error (for instance, Google throws a BAD REQUEST error when this happens.

Any thoughts? @kaendfinger @cgarciae

@TheBosZ
Copy link
Contributor

TheBosZ commented Feb 24, 2016

👍 Just ran into this problem myself.

I have an Angular2 (Ionic2) app talking to my server and when I try to
http.get('server/users') my auth interceptor throws up.

According to the Angular2 docs get doesn't even have an option for a body.

@Pacane
Copy link
Contributor

Pacane commented Feb 24, 2016

Can you show the code you use to initiate the query? It is likely to be invalid to provide a body for a GET.

@TheBosZ
Copy link
Contributor

TheBosZ commented Feb 24, 2016

var headers = AuthInterceptor.generateHeaders(url, null, 'GET', contentType, 'bob', 'lskfjwefweljfk');
var result = this.http.get(url, {
    headers: headers
});

Notice that there's only 2 parameters for get: url and options.

@Pacane
Copy link
Contributor

Pacane commented Feb 24, 2016

so this is JS right?

@TheBosZ
Copy link
Contributor

TheBosZ commented Feb 24, 2016

That particular bit of code is Typescript, but a Javascript version would look exactly the same.

To me, it seems like a simple fix would be to change http_body_impl to check the body before trying to decode it. Any reason why that wouldn't work?

@Pacane
Copy link
Contributor

Pacane commented Feb 24, 2016

No that would be fine I suppose. I just didn't know if we should throw an error or just ignore the body on a GET. There doesn't seem to be a standard for this AFAIK. If you know anything about such standard, let me know.

@Pacane
Copy link
Contributor

Pacane commented Feb 24, 2016

Anyway, if you want to contribute it, I'd be glad to merge it, otherwise I'll add this to my todo list. :)

@Pacane Pacane self-assigned this Feb 24, 2016
@TheBosZ
Copy link
Contributor

TheBosZ commented Feb 24, 2016

Well, this response on that Stack Overflow post has this bit:

So, yes, you can send a body with GET, and no, it is never useful to do so.

And reading the comments and other thoughts, it sounds like the best idea is parse it if it's there, but otherwise ignore it.

Webstorm is being stupid about letting me edit imported libraries, but I'll try my idea and see if I like the results.

@Pacane
Copy link
Contributor

Pacane commented Feb 24, 2016

So "parse it if it's there", and do what with it?

@TheBosZ
Copy link
Contributor

TheBosZ commented Feb 24, 2016

Now that I've had a chance to look at what the code actually does, I don't really see an easy way to do what I was thinking. I was thinking he had more control in how it parses, but now I see that it's handled outside of this library.

Playing around with it, what if we did something like this in request_parser.dart, around line 37, essentially ignoring a body if it's not expected:

if (_body == null && validBodyMethods.contains(httpRequest.method)) {

@Pacane
Copy link
Contributor

Pacane commented Feb 24, 2016

I think this could be something doable yes. I still wonder if we should parse it or not. I think we should just ignore the body when it's a GET.

@TheBosZ
Copy link
Contributor

TheBosZ commented Feb 24, 2016

I agree. From the SO post, that seems to be an acceptable solution.

@Pacane
Copy link
Contributor

Pacane commented Feb 27, 2016

@TheBosZ Fix deployed in the wild with v0.6.5. Thanks again!

@fwgreen
Copy link

fwgreen commented Jul 27, 2016

This issue persists if you use Postman and don't explicitly disable application/json on the request body.

@Pacane Pacane reopened this Jul 30, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants