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

Discussion: h11 API changes and real-world use experience #17

Open
njsmith opened this issue Oct 25, 2016 · 12 comments
Open

Discussion: h11 API changes and real-world use experience #17

njsmith opened this issue Oct 25, 2016 · 12 comments

Comments

@njsmith
Copy link
Member

njsmith commented Oct 25, 2016

h11 is in this awkward life stage, where I believe it's basically complete and correct... but it's an unusual API with few precedents, and it hasn't been used much beyond examples and demos. (For example, as of 2016-10-24, I'm not aware of any even half-serious attempts to use it to write an HTTP client.) So maybe the API has some terrible glaring flaws – there's no way to know until people try it. Which means that freezing the API now would be very risky. But OTOH, we do want people to actually use it for real things, because otherwise we'll never find out whether it works or not. So: I'm creating this issue as a central place for discussion of breaking changes; I can't promise we won't break things, but I can at least promise to give everyone fair warning.

Please do go ahead and start using h11 for real things. And when you do, please (for now) follow these guidelines:

  • Pin your version. For example, in your install_requires= and/or requirements.txt, do not use: h11 >= 0.6.0. Instead, use: h11 ~= 0.6.0.
    • You can also use == if you prefer. The difference is that while both == 0.6.0 and ~= 0.6.0 will disallow upgrading to 0.7.0, ~= 0.6.0 allows upgrading to 0.6.1. Note that I don't intend to release any 0.x.1 versions unless we find some terrible security bug or similar.
  • Please report back how it works for you, e.g. by posting a comment on this issue.
    • Especially any rough spots you ran into where the API wasn't as helpful as it could be.
    • Especially if you didn't run into any rough spots, because that information will be really useful for giving us the confidence to eventually release 1.0 and stop breaking things.
  • Subscribe to this issue to get warning of potential breaking API changes, and a chance to influence them

CC'ing folks who I think might be using h11: @mike820324 @Lukasa @jeamland

@njsmith
Copy link
Member Author

njsmith commented Oct 25, 2016

More CC's that I missed: @chhsiao90 @RazerM @dabeaz

@Lukasa
Copy link
Member

Lukasa commented Oct 25, 2016

I'm in the middle of prototyping h11 as a replacement for http.client in a few places, so hopefully l'll start having some sensible opinions on this shortly.

@mike820324
Copy link

Currently @chhsiao90 and I are building a http proxy tool, microProxy, that use hyper-h2 and h11.

Since we want out proxy as transparent as possible, it would be nice that we can find a way to keep the original case of a HTTP Request/Response header. Currently, the h11 will change the header field to lower case.

Here is a commit that @chhsiao90 did which will keep the case of HTTP Header.

@njsmith
Copy link
Member Author

njsmith commented Oct 26, 2016

Currently @chhsiao90 and I are building a http proxy tool, microProxy, that use hyper-h2 and h11.

Awesome!

original case of a HTTP Request/Response header

...hmm. I'm reluctant to do this without a very clear rationale, since having to cope with unnormalized headers adds non-trivial complexity in a security-sensitive area; plus it somewhat pushes back against h11's whole purpose of exposing HTTP's semantics while encapsulating the syntactic details. Looking around, it appears that the standard HTTP libraries in both Node and Go normalize header names (node -> lowercase, Go -> Camel-Case), and they seem to do okay. (Plus HTTP/2 simply removed the ability to even transmit non-normalized headers, so any clients/servers that care about this are in trouble in that regard.) How certain are you that you really need this -- can you point to any specific issues? Does the goal of being "as transparent as possible" also extend to other semantically-null parts of the HTTP wire encoding, like trailing whitespace on header values? Do you need to support illegal headers? Are you implementing the parts of RFC 7230 where it says that proxies MUST add/drop certain headers?

@Lukasa
Copy link
Member

Lukasa commented Oct 26, 2016

To double-down on this: all header field names in h2 are lower case. All of them, no choices, you just gotta do it. hyper-h2 will reject header fields it receives that aren't and will forcibly downcase the user's header field names.

I recommend not getting too worked up about keeping header case as provided.

@chhsiao90
Copy link
Member

chhsiao90 commented Oct 26, 2016

Thanks for your comments and it all make sense and had convinced me about the case.

My only concert is that if some user used the proxy server that used h11 for http parsing,
to test some http server that had problem on handling http header with their case.
And the user will be confused in that situation.
But I think that would never happen.

So maybe keep current design about the case is the best choice!
And there is no reason to increase the code complexity to implement such a useless functionality.

@mike820324
Copy link

Sounds good to me, in most implementation it is very rare to see the malformed header format. And since node and go will also normalize the header format. I think I'm good with this part too.

Thank you all for your clarification. :)

@Lukasa
Copy link
Member

Lukasa commented Nov 1, 2016

So the first thing I've bumped into, API-wise, with h11 is in #19.

@njsmith
Copy link
Member Author

njsmith commented Nov 26, 2016

I just release h11 v0.7.0, which is backwards compatible with v0.6.0. Changelog

@Lukasa
Copy link
Member

Lukasa commented Dec 1, 2016

Bumped into something new: #23.

@Lukasa
Copy link
Member

Lukasa commented Dec 8, 2016

Relevant to the people in this thread: urllib3/urllib3#1067.

@njsmith
Copy link
Member Author

njsmith commented Jan 3, 2017

FYI, I just made a minor breaking change to h11 trunk: it now applies stricter validation to outgoing headers. Specifically, it now validates that outgoing header names and values are valid (e.g., if you try to put a newline into a header value it will reject that), and it now requires that the header values you passed in should have no leading or trailing whitespace. (Previously it accepted such values and discarded the whitespace.)

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

4 participants