Conversation
|
Fixed format-related things mostly. I think we should still do a full repo |
Fixes python-hyper#1236. This patch makes all header operations operate on `bytes` and converts all headers and values to bytes before operation. With a follow up patch to `hpack` it should also increase efficiency as currently, `hpack` casts everything to a `str` first before converting back to bytes: https://github.com/python-hyper/hpack/blob/02afcab28ca56eb5259904fd414baa89e9f50266/src/hpack/hpack.py#L150-L151
c95e738 to
605f0ea
Compare
|
Please keep the changes in each PR to a minimum - in this case please revert the style-related changes. |
|
@Kriechi we should be good now, sorry for the trouble. |
| norm_headers = h2.utilities.normalize_outbound_headers(headers, None, False) | ||
| norm_headers = h2.utilities.normalize_outbound_headers( | ||
| h2.utilities.utf8_encode_headers(headers), None, False | ||
| ) |
There was a problem hiding this comment.
I'm contemplating making the encoding of headers part of the normalization pipeline, instead of being the responsibility of the outside caller.
What do you think about integrating it here?
There was a problem hiding this comment.
That's exactly what I wanted but unfortunately we have an option to bypass normalization, which breaks that neat abstraction :(
There was a problem hiding this comment.
utf8_encode_headers is currently called in two places: connections.py - push_stream and stream.py - send_headers. Both share a code path with H2Stream._build_headers_frames.
I'll try to refactor it and share the update to discuss.
Fixes #1236.
This patch makes all header operations operate on
bytesand converts all headers and values to bytes before operation. With a follow up patch tohpackit should also increase efficiency as currently,hpackcasts everything to astrfirst before converting back to bytes: https://github.com/python-hyper/hpack/blob/02afcab28ca56eb5259904fd414baa89e9f50266/src/hpack/hpack.py#L150-L151