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

What does "combined value" return for a name not in the header list? #752

Closed
jyasskin opened this issue Jun 1, 2018 · 7 comments

Comments

3 participants
@jyasskin
Copy link
Member

commented Jun 1, 2018

A combined value, given a name (name) and header list (list), is the values of all headers in list whose name is a byte-case-insensitive match for name, separated from each other by 0x2C 0x20, in order.

I'd appreciate specifying one of these so I don't accidentally write a use that's ambiguous.

@annevk

This comment has been minimized.

Copy link
Member

commented Jun 2, 2018

The current setup requires you to do a contains check (which maybe we should rename to has?).

I want to add "get" to https://fetch.spec.whatwg.org/#concept-header-list that gives you null or a byte sequence, but I'm not a 100% sure if the current "combined value" definition is good enough.

In particular, I think @mnot's structured headers don't have the additional space. So:

Some-String-Header: "test
Some-String-Header: test"

ends up with "test,test" and not "test, test" as Fetch (and believe most implementations) do.

@annevk

This comment has been minimized.

Copy link
Member

commented Jun 2, 2018

@domenic was also interested in this.

@mnot

This comment has been minimized.

Copy link
Member

commented Jun 27, 2018

SH doesn't specify how to combine headers; HTTP does. Currently it doesn't have advice about this, but if we did enough testing, perhaps we could get some in.

SH could specify how to combine headers when the actual SH processor is doing it; do you think that would be useful?

@annevk

This comment has been minimized.

Copy link
Member

commented Jul 31, 2018

See also #757 about us currently not excluding empty values whereas HTTP does do that. (I don't think browsers do that universally though.)

@annevk

This comment has been minimized.

Copy link
Member

commented Jul 31, 2018

@mnot I don't think it makes sense for SH to define it. SH should just operate on combined values and leave combining to HTTP (or Fetch).

annevk added a commit that referenced this issue Oct 11, 2018

Change combined values
This marks XMLHttpRequest's approach in setRequestHeader() for combining values as naïve (it doesn't follow the spirit of HTTP, but is technically allowed) and changes "combined value" to more closely match HTTP semantics.

Tests: web-platform-tests/wpt#13471.

Fixes #757. (Addresses #752 to some extent, but leaving that open for a better API.)
@annevk

This comment has been minimized.

Copy link
Member

commented Oct 12, 2018

I created httpwg/http-core#148 to discuss how things are to be combined per HTTP and thus far the feedback I got is that this is largely up to the implementation, so if we always used 0x2C 0x20 that'd be fine. It does seem a little bad that implementations are allowed to vary on this, but I guess we can let that be somebody else's problem.

annevk added a commit that referenced this issue Oct 17, 2018

Define parsing for X-Content-Type-Options in detail
And add some of the infrastructure needed to define parsing better for all headers going forward (needed for #814). Fixes #752.

This also fixes an issue with CORB as it simply assumed an X-Content-Type-Options was present.

Tests: web-platform-tests/wpt#13559.
@annevk

This comment has been minimized.

Copy link
Member

commented Oct 17, 2018

#818 has a fix for this in the form of a "get" operation.

@annevk annevk closed this in #818 Nov 1, 2018

annevk added a commit that referenced this issue Nov 1, 2018

Define parsing for X-Content-Type-Options: nosniff in detail
And add some of the infrastructure needed to define parsing better for all headers going forward (needed for #814). Fixes #752.

This also fixes an issue with CORB as it simply assumed an X-Content-Type-Options was present.

Tests: web-platform-tests/wpt#13559.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.