fix: treat HTAB as optional whitespace in If-None-Match / If-Match lists#77
Merged
Merged
Conversation
`parseTokenList` only skipped SP (0x20) between entity-tags, so a tab
around the comma in an `If-None-Match` (or `If-Match`) list left the tab
glued to the next tag, which then never compared equal:
fresh({ 'if-none-match': '"foo",\t"bar"' }, { etag: '"bar"' }) // was false
Per RFC 7230 section 3.2.3, `OWS = *( SP / HTAB )`, and the list rule of
section 7 allows OWS around the comma, so a tab-separated list is valid and
must match. A missed match makes a conditional request return 200 instead
of 304, defeating caching. Treat HTAB (0x09) as whitespace too.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
parseTokenListonly skips SP (0x20) between entity-tags, so a tab around the comma in anIf-None-Match(orIf-Match) list is treated as part of the next tag, which then never matches:"bar"is a member of the list in both cases.Why it's a bug
Per RFC 7230 §3.2.3,
OWS = *( SP / HTAB ), and the list rule of §7 is1#element => element *( OWS "," OWS element ). A tab around the comma is therefore valid wire syntax that any client or proxy may emit, so the list must still match. When it doesn't, a conditional request that should produce304 Not Modifiedreturns a full200, defeating caching.The sibling jshttp library
negotiatoralready treats HTAB as whitespace (it parses its lists withString.prototype.trim()), so honoring OWS-with-tab is the established convention here — the SP-only handling is an oversight.Fix
Add
0x09(HTAB) to the whitespace case inparseTokenList:Verification
test/fresh.js(tab-separated list); fails before, passes after.