Fails to parse quoted parameters

[ethanresnick]

According to the spec, the value for a media type parameter can be a quoted string, but parsing Accept headers with quoted parameter values doesn't work as expected:

1. Specifically, using the internal parseMediaType function, calling parseMediaType('application/mytype+json;test="bob"') returns a parameters hash that looks like this {"test": '"bob"'}, where "bob" is quoted, even though it shouldn't be. (The spec says that the including the quotes or not in the Accept header shouldn't have an effect: "The quoted and unquoted values are equivalent.")
2. The bigger problem, though, comes when the quoted value contains a comma (which is valid under the definition of qdtext). Consider: parseAccept('application/x+json;test="bob,jill",*/*'). Currently, parseAccept function naively splits this at the commas, which leads it to try to parse it as three media types: 'application/x+json;test="bob', 'jill", and '*/*'. The second string is an invalid type, so it gets ignored, so the parsed value for the test parameter ends up as '"bob', with an extra left quote, instead of 'bob, jill'.

[dougwilson]

PR welcome :)! Otherwise I'll look into getting this fixed in the next couple weeks without one.

[ethanresnick]

Cool! Would you accept a PEG parser written to the RFC? I already wrote that for my own project, since I wasn't confident that I could handle all the edge cases with a regexp, and I'd be happy to contribute it.

[dougwilson]

Everything has to run in Node.js 0.6, and last I looked at PEG.js that was not the case.

[dougwilson]

You can always build off the RegExp s in the content-type module if you like, if the PEG generator doesn't work in the old Node.js versions.

[ethanresnick]

Ahh compatibility woes :(

I'll try to whip up a PR then if I have the time, but I'm not sure I'll be able to get around to it very soon.

[dougwilson]

Feel free to check out my abandoned PEG.js file for this, though :) https://github.com/jshttp/http-utils/blob/peg/src/rfc7231.pegjs

[dougwilson]

Ok, sorry I've been absent on this issue at have been letting it sit. The biggest barrier to change was the test suite no longer functioned on Node.js 0.6. The last few days I worked on converting it to mocha so it'll run :) I've fixed the second issue so far.

[dougwilson]

Alrighty, both your issues should be fixed on master now. If you have time, I encourage you to test the changes (npm install jshttp/negotiator) to confirm/deny if your issue has been fixed :)!

[dougwilson]

Published to npm as 0.5.2

[ethanresnick]

Thanks @dougwilson! I'll let you know if I encounter any problems!