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

unnecessary use of 1#element for header field definitions #210

Closed
royfielding opened this issue Mar 27, 2019 · 10 comments · Fixed by #421
Closed

unnecessary use of 1#element for header field definitions #210

royfielding opened this issue Mar 27, 2019 · 10 comments · Fixed by #421

Comments

@royfielding
Copy link
Member

royfielding commented Mar 27, 2019

When working on #164 I noticed that our existing use of 1#element (header fields requiring at least one list item) is kind of pointless. The following fields use it:

 Cache-Control   = 1#cache-directive
 Transfer-Encoding = 1#transfer-coding
 Connection        = 1#connection-option
 Upgrade          = 1#protocol
 Trailer = 1#field-name
 Via = 1#( received-protocol RWS received-by [ RWS comment ] )
 Content-Encoding = 1#content-coding
 Content-Language = 1#language-tag
 If-Match = "*" / 1#entity-tag
 If-None-Match = "*" / 1#entity-tag
 Accept-Charset = 1#( ( charset / "*" ) [ weight ] )
 Accept-Language = 1#( language-range [ weight ] )
 Vary = "*" / 1#field-name
 WWW-Authenticate = 1#challenge
 Proxy-Authenticate = 1#challenge

It is also used (incorrectly?) within Byte Ranges fields as

 byte-range-set  = 1#( byte-range-spec / suffix-byte-range-spec )
 acceptable-ranges = 1#range-unit / "none"

We should audit these because it makes more sense to handle empty fields as non-error conditions.

@mnot
Copy link
Member

mnot commented Sep 2, 2019

If you mean that we should change them to #rule rather than 1#rule, I tend to agree -- but IIRC there was a reason this was done. @reschke?

@reschke
Copy link
Contributor

reschke commented Sep 3, 2019

See #164. This change would reduce the complexity of the list rule. We'd just have to put the "minimally one instance" constraint into prose.

@mnot
Copy link
Member

mnot commented Sep 3, 2019

I think Roy's point is that we don't need to limit to a minimum number of elements -- even in prose.

@reschke
Copy link
Contributor

reschke commented Sep 3, 2019

Well, for at least some of these, we may have to say what it means if there is none.

@mnot
Copy link
Member

mnot commented Sep 3, 2019

From Roy's list, I've ticked the ones that I think that's true for:

  • Cache-Control = 1#cache-directive
  • Transfer-Encoding = 1#transfer-coding
  • Connection = 1#connection-option
  • Upgrade = 1#protocol
  • Trailer = 1#field-name
  • Via = 1#( received-protocol RWS received-by [ RWS comment ] )
  • Content-Encoding = 1#content-coding
  • Content-Language = 1#language-tag
  • If-Match = "*" / 1#entity-tag
  • If-None-Match = "*" / 1#entity-tag
  • Accept-Charset = 1#( ( charset / "*" ) [ weight ] )
  • Accept-Language = 1#( language-range [ weight ] )
  • Vary = "*" / 1#field-name
  • WWW-Authenticate = 1#challenge
  • Proxy-Authenticate = 1#challenge

You could argue that Accept-* need them too, but I think the right think will happen regardless...

@royfielding
Copy link
Member Author

royfielding commented Sep 5, 2019

I think that when an empty header field indicates an error instead of just an empty list, we should define it as 1#value. When a field might be generated, truncated, or whitespaced-out by multiple implementations in a chain of communication, allowing the field to be sent empty is a common simplification (or at least a common error case that only wastes a little bandwidth). This is particularly true if we expect extensions or intermediaries to drop or append some values, as is the case for all of those fields except If-Match.

@mnot
Copy link
Member

mnot commented Sep 9, 2019

That sounds about right. Although we should specify that error handling in prose too, I think.

@mnot mnot self-assigned this Jul 2, 2020
@mnot
Copy link
Member

mnot commented Jul 20, 2020

AIUI we want to remove 1# from the top level of all field values EXCEPT those ticked above (If-Match, If-None-Match, Vary, WWW-Authenticate, Proxy-Authenticate), because in each of those cases, an empty value is considered an error (typically, the intended semantics are unclear and therefore interop could suffer).

Questions:

  1. Is that correct, generally speaking?
  2. Is that specific list of headers correct?
  3. For the headers that we want to keep as 1#, should we define the semantics of an empty field value (if only to say that they're not defined, so it isn't interoperable)?

@reschke
Copy link
Contributor

reschke commented Jul 21, 2020

  • *-authenticate: same as a non-empty list containing only unknown auth schemes

@mnot
Copy link
Member

mnot commented Jul 29, 2020

That sounds like we want to remove 1# from *-Authenticate.

@mnot mnot mentioned this issue Jul 31, 2020
reschke added a commit that referenced this issue Aug 5, 2020
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.

3 participants