Skip to content

fix: respect backslash-escaped quotes when splitting quoted parameters - #107

Open
rajanpanth wants to merge 1 commit into
jshttp:masterfrom
rajanpanth:fix/quoted-string-escaped-quotes
Open

fix: respect backslash-escaped quotes when splitting quoted parameters#107
rajanpanth wants to merge 1 commit into
jshttp:masterfrom
rajanpanth:fix/quoted-string-escaped-quotes

Conversation

@rajanpanth

Copy link
Copy Markdown

Summary

quoteCount() counts every literal DQUOTE to decide whether a comma/semicolon falls inside an open quoted-string, with no awareness of a preceding backslash. Per RFC 7230 §3.2.6, quoted-pair allows an escaped DQUOTE inside a quoted-string without closing it — so a parameter value containing \" throws off the parity count, and the parser thinks it's still inside a quoted string when the next media type begins.

var Negotiator = require('negotiator')
var n = new Negotiator({ headers: { accept: 'text/html;p="\\"", application/json' } })
n.mediaType(['application/json'])   // undefined -- should be 'application/json'

A perfectly valid Accept header that explicitly lists application/json gets it silently dropped, which could cause accepts/Express's res.format() to return a wrong-content-type response or a spurious 406.

Changes

  • lib/mediaType.js: quoteCount() now walks the string char-by-char, skipping the character after any backslash instead of naively counting every DQUOTE.
  • test/mediaType.js: two new cases alongside the existing unescaped-comma-in-quotes test (issue Fails to parse quoted parameters #36) — an escaped DQUOTE, and an escaped backslash immediately before a real closing DQUOTE (to confirm double-backslash doesn't false-positive).

Testing

  • Full suite: 251/251 passing (3 pending, pre-existing/unrelated).
  • eslint . clean.
  • Manually verified before/after with the repro above.

quoteCount() counted every literal DQUOTE to decide whether a comma or
semicolon fell inside an open quoted-string, with no awareness of a
preceding backslash. Per RFC 7230 section 3.2.6, quoted-pair allows an
escaped DQUOTE inside a quoted-string without closing it, so a value
like p="\"" threw off the parity count and caused the next,
unrelated media type in the Accept header to be silently merged into
the previous one and dropped from the result.
@rajanpanth

Copy link
Copy Markdown
Author

Friendly follow-up on this PR 👋

When you have a moment, could you please take a look? I’m happy to update anything if needed.

Thanks!

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 this pull request may close these issues.

1 participant