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

can't use multiple HTTP Headers with the same name #130

Closed
cythrawll opened this issue Feb 21, 2013 · 11 comments · Fixed by #1190
Closed

can't use multiple HTTP Headers with the same name #130

cythrawll opened this issue Feb 21, 2013 · 11 comments · Fixed by #1190
Labels
bug Something isn't working planned Solution is being worked on
Milestone

Comments

@cythrawll
Copy link

cythrawll commented Feb 21, 2013

In HTTP it allows multiple headers with the same name. However, when I went to test
webapp that seems to break when IE10 sends multiple cookie headers, I can't use httpie to test it as it only puts in the last header value in the request.

example:

$ http -v example.org Cookie:foo1=bar1 User-Agent:BAM\1.0 Cookie:foo2=bar2

Current output:

GET / HTTP/1.1
Accept: _/_
Accept-Encoding: gzip, deflate, compress
Cookie: foo2=bar2
Host: example.org
User-Agent: BAM1.0

//response omitted

Expected output:

GET / HTTP/1.1
Accept: _/_
Accept-Encoding: gzip, deflate, compress
Cookie: foo1=bar1
Cookie: foo2=bar2
Host: example.org
User-Agent: BAM1.0

Really HTTP headers should be represented as a case-insensitive MultiMap of some sort, not a Dict.

@jkbrzt
Copy link
Member

jkbrzt commented Feb 22, 2013

This first needs to be allowed by Python-Requests: #1155 Add MultiDict Support

@steven-lai
Copy link

steven-lai commented Jan 16, 2018

The suggested workaround of using: HEADER:"VAL_1, VAL_2" doesn't quite work. I'm not sure if this is because of the server I'm using (Finatra/Finagle). I'm logging the headers and the following is the results of using curl vs httpie.

Using curl http://localhost:9999/api/test --header FOO:first --header FOO:second produces:
HEADER MAP=Map(Host -> localhost:9999, User-Agent -> curl/7.43.0, Accept -> */*, FOO -> first, FOO -> second)

Using http http://localhost:9999/api/test FOO:"first, second" produces:
HEADER MAP=Map(Host -> localhost:9999, User-Agent -> HTTPie/0.9.9, Accept-Encoding -> gzip, deflate, Accept -> */*, Connection -> keep-alive, FOO -> first, second)

Finatra/Finagle header implementation treats headers as multi-map so they do offer a getAll for a given key.

With httpie getAll returns one entry and curl returns two entries.

@luckydenis
Copy link
Contributor

I found what the problem is, can I take this task?

@steven-lai
Copy link

I can't answer whether you can take this or not but I'm interested in what the problem is, can you provide some details?

@luckydenis
Copy link
Contributor

luckydenis commented Apr 25, 2019

https://github.com/httpie/httpie/blob/fd44f1af93ce1d2c84f324b8474d2d075b5a7b13/httpie/input.py#L133

Here we give our arguments to a parser built into a Python, and inside it uses dict instead of multidict resulting in key mashing. I just wanted to ask the community - in what form it is better to solve it. 1. There is the idea, or completely rewrite the logic of parsing, 2. Just add a function that will glue the value of the same keys.

@tgr
Copy link

tgr commented Jan 2, 2021

