encode/httpx#1214 prefer sending outbound cookies separately to improve headers compression

[cdeler]

According to the rfc7540#section-8.1.2.5,

To allow for better compression efficiency, the Cookie header field
MAY be split into separate header fields

The h2 framework is used by the httpx client, and some users require to have an ability to send cookies via separate headers to follow http2 practice. Or at least to make this option configurable (more: encode/httpx#1214).

As h2 decides how to properly send data over the transport, h2 seems to be a right place to add an ability to split the cookie headers. Also the opposite functionality is added to the inbound cookies processing (ref).

Notes:

I have concerns about my PR, may be you can help me with suggestings:

• I used the "server" connection in the client connections tests. I don't know if it's a right way to do
• may be it makes sense to hide this feature under a config? If so, do you have suggestions where to add it?

[Kriechi]

At this point, what are the trade-offs to simply always create HeaderTuple's?

[Kriechi]

Since we use the same approach in

h2/src/h2/utilities.py

Lines 549 to 552 in c7f967d

	if isinstance(header, HeaderTuple):
	yield header.__class__(header[0].strip(), header[1].strip())
	else:
	yield (header[0].strip(), header[1].strip())

, I don't mind either way.

[cdeler]

I'm not sure about that, but I see that all outbound cookies are handled using this pattern: isinstance(header, HeaderTuple): ...

h2/src/h2/utilities.py

Lines 534 to 538 in 63b6b97

	for header in headers:
	if isinstance(header, HeaderTuple):
	yield header.__class__(header[0].lower(), header[1])
	else:
	yield (header[0].lower(), header[1])

h2/src/h2/utilities.py

Lines 548 to 552 in 63b6b97

	for header in headers:
	if isinstance(header, HeaderTuple):
	yield header.__class__(header[0].strip(), header[1].strip())
	else:
	yield (header[0].strip(), header[1].strip())

Also I'm not sure about using NeverIndexedHeaderTuple (e.g. how it can negatively affect compression)

[sethmlarson]

Question for some context: if a client sends Cookie headers that are already split does h2 combine them somewhere or will the split header fields be encoded as expected?

[Kriechi]

@sethmlarson I asked myself the same thing, and found this:

h2/src/h2/utilities.py

Lines 583 to 603 in c7f967d

	def _combine_cookie_fields(headers, hdr_validation_flags):
	"""
	RFC 7540 § 8.1.2.5 allows HTTP/2 clients to split the Cookie header field,
	which must normally appear only once, into multiple fields for better
	compression. However, they MUST be joined back up again when received.
	This normalization step applies that transform. The side-effect is that
	all cookie fields now appear *last* in the header block.
	"""
	# There is a problem here about header indexing. Specifically, it's
	# possible that all these cookies are sent with different header indexing
	# values. At this point it shouldn't matter too much, so we apply our own
	# logic and make them never-indexed.
	cookies = []
	for header in headers:
	if header[0] == b'cookie':
	cookies.append(header[1])
	else:
	yield header
	if cookies:
	cookie_val = b'; '.join(cookies)
	yield NeverIndexedHeaderTuple(b'cookie', cookie_val)

[Kriechi]

as a first step to a review, I fixed our project CI (which was a challenge by itself).
#1276

I don't see any obvious issues with the code itself - though I still need time to think and read through the spec and implications. Happy to hear other thoughts in the mean time!

@cdeler you said:

some users require to have an ability
and
to add an ability to split

I don't see an "option" in your PR - its "always on"?
Or do you think this should be the new one-and-only way to handle cookies?

[cdeler]

Hello @Kriechi

Or do you think this should be the new one-and-only way to handle cookies?

I don't think so (because RFC says "may") :-) , so that I updated the PR adding split_outbound_cookies flag to the h2.config.H2Configuration (False by default)

[Kriechi]

LGTM!

[kuntaoKZ]

Hello,
seems this change was not covered in 4.1.0 version?