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

"get, decode, and split a header value" algorithm appears to choke on malformed values #1778

Closed
ricea opened this issue Sep 27, 2024 · 2 comments

Comments

@ricea
Copy link
Collaborator

ricea commented Sep 27, 2024

What is the issue with the Fetch Standard?

To reproduce:

  1. Run the "get, decode, and split a header value" algorithm on malformed input like "a""b"

Expected behaviour:

Algorithm returns something, or possibly signals an error.

Actual behaviour:

Algorithm hits an assert.

As far as I can tell, there's nothing in the callers to this algorithm that guarantees the input is well-formed.

My interpretation of the spec steps inside the loop when applied to the input "a""b":

  1. 0 characters are appended to temporaryValue.
  2. collecting an HTTP quoted string results in a being appended to temporaryValue. postion now points to the third ".
  3. Whitespace is trimmed from temporaryValue. It is still a.
  4. a is appended to values.
  5. temporaryValue is cleared.
  6. We do not return.
  7. code point at position is ", not ,, so the assert fails.

If an implementer ignores the assert failure I think they will get the output «a, b », which is a behaviour.

Based on code inspection I believe Chromium will reject this particular input, but something like name="a""b" will be treated as name="a\"\"b", which is also a behaviour.

@annevk
Copy link
Member

annevk commented Sep 27, 2024

I have the feeling you skipped step 5.2.2? At least the way I read https://fetch.spec.whatwg.org/#header-value-get-decode-and-split is that once you see the first " and collect "a" (appending a to temporaryValue), position is not past the end of input, so you go back to the start of the while loop. Then you append nothing to temporaryValue as you're looking at " so then you collect "b" (appending b to temporaryValue). And only then do you start trimming whitespace.

I also double checked with the changes we recently made in #1769 and I don't think they regressed something, so if there is some kind of error here (and it does seem like we don't match browsers and maybe ought to do something with this), it's a novel one.

@ricea
Copy link
Collaborator Author

ricea commented Sep 28, 2024

You're right, I missed the continue. Sorry about that.

@ricea ricea closed this as completed Sep 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants