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

Clarify behavior for multiple values for CH headers #700

Closed
igrigorik opened this issue Sep 29, 2018 · 13 comments · Fixed by #862
Closed

Clarify behavior for multiple values for CH headers #700

igrigorik opened this issue Sep 29, 2018 · 13 comments · Fixed by #862
Assignees

Comments

@igrigorik
Copy link
Member

Migrating feedback from Kari Hurtta..

On Wed, Jul 25, 2018 at 9:14 AM HTTP Working Group ietf-http-wg@w3.org
wrote:

| If DPR occurs in a message more than once, the last value overrides
| all previous occurrences.

So is that saying that if there is several DPR values on

DPR Header Field, last value is used ?

Yes, that's the intent.

If that in interpreted only that if last DPR header field is used,

and header field is ignored if it does not fit

DPR = 1DIGIT [ "." 1DIGIT ]

syntax, result is different is recipient combines multiple
DPR headers.

The intent was to specify that last valid value (regardless if it is within
a combined or separate header) is the one that should be used by the
client. My read of the current wording is that it matches this intent, but
I'll defer to the experts here on how to best word this if that's not the
case.

Mark, Julian: any guidance on this one?

| If occurs in a message more than once, the last value
| overrides all previous occurrences.

So it should mention that on these header fields also last value
is used when values are ',' -separated.

My understanding is that this is implicit, but if that's not the case,
happy to rework the wording.


Julian:

No, it's not implicit, so yes, a clarification would be good. I don't think we have good text yet.

@igrigorik igrigorik self-assigned this Sep 29, 2018
@igrigorik
Copy link
Member Author

@reschke @mnot per httpwg/http-core#111 (comment), how about...

The “DPR” request header field is a number that indicates the client’s current Device Pixel Ratio (DPR), which is the ratio of physical pixels over CSS px (Section 5.2 of [CSSVAL]) of the layout viewport (Section 9.1.1 of [CSS2]) on the device.

DPR = 1DIGIT [ "." 1DIGIT ]

The last valid value of the aggregated DPR field value overrides all previous occurrences.

Does that make sense? Would appreciate your guidance on this one :)

@mnot
Copy link
Member

mnot commented Oct 1, 2018

I'm starting to wonder if Client Hints should adopt Structured Headers to avoid having to deal with these sorts of questions. Ilya, WDYT?

@igrigorik
Copy link
Member Author

@mnot that would hit a major reset on all existing CH implementations and set us back another couple years, I don't think this is the right call for CH. /cc @yoavweiss

Surely CH is not the first instance where we had to spell out behavior for selecting one out of N header values? That said, if we don't have established verbiage for this, I'm happy to just make up some language ourselves to clarify how it should behave.. what we're aiming for ain't rocket science. :)

@mnot
Copy link
Member

mnot commented Oct 2, 2018

I wouldn't want to set CH back that much (although it is lingering on :).

I only mention it because all of the defined syntax seems like it would easily fit into SH, and Chrome already has a good start on a SH parser, thanks to @jyasskin's work on signed exchanges, etc.

@colinbendell
Copy link

As an aside, the same clarification should also apply to content-dpr response headers. Currently Chrome treats multiple content-dpr responses headers as an error and ignores both.

@igrigorik
Copy link
Member Author

@mnot my preference is to get this over the finish line without introducing dependency on SH. For one, we're also relying on Feature-Policy for delegation and we decided against SH there.

WDYT of suggested text in #700 (comment) — warmer?

@colinbendell ack.

@mnot
Copy link
Member

mnot commented Oct 5, 2018

@igrigorik I hear you. My thinking is:

  • We've already waited a good while for CH, and it's not clear that it's really really done yet (I suspect we have at least one more round of doc review).

  • People are already using CH as a framework for new hints, so getting SH in would be best now if it's going to get in.

  • If CH are going to be commonly sent, having well-defined parsing behaviour and the possibility of better serialisations (which SH gives us) is very attractive.

To be clear -- this is me as me, not me as chair.

@yoavweiss
Copy link
Contributor

@mnot - could you clarify what you mean by CH adopting SH?

Do you mean that the DPR/Content-DPR headers would be defined as sh-item/sh-flat? Or that they'd be define as sh-list with extended processing model (which guarantees that the last item in the list "wins")? Or something else altogether?

@mnot
Copy link
Member

mnot commented Oct 8, 2018

Both DPR and Content-DPR could be sh-floats.

Accept-CH would be a sh-list of sh-identifiers.

Both Width and Viewport-Width would be sh-integers.

Accept-CH-Lifetime would be a sh-integer. I'm not really sure why its current ABNF is a list. @igrigorik?

Considering that parsing and serialisation algorithms for these haven't been defined yet at all, I don't think it would take a lot of work (and would be happy to do a PR).

@igrigorik
Copy link
Member Author

Accept-CH-Lifetime would be a sh-integer. I'm not really sure why its current ABNF is a list.

Good catch, I think that's an oversight and we can fix that.


Stepping back, my preference is (still) to proceed at this stage without introducing dependency on SH. First off, introducing SH does not address the issue we're actually trying to solve in this bug. Second, it adds another significant dependency to CH adoption that I would like to avoid at this stage, as it's yet another reason for a browser vendor to drag their feet and delay integrating support for SH.

To get CH over the last call barrier, I believe we have two outstanding tasks:

  1. Clarify Accept-CH-Lifetime definition, per above.
  2. Provide some minimum guidance on how to deal with aggregated values — see Clarify behavior for multiple values for CH headers #700 (comment).

I don't think we'll land on perfect text for (2), but this is not a unique or new gap vs other and existing specs, and I don't think we should block on this.

@mnot does this sound reasonable?

@igrigorik
Copy link
Member Author

friendly bump :-)

@mnot @reschke PTAL, any thoughts or guidance on the above text?

@mnot
Copy link
Member

mnot commented Nov 3, 2018

The last valid value of the aggregated DPR field value overrides all previous occurrences.

The issue is that it's defined to only have one value. I think you'd need to say something like:

if more than one DPR header field is present (either because multiple header instances exist, or because they've been folded into comma-separated values)...

Or, to really define it, define an algorithm.

@yoavweiss
Copy link
Contributor

This was discussed at the HTTPWG meeting today and it was decided to go ahead with Structured Headers!

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

Successfully merging a pull request may close this issue.

4 participants