Invalid Content-Type for get and multipart requests

[beskacz]

I wonder if this is actually valid:
https://github.com/catamphetamine/react-website/blob/master/source/HttpRequest.js#L59

1. GET does not send any content body, so there is no need for Content-Type; what's more, google geocode API (https://maps.googleapis.com/maps/api/geocode/json) will return error response when Content-Type is set.

2. Multipart/* should specify boundary:
   As it's stated in the https://tools.ietf.org/html/rfc2046#section-5.1.1

The Content-Type field for multipart entities requires one parameter, "boundary".

so it should look something like: Content-Type: multipart/form-data; boundary=XXX where XXX is the boundary delimiter.

Currently, thanks to the previous issue: #73
I was able to modify the request before sending with:

if (request.method === 'GET' || request.header['Content-Type'] === 'multipart/form-data') { request.set('Content-Type', null); }

so it does not add Content-Type to the GET request to Google, and superagent automatically adds a proper header with the boundary parameter for form-data.

What do you think?

[catamphetamine]

Hmmm, good thoughts.

https://stackoverflow.com/questions/5661596/do-i-need-a-content-type-for-http-get-requests
Says:

It means that the Content-Type HTTP header should be set for PUT and POST requests.

At the same time, there's no prohibition of Content-Type on GET requests.
Still I guess setting Content-Type manually for GET requests could be removed from the code.
But if I changed that then it would be a breaking change.
We could add a flag for that, something like http.alwaysSetContentType: false.

I also googled this:
ladjs/superagent#1202

Never set Content-Type with multipart, as it erases the boundary argument that has to be set by the browser.

Also this:
ladjs/superagent#1152

In the browser version the boundary is set automatically by FormData element and we have no control over it.

And they also say:

You can either use your own content-type and your own payload with .send() / .set() OR you can let all of it being 100% auto-generated by the browser by using .attach() / .field(). They don't mix.

I guess .send(new FormData(...)) will have to be rewritten as .attach() / .field() (and then tested) and that would fix the FormData part.

So, the fix would mean:

• Adding http.alwaysSetContentType: false flag.
• Rewrite HttpRequest.js from sending FormData directly into using .attach() / .field() and then test that it works.

I can do that when I have some time for that.
In the meanwhile you can use your workaround in onRequest().

[beskacz]

Well, I am not a fan of adding another flag which will be true by default. It will be a breaking change to remove that chunk of code, but you can add a code snippet within the changelog how to set default ContentType if someone needs it:

  http: {
    onRequest: request => {
      if (!request.header['Content-Type']) {
        request.set('application/json');
      }
    },
  },

As to the FormData, the idea to use attach/field looks good, if you create a branch with a fix, I can check if it works.

Take your time, I can wait as I've got that workaround and it's not blocking me. I've created this issue especially that FormData looks to be broken.

Thanks for your time

[catamphetamine]

Fixed in react-pages: content-type not set explicitly and multipart/form-data has a boundary.