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

characters allowed in field values #815

Closed
reschke opened this issue Feb 5, 2021 · 6 comments · Fixed by #846
Closed

characters allowed in field values #815

reschke opened this issue Feb 5, 2021 · 6 comments · Fixed by #846
Assignees

Comments

@reschke
Copy link
Contributor

reschke commented Feb 5, 2021

https://tools.ietf.org/html/rfc7540#section-10.3:

Similarly, HTTP/2 allows header field values that are not valid.
While most of the values that can be encoded will not alter header
field parsing, carriage return (CR, ASCII 0xd), line feed (LF, ASCII
0xa), and the zero character (NUL, ASCII 0x0) might be exploited by
an attacker if they are translated verbatim. Any request or response
that contains a character not permitted in a header field value MUST
be treated as malformed (Section 8.1.2.6). Valid characters are
defined by the "field-content" ABNF rule in Section 3.2 of [RFC7230].

There are multiple issues here:

  • it's confusing to say "HTTP/2 allows" when next they are actually forbidden with a MUST requirement; it might be better to say something like that the wire protocol in theory enables the transmission of these forbidden characters
  • this puts a normative requirement on top of the base spec; but so does the revision of the core specs (see field-value production rules out CTL characters, but these are common in the wild http-core#683). We need to make sure that there are no conflicting requirements. It also should be checked whether the currently present requirements are actually implemented in UAs (and if they are not, what could be done about it)
@Lukasa
Copy link
Collaborator

Lukasa commented Feb 5, 2021

It would presumably be more accurate to say that "HPACK allows", right?

@afrind
Copy link

afrind commented Feb 5, 2021

HPACK also allows encoding a field line with an empty name. I'm not sure this needs to be mentioned explicitly or not.

@reschke
Copy link
Contributor Author

reschke commented Feb 5, 2021

HPACK also allows encoding a field line with an empty name.

Well, that's consistent with HTTP/1.1 :-)

@martinthomson
Copy link
Collaborator

I hate having to do this, but what is possible and what is permissible need to be more clearly separated.

@martinthomson
Copy link
Collaborator

I'm going to tag this as editorial: the possible/permissible split is clearly editorial. And we've decided to depend on the core docs, and whether or not it was clear before what was allowed or not, this is now very crisp in the core semantics. I want to wait until we get #811 in before doing this though.

@martinthomson
Copy link
Collaborator

Discussed in the context of leading and trailing whitespace in -semantics. I think that we should be clearer here about a few things:

  1. CRLF and NUL are strictly prohibited
  2. leading and trailing WS are strictly prohibited
  3. Maybe loosen the requirement a little so that it is less strict about adherence to the ABNF

That makes this non-editorial, I think.

martinthomson added a commit to martinthomson/http2v2 that referenced this issue Apr 23, 2021
This encodes the conclusions from interim discussions:

1. CR, LF, and NUL cannot appear anywhere in field names or values.
2. SP and HTAB cannot appear at the start or end of field names or
values.
3. COLON cannot appear anywhere in a field name, except for the colon at
the start of a pseudo-header field name.

The strong requirements about validating fields according to ABNF has
been replaced.

The text is clearer about how the pieces fit together: it is HPACK that
allows any octet, whereas HTTP/2 makes certain choices invalid.

Closes httpwg#815.
@martinthomson martinthomson self-assigned this May 24, 2021
gregw added a commit to gregw/http2-spec that referenced this issue Jun 3, 2021
Improve on httpwg#846 that fixes for httpwg#815 by adding extra clarity:
 + The validation for uppercase characters is no longer listed separately
 + It is clearly stated that violations of the full HTTP ABNF field definition MAY be treated as *Malformed*
gregw added a commit to gregw/http2-spec that referenced this issue Jun 10, 2021
The validation for uppercase characters is no longer listed separately, but instead included with the minimal validation.
gregw added a commit to gregw/http2-spec that referenced this issue Jun 10, 2021
Factored out non controversial change to httpwg#864
martinthomson added a commit that referenced this issue Jun 10, 2021
Improve on fix for #815 for lowercase validation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants