search

jshttp / negotiator

An HTTP content negotiator for Node.js
MIT License
320 stars 36 forks source link

Parameter value case-sensitivity is media-type specific #38

Open ethanresnick opened 9 years ago

ethanresnick commented 9 years ago

I'm not sure what to do about this one...

Right now, it looks like this library is always assuming that parameter values are case-insensitive. This is technically incorrect, as RFC 7231 says that "Parameter values might or might not be case-sensitive, depending on the semantics of the parameter name." And, in fact, there are some media types that demand case sensitivity (for example, security-related types like "application/cms").

That said, being fully spec compliant here would require a ton of work (reading every registered media type's specification?) so it probably isn't worth it. Maybe the right answer is to continue to assume case-insensitivity and then keep a list of media type parameters whose values are case-sensitive, and add to that list only as real bugs arise?

dougwilson commented 9 years ago

Hm, not sure. I believe the values used to be case-sensitive, but it seems the majority of the time they should have been case-insensitive, so it was swapped. I would rather play it by ear and wait for someone who has a legitimate want for them to be case-sensitive, otherwise re-evaluate before 1.0.

dougwilson commented 9 years ago

Reading this and read #35 it's basically sounding like doing any value matching on media type parameters is bound to be type-specific and cannot be done in a generic way. Does this sound right to you?

ethanresnick commented 9 years ago

I'm not sure. My hope is that we can do at least a bit better than that. See my comment on #35

jasnell commented 9 years ago

One particular case where case is significant is the profile attribute (see http://www.w3.org/TR/json-ld/#iana-considerations). The value of the profile attribute is URI. Comparison must always be done in a case-sensitive way (e.g. profile="http://example.org/#foo" is different than profile="http://example.org/#Foo")

dougwilson commented 9 years ago

I was just looking through the code to see when the value comparison was made case-insensitive, and it looks like it sneaked in in a4fafd8d21629b98eb4b0c9d21b4830ed3e25b6a

I did a little digging, as if we were to pick case-sensitive vs. case-insensitive for media type parameter value matching, it seems like choosing case-sensitive is likely the most widely-applicable, with a big notable exception being the very common charset parameter, though it's odd to negotiate on this (as Accept-Charset exists for this, in general).

I really want to address this with a refactor that would make the point moot with a way to configure. Since there is no way to configure currently, it's very difficult to do a case-sensitive match.