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

Mysterious header "content-type" => "application/json" being added #52

Open
jackalcooper opened this issue Mar 5, 2017 · 9 comments
Open

Comments

@jackalcooper
Copy link
Contributor

jackalcooper commented Mar 5, 2017

{:ok, %Maxwell.Conn{method: :post, opts: [], path: "/comet", query_string: %{},
 req_body: nil,
 req_headers: %{"Content-Type" => "application/json",
 "authorization" => "Bearer J9O6qCHqNU0jADFWoHtmTroLsBYQEmCG3FWd-B09YAqWjQQV", 
"content-type" => "application/json"}, resp_body: "Bad request",
 resp_headers: %{"connection" => "keep-alive", "content-type" => "text/plain; charset=utf-8", "date" => "Sun, 05 Mar 2017 13:55:05 GMT", 
"server" => "nginx/1.10.1", "transfer-encoding" => "chunked"}, 
state: :sent, 
status: 400, url: "http://test.com:8030"}}

There should not be a "content-type" => "application/json because that's my Maxwell.Middleware.Headers's params

middleware Maxwell.Middleware.Headers, %{
      "Content-Type" => "application/json"
  }
@jackalcooper
Copy link
Contributor Author

jackalcooper commented Mar 5, 2017

It seems to have something to do with the 2.2 update. I wrote the code a while ago before I update the dependency and I worked well.

@secretworry
Copy link
Collaborator

@jackalcooper The maxwell are keeping headers in their lower case format internally to dismiss the need to handle headers with different format.
So the simple answer is, using "content-type" instead of "Content-Type" in your code.

@zhongwencool I think we should downcase the header names user passed in to Maxwell.Middleware.Header, or use put_req_header to set the headers in it.

@jackalcooper
Copy link
Contributor Author

There are cases in which the back end is not that tolerant about it. For instance Rails allow the existence of both upper and down cases while Phoenix doesn't. I got this problem with Elm's HTTP library before as well.

@jackalcooper
Copy link
Contributor Author

This is so tricky. I cleared up a module which used Httpoison and the extra header disappeared

@secretworry
Copy link
Collaborator

@jackalcooper Definitely, the HTTP protocol requires HTTP servers to handle header names in a case-insensitive manner. Rails keep headers in an environment variable format, while phoenix use plug internally, and plug keeps header in a lower case format. So both of them support both upper and down case. The different is in Rails you can query a header in any format because it makes the converting at the query time, and in Phoenix, you should use the lower case header name.

@ijcd
Copy link

ijcd commented Jun 14, 2017

You should be case-insensitive but, in general, case-respecting. Downcase when you do comparisons, but it's annoying that an adapter is monkeying with the data while passing it though. I just started using Maxwell and this has already bitten me.

@secretworry
Copy link
Collaborator

@ijcd We did implement a version that tries to keep the case of header names user passed in while keeping a case-insensitive format internally with something likedown_case_header => {original_header, value}.

But knowing all the adapted clients use downcase headers internally, we stop bothering ourselves to keep a user-specified case. Users can use Conn.put_req_header/3 and Conn.get_req_header/2 to interact with headers without worrying the case stuff.

So could you be more specific about the scenario that you are bitten?

@ijcd
Copy link

ijcd commented Jun 14, 2017

In my case I had a few hundred tests that I moved over from HTTPotion. The headers being downcased caused me to have to edit many of them. I worked it out, but it's annoying. If the server won't care about the case, I want the driver to send what I ask it to, or at least have an option to do so. It shouldn't have opinions about this (the protocol doesn't).

@secretworry
Copy link
Collaborator

secretworry commented Jun 14, 2017

@ijcd Actually the HTTPotion use ibrowse internally, so it's impossible for it to send case-sensitive headers. You can find more details here.

The thing the ibrowse doesn't do is to convert the response. And you tried to match the response in a case-sensitive way. So your tests got baffled.

Matching headers with unified case( downcased in our case) would be much easier than matching headers with mixed case. And the changes you made to your test cases definitely improved their robustness:). So we will just leave it the way it is.

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

No branches or pull requests

3 participants