Skip to content

Conversation

rmaclean
Copy link

The content-type header did not support an array of header values (for example headers['Content-Type'] = ['application/json', 'text/json', 'application/xml', 'text/xml', 'application/x-www-form-urlencoded'];) it would just produce a single string (i.e. headers['Content-Type'] = 'application/json, text/json, application/xml, text/xml, application/x-www-form-urlencoded;.

Which is incorrect, it should be an array as Accept does. Refactored out the code from accept to a function (produceHeaderKeyValue) and changed Accept & Content-Type headers to use that one function to generate their own header values.

rmaclean added 3 commits May 25, 2016 09:35
…rays of values

The content-type header did not support an array of header values (for example headers['Content-Type'] = ['application/json', 'text/json', 'application/xml', 'text/xml', 'application/x-www-form-urlencoded'];) it would just produce a single string (i.e. headers['Content-Type'] = 'application/json, text/json, application/xml, text/xml, application/x-www-form-urlencoded;

Which is incorrect, it should be an array as Accept does. Refactored out the code from accept to a function (produceHeaderKeyValue) and changed Accept & Content-Type to use that one function.
@wcandillon
Copy link
Owner

wcandillon commented May 25, 2016

@rmaclean It makes sense. Can you fix the build and maybe add a test for it? I could setup an endpoint that echos the accept headers from the request?

@hmadisonturner
Copy link

This isn't quite right. The consumes property contains a list of acceptable MIME types, while the Content-Type header should contain the actual type of the entity body. Thus both the implementation in master and this one produce invalid Content-Type headers when consumes is a list.

This can lead to error responses. For example, IIS 7.5 strips the invalid header and ASP.NET Web API 2 assumes a default of application/octet-stream and responds 415 Unsupported Media Type.

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