Content-Type issue in dispatch 0.11.1

[Nathan Hamblen]

Heads up, if you're upgrading from 0.11.0 and specify your own
content-type header you might see problems, described below.

Comment on the issue if you have an opinion about how it should work
differently, or better yet post a pull request.

https://github.com/dispatch/reboot/issues/82

-------- Original Message --------
Subject: [reboot] Multiple Content-Type Headers (#82)
Date: Wed, 28 May 2014 12:02:50 -0700
From: efuquen <notifi...@github.com>



I recently upgraded from dispatch 0.11.0 to 0.11.1 and noticed all my
POST requests had now broken. Using wireshark to investigate the headers
I saw that my requests were now adding an two |Content-Type| headers to
all POST requests. One that I hadn't specified, |Content-Type:
text/plain; charset=UTF-8|, and the one that I had, |Content-Type:
application/json|, where in 0.11.0 only the json |Content-Type| was
added to my POST request.

After looking through the changelog for 0.11.1 I saw this PR that ended
up changing the behavior of POSTs (#72
<https://github.com/dispatch/reboot/pull/72>). Now setting the body with
|<<| will add a text/plain content type if one isn't set, whereas before
it wouldn't. In my code I had set the headers /after/ I had set the
body, and it seems in this case Dispatch will happily add two
Content-Types. I've got a simple project that will duplicate the issue
that occurs in my code and prints out the request here: :
https://github.com/efuquen/dispatch-multiple-content-type/tree/master .
I see two things wrong with this.

1) After doing some digging on the http spec it does not seem like it's
legal for a client to sent multiple |Content-Type| headers. In this SO
response to a query about multiple |Cache-Control| headers
(http://stackoverflow.com/questions/4371328/are-duplicate-http-response-headers-acceptable/4371395#4371395)

the author references the spec, which states in short that a header can
be repeated if the header is already defined to validly have multiple
values, the multiple headers should then be treated as if this had been
a single header with the multiple values listed out. So, for example:

| Cache-Control: no-store
Cache-Control: no-cache
|

is fine because it's equivalent to:

| Cache-Control: no-store, no-cache
|

/But/, |Content-Type| is not specified by the http spec to validly have
multiple values, if you read it's definition here:
http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.17 .
Therefore you should not be allowed to specify multiple |Content-Type|
headers. IMO the correct behavior here is that if a user specifies a
header, in any order when building the request (my bug would have not
manifested if I had specified headers /before/ setting the body with
|<<|) it should override any default that is set by Dispatch, the user
clearly has no knowledge that a default header is being set and in the
end that is what causes the conflict. If the user was to specify two
|Content-Type| headers, which I haven't tested with the current
Dispatch, I would think the second one should overwrite the first or it
should throw an error.

1. This change wasn't fixing any bug, it was an enhancement. Not only
that but it also was changing behavior from the previous version and
breaking backwards compatibility. In my mind these types of changes
should not be going into a point release. I would have expected it
to occur in a 0.11.x to 0.12.0 upgrade, not a release within the
0.11.x line. Not sure what the policy is with changes between
versions but that's my 2 cents.

—
Reply to this email directly or view it on GitHub
<https://github.com/dispatch/reboot/issues/82>.