feat: support multiple headers (Replace #1090)

[medz]

Replace:

• feat: support multiple headers #1090

• I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Contribution guidelines:

• See our contributor guide for general expectations for PRs.
• Larger or significant changes should be discussed in an issue before creating a PR.
• Contributions to our repos should follow the Dart style guide and use dart format.
• Most changes should add an entry to the changelog and may need to rev the pubspec package version.
• Changes to packages require corresponding tests.

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

[medz]

Hello, @brianquinlan :

Since #1090 had some incorrect changes (almost all in the test files), I created this new PR to replace it.

This PR test passes, but one test fails, which does not seem to be caused by this PR:

00:01 +23 -2: test/io/client_test.dart: #send with an invalid URL [E]
   Expected: throws (<Instance of 'ClientException'> with `uri`: Uri:<http://http.invalid> and <Instance of 'SocketException'> with `SocketException.toString`: match 'ClientException with SocketException.*, uri=http://http.invalid')
     Actual: <Instance of 'Future<IOStreamedResponse>'>
      Which: threw ClientException:<ClientException: Connection closed before full header was received, uri=http://http.invalid>
             stack package:http/src/io_client.dart 151:7 IOClient.send

             which is not an instance of 'SocketException'

   package:test_api OutstandingWork.complete

[medz]

@brianquinlan Although this PR also implements the Web headers API of the MDN specification, it does not destroy the widely used common interface. It only changes the type, from Map<String, String>? to Object?.

In short, the usage of previous versions of http is compatible. According to the init specified by the Web headers API, the following types are supported:

Headers
String
Iterable<String>
Iterable<(String, String)>
Iterable<Iterable<String>>
Map<String, String>
Map<String, Iterable<String>>

As you said at #1090 (comment), Headers are indeed implemented using Record.

Seen in headers.dart Line 13:

final List<(String, String)> _storage;

[brianquinlan]

@brianquinlan Although this PR also implements the Web headers API of the MDN specification, it does not destroy the widely used common interface. It only changes the type, from Map<String, String>? to Object?.

In short, the usage of previous versions of http is compatible. According to the init specified by the Web headers API, the following types are supported:

This will still break any code that implements BaseClient/BaseResponse/BaseRequest.

In your previous PR, you wrote that:

This PR will indeed cause breaking changes, but it will make subsequent releases permanent. Splitting using , is indeed possible, but many http header values contain , which can easily cause wrong splitting.

According to RFC-2616 section 4.2:

Multiple message-header fields with the same field-name MAY be
present in a message if and only if the entire field-value for that
header field is defined as a comma-separated list [i.e., #(values)].
It MUST be possible to combine the multiple header fields into one
"field-name: field-value" pair, without changing the semantics of the
message, by appending each subsequent field-value to the first, each
separated by a comma. The order in which header fields with the same
field-name are received is therefore significant to the
interpretation of the combined field value, and thus a proxy MUST NOT
change the order of these field values when a message is forwarded.

So any implementation that differentiates between repeated fields and comma-separated field values is incorrect.

[medz]

@brianquinlan

This will still break any code that implements BaseClient/BaseResponse/BaseRequest.

Yes, this PR does make breaking changes to the headers of Client/Request/Response, but to be compatible with the previous Map we can add to the Headers using:

operator [](String key) => get(key);

operator []=(String key, String value) => set(key, value);

To achieve compatibility, but if someone is doing Map processing, it will be difficult to allow this situation. Some modifications are needed.

According to RFC-2616 section 4.2:

Multiple message-header fields with the same field-name MAY be
present in a message if and only if the entire field-value for that
header field is defined as a comma-separated list [i.e., #(values)].
It MUST be possible to combine the multiple header fields into one
"field-name: field-value" pair, without changing the semantics of the
message, by appending each subsequent field-value to the first, each
separated by a comma. The order in which header fields with the same
field-name are received is therefore significant to the
interpretation of the combined field value, and thus a proxy MUST NOT
change the order of these field values when a message is forwarded.
So any implementation that differentiates between repeated fields and comma-separated field values is incorrect.

You are right about this position, but it is also wrong, because this is the agreed part of HTTP Protocol Message generate. This is not something that the http package handles!

Something like this is already done in dart:io's HttpClient! And XHR also has this feature!

Here's a demo of me using this Headers to send the same headers using dart:io HttpClient and JS fetch:

dart:io

JS (Bun.js)

So this is not something that the http package needs to handle, we just need to deliver the headers correctly to the http client (any! XHR, Fetch, HttpCLient and more...) when they use Socket to deliver the Http Protocol Message!

The accusation of http package is only to deliver the declared headers correctly!

[medz]

In addition, the reason why I chose the MDN specification headers implementation instead of HttpHeaders in dart:io as the implementation target is because MDN headers are easier to use (in my opinion)

One more off-topic idea, it would be easier to switch the web client implementation from XHR to Fetch, I've tried it with success:

[brianquinlan]

@medz I don't fully understand you. My point is that, according to RFC 2616, the following message are equivalent:

Fruit: Apple
Fruit: Banana

and

Fruit: Apple,Banana

The package:http Client interface presents headers in the second form through Map<String, String>. But they are easy to transform into the first.

[brianquinlan]

In addition, the reason why I chose the MDN specification headers implementation instead of HttpHeaders in dart:io as the implementation target is because MDN headers are easier to use (in my opinion)

I agree but I don't think that it is worth breaking the existing API to change it.

[medz]

I don't think that it is worth breaking the existing API to change it.

True, but it's difficult to implement the functionality of set-cookie and cookie passes without breaking headers changes. This is a difficult choice, the http package has a large number of developers using it, and any disruptive move will cause pain to the developers.

Sorry, I didn't get any useful discussion information from the discussion of #24. But this PR does fix that.

In the HTTP protocol message, cookie is a very unique existence. It cannot be cut by , correctly. Others that allow duplication are basically implemented using , cutting, which is also incorrect, but not cutting is also incorrect. But I have referred to many http libraries, and the usual method is , cutting.

But this does not work for cookies. It must be a passed header to be valid.

I suggest that we discuss how to achieve this

Is it possible to manage this using the cookies field similar to what is done in dart:io?

[brianquinlan]

I don't think that it is worth breaking the existing API to change it.

True, but it's difficult to implement the functionality of set-cookie and cookie passes without breaking headers changes. This is a difficult choice, the http package has a large number of developers using it, and any disruptive move will cause pain to the developers.

Sorry, I didn't get any useful discussion information from the discussion of #24. But this PR does fix that.

In the HTTP protocol message, cookie is a very unique existence. It cannot be cut by , correctly. Others that allow duplication are basically implemented using , cutting, which is also incorrect, but not cutting is also incorrect. But I have referred to many http libraries, and the usual method is , cutting.

But this does not work for cookies. It must be a passed header to be valid.

Why doesn't this work for cookies? By RFC 6265 4.1.1 cookie values may not contain commas.

Why doesn't this work with cookies?

I suggest that we discuss how to achieve this

Is it possible to manage this using the cookies field similar to what is done in dart:io?

That seems reasonable.

[medz]

Well, you convinced me. I think this PR is no longer necessary. Its changes are too big and will cause destructive changes to the existing requdst, response, and client.

[medz]

@brianquinlan The following is a standard set-cookie template:

set-cookie: has_recent_activity=1; path=/; expires=Sat, 06 Jan 2024 03:32:46 GMT; secure; HttpOnly; SameSite=Lax

When there are multiple set-cookie, it is no longer valid. The purpose of this PR is to solve the problem that set-cookie sent by the server to the client can be processed correctly.

And set-cookie contains , If multiple set-cookies are combined using , merging is completely wrong.

Off topic: Maybe this PR is no longer important to me, I spent a day making a replacement for package:http that is fully compliant with the MDN spec Web API and handles all headers correctly.