As noted in the upstream bug, RFC 7230 says A sender MUST NOT generate multiple header fields with the same field name in a message unless either the entire field value for that header field is defined as a comma-separated list [i.e., #(values)] or the header field is a well-known exception. The RFC is not very clear about it but the only well-known exception seems to be Set-Cookie.

So httpie could just give some sort of warning on repeated headers, as such requests tend to be incorrect.

@Almad Almad added blocked by upstream The issue is in a dependency. We are waiting for the upstream library to lay the groundwork. and removed blocked by upstream The issue is in a dependency. We are waiting for the upstream library to lay the groundwork. labels Feb 3, 2021
@Almad
Copy link

Almad commented Feb 3, 2021

Requests implemented MultiDict in psf/requests#1155 , but it works for response only.

Also, they don't support multiple header field lines, they'd get combined into a single line as per RFC. We could do that in httpie as well.

The complete fix would need to use urllib3 directly.

@Almad Almad added planned Solution is being worked on and removed blocked by upstream The issue is in a dependency. We are waiting for the upstream library to lay the groundwork. labels Feb 3, 2021
@Ousret
Copy link

Ousret commented Feb 13, 2021

Something so simple yet so unsupported nowadays. Except for httpx. Also I have made something basic to help resolving issues with headers. Check out https://github.com/Ousret/kiss-headers if anyone is looking for a solution.

@isidentical
Copy link
Contributor

isidentical commented Oct 23, 2021

As people already quoted RFC 7230, Section 3.22 says;

A sender MUST NOT generate multiple header fields with the same field name in a message unless either the entire field value for that header field is defined as a comma-separated list [i.e., #(values)] or the header field is a well-known exception (as noted below).

Which I interpret as, if a header is required to be sent multiple times with different values it can be either of these;

  • a header that wouldn't change it's semantics when joined to ',' character
  • a header that is defined as a special case (Set-Cookie, which the same section claims 'may often appear multiple times in a response message')

Normally the safer way to go is just preparing the HTTP request to contain the same header multiple times (just like curl), but it is unfortunately not possible because of the underlying backend (requests) that httpie uses (due to this). The backend simply puts everything into a normal (but case insensitive) dictionary, which in our case make the last header take precedence over all else (even if we properly support MultiDict).

So I believe we can make the assumption that if a user passes us multiple HTTP headers with the same name, we can combine their values into a single list that is joined with , (just like the underlying implementation of urllib's HTTPHeaderDict) and this should not lead any change in semantics.

Here is a draft patch that basically implements the said functionality: isidentical@678180e (for the sending part of the CLI(, not the receiving).

@jkbrzt
Copy link
Member

jkbrzt commented Oct 24, 2021

@isidentical

As people already quoted RFC 7230, Section 3.22 says;

A sender MUST NOT generate multiple header fields with the same field name in a message unless either the entire field value for that header field is defined as a comma-separated list [i.e., #(values)] or the header field is a well-known exception (as noted below).

Which I interpret as, if a header is required to be sent multiple times with different values it can be either of these;

  • a header that wouldn't change it's semantics when joined to ',' character
  • a header that is defined as a special case (Set-Cookie, which the same section claims 'may often appear multiple times in a response message')

This looks like a correct interpretation to me 👍

We (and Requests) already handle repeated response headers correctly, so we don’t need to change anything there:

$ https -h  github.com | grep Set-Cookie | wc -l 
3

Normally the safer way to go is just preparing the HTTP request to contain the same header multiple times (just like curl)

Agree, this would probably be the best/safest solution.

Benefit 1/

It would give the user a choice:

  1. Want a comma-separated list? → pass A:A,B
  2. Want multiple headers lines? → pass A:A A:B

Benefit 2/

Even though these two representations are semantically identically, the user might be interested in testing if/how a server handles the individual scenarios. HTTPie is popular in the pentesting community, for example, who tend to care about these low-level things.

but it is unfortunately not possible because of the underlying backend (requests) that httpie uses (due to this). The backend simply puts everything into a normal (but case insensitive) dictionary, which in our case make the last header take precedence over all else (even if we properly support MultiDict).

What if we patched PreparedRequest.prepare_headers()? 🤔

We already access each prepared request before we send it.

So if, for example, simply providing a custom .prepare_headers() implementation would lead to the repeated header preservation, then we should consider it. If it would require additional patching, however, then it’s probably not worth it.

So I believe we can make the assumption that if a user passes us multiple HTTP headers with the same name, we can combine their values into a single list that is joined with , (just like the underlying implementation of urllib's HTTPHeaderDict) and this should not lead any change in semantics.

This is correct 👍🏻 But let’s just explore the multi-line version a bit more first because it would be nice to provide the user the benefits that it offers.

Here is a draft patch that basically implements the said functionality: isidentical@678180e (for the sending part of the CLI(, not the receiving).

This looks like a solid start for plan B. Feel free to open a draft PR. Quick feedback off the top of my head:

https://github.com/isidentical/httpie/blob/678180e10b622c5103f5b6da50fa122cdbdf7ab1/httpie/cli/dicts.py#L28-L31

A:A A: A:B should not error, but result in A:B. The A: bit should just drop any previously specified value(s) for A.

I wonder about the type mixing: maybe we should just ensure str for each item instead of throwing an exception.

Other thoughts:

  • Regardless of the approach we end up going for, we should make sure it plays well with --session and test-cover it.
  • This is actually a breaking change (A:A A:B will send a different request than before), so we’ll need to bump the major version.

@isidentical
Copy link
Contributor

What if we patched PreparedRequest.prepare_headers()? thinking

Seems like the underlying urllib3.urlopen() method (through http.client) works alright with MultiDict(), so I'd say that is also an option (we can just set the .headers = <>) after the creating the PreparedRequest (possibly not a publicly documented API to mess with, but shouldn't be a much problem).

I'll create another branch with an implementation of the following suggestion.

A:A A: A:B should not error, but result in A:B. The A: bit should just drop any previously specified value(s) for A.

Thanks for clarifying the actual behavior, I went with a little bit of an improvisation on this but can modify it to take this into account.

I wonder about the type mixing: maybe we should just ensure str for each item instead of throwing an exception.

I feel like bytes is the way to go instead of ensuring everything is a string. Since finalize_headers() already encode() everything to bytes, so we can do that during the insertion. But it won't be needed if we divert to MultiDict.

Regardless of the approach we end up going for, we should make sure it plays well with --session and test-cover it.

Sure (the implementation above was just a sketch of what I was planning, not the original patch so I tested the bare minimum. actual implementation will probably include way more comprehensive tests).

This is actually a breaking change (A:A A:B will send a different request than before), so we’ll need to bump the major version.

Wouldn't it make sense to bump the version number on the PR that is actually releasing this? But yes, this is a change of behavior (which I guess can be justified since AFAIK the previous behavior was undocumented).

Thanks for your feedback again, will let you when I have the second implementation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working planned Solution is being worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants