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

consider guaranteeing the iteration order of HeaderMap::get_all #2466

Closed
robjtede opened this issue Nov 26, 2021 · 1 comment · Fixed by #2467
Closed

consider guaranteeing the iteration order of HeaderMap::get_all #2466

robjtede opened this issue Nov 26, 2021 · 1 comment · Fixed by #2467
Labels

Comments

@robjtede
Copy link
Member

It was brought up that header collapsing is difficult without being able to find out/guarantee ordering at some level.

A recipient MAY combine multiple header fields with the same field name into one "field-name: field-value" pair, without changing the semantics of the message, by appending each subsequent field value to the combined field value in order, 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; a proxy MUST NOT change the order of these field values when forwarding a message.
- https://datatracker.ietf.org/doc/html/rfc7230#section-3.2.2

This idea might also extend to guaranteeing same-name header orders elsewhere.

@robjtede robjtede added needs-investigation A-http project: actix-http labels Nov 26, 2021
@robjtede robjtede added this to the actix-web post-v4 milestone Nov 26, 2021
@mkantor
Copy link
Contributor

mkantor commented Nov 27, 2021

It appears that the current GetAll iterator (tested in 89c6d62) already does iterate in insertion order, while the version in actix-web 3.x does not (it's close, but the first two entries are transposed, e.g. 2,1,3,4,5,6,…).

I think this would only be a doc change and perhaps some tests to prevent regression.

@robjtede thanks for filing this on my behalf!

robjtede added a commit that referenced this issue Nov 28, 2021
robjtede added a commit that referenced this issue Nov 28, 2021
robjtede added a commit that referenced this issue Nov 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants