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

Use , rather than , for combine operation #207

Closed
annevk opened this issue Jan 28, 2016 · 19 comments
Closed

Use , rather than , for combine operation #207

annevk opened this issue Jan 28, 2016 · 19 comments

Comments

@annevk
Copy link
Member

annevk commented Jan 28, 2016

Fetch defines a combine operation mostly for XMLHttpRequest. We combine other values without 0x20, but we don't do that here mostly due to legacy. Should we just align these?

@annevk
Copy link
Member Author

annevk commented Jan 29, 2016

I think we should by the way, this is mostly a check for implementers.

@annevk
Copy link
Member Author

annevk commented Mar 25, 2016

@hallvors, thoughts?

@hallvors
Copy link

A bit confused by this summary - should " ," be ", " ?

@annevk annevk changed the title Use , rather than , for combine operation Use , rather than , for combine operation Mar 25, 2016
@annevk
Copy link
Member Author

annevk commented Mar 25, 2016

Yeah, my bad.

@hallvors
Copy link

Gut feeling based on absolutely nothing is that it should be OK to align this without causing web compatibility problems. I may have to eat those words of course.. :p but I have rarely seen scripts setting same value multiple times and relying on concatenation. On the other hand, I think we should look at how HTTP does things and what common implementations do with list headers the browser sends - for consistency and aesthetic reasons. AFAIK HTTP says white-space is optional but if it's commonly implemented I could support making it mandatory..

@hallvors
Copy link

Firefox disagrees with itself :) "en-US,en;q=0.5" but "gzip, deflate".

@hallvors
Copy link

So well, I think we should drop that space.. and I think Necko should announce "gzip,deflate" support ;)

@mnot
Copy link
Member

mnot commented Apr 11, 2016

One problem might be around how header values are compared by caches for purposes of Vary support; if they're dumb, they'll consider differences in whitespace to be different cache keys.

There was a FF bug about this a while back, to align headers across implementations.

@annevk
Copy link
Member Author

annevk commented Apr 11, 2016

Paging @mcmanus for more thoughts.

@mayhemer
Copy link

According rfc2616 [1] LWS (=white spaces excluding CR LF) are permitted around comma separated field-value's. That same article also says something about ordering of the combined field-value's being something that affects interpretation of it. Looks like "gzip,deflate" is something else than "deflate,gzip" (in this case definitely, but for e.g. caching directives that should apply as well, regardless that semantically they would be identical when reordered).

Based on that, for instance caches or proxies can easily be dumb when comparing the Vary headers (when LWS are removed from the raw string).

I don't have arguments what to do here, but it's IMO better to not add spaces that adding them. And being consistent is definitely a good thing ;)

[1] https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2

@mayhemer
Copy link

@annevk
Copy link
Member Author

annevk commented May 30, 2016

Should probably also ask @tyoshino and @yutakahirano explicitly.

@yutakahirano
Copy link
Member

Chrome is inconsistent, too.

accept-encoding: gzip, deflate, sdch, br
accept-language: ja,en-US;q=0.8,en;q=0.6

@yutakahirano
Copy link
Member

I think it's OK to change the spec for XHR and WebSocket.
We don't have a plan to replace all ', ' with ',' in Chrome though.

@mnot
Copy link
Member

mnot commented May 31, 2016

I'm not sure that consistency is a good thing, here.

While you might be able to get alignment between client-generated headers over time (because there are relatively few codebases), it's not really practical to think that will happen for server-generated ones -- or even for those generated by intermediaries (e.g., request headers inserted by a forward or reverse proxy).

However, if clients mostly send it with the spacing you specify, people will inevitably write code to expect that behaviour, and things will break in weird ways.

What do you actually improve by going to this level of detail, and is it worth the risk noted above?

I'm not against this strongly, just wondering why it's necessary.

@annevk
Copy link
Member Author

annevk commented May 31, 2016

I think it's exactly the same as us having to define the casing of header names. And yeah, what you describe is basically a short version of https://annevankesteren.nl/2016/05/client-server.

@mnot
Copy link
Member

mnot commented May 31, 2016

Nice. I think you need and @martinthomson need to mix that with https://tools.ietf.org/html/draft-thomson-postel-was-wrong and make some grey beards even unhappier.

@annevk
Copy link
Member Author

annevk commented May 31, 2016

That looks pretty good. I prefer the principles under https://www.w3.org/TR/html-design-principles/#interoperability more I think, but it's quite similar.

I think a problem with @martinthomson's work is that if you are maximally strict, extensions will be trickier or need to be negotiated. We have many client-side formats where you can add new things, before all clients support them, and nothing breaks down for the end user if you do. This is true for protocols too, including a new header doesn't cause a "corrupted content" screen to appear in an older Firefox. What is important is that it's defined what happens, not that you immediately treat such occurrences as fatal errors.

@mnot
Copy link
Member

mnot commented May 31, 2016

Good point. I think there are a variety of factors influencing the decision, including where you're at in the protocol/format's life cycle (as you say above), relative sizes of implementation pools, etc.

I'd like to see someone produce a nuanced piece on this; most of what's been written has been very dogmatic. Yours is the closest I've seen yet,but it needs more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants