jshttp / content-type

Create and parse HTTP Content-Type header
MIT License
131 stars 27 forks source link

FIX: When property value contains forwardslash #12

Closed srolija closed 7 years ago

srolija commented 7 years ago

We encountered edge case when using FormData on iOS inside React Native would generate boundary value that would include forward slashes.

I checked RFC7231, section 3.1.1.1. (Media Type) and there is nothing specified regarding property value format. Since there is obviously case when such property is generated I believe that it should be supported (especially since there are other libraries that depend on this module used for server side parsing).

I added test covering case we encountered. I didn't touch versioning but if you want I can create minor version bump.

dougwilson commented 7 years ago

Hi @srolija , the / character is not a valid character that can exist in a parameter value outside of being inside quotes.

You can read more about the syntax for a Content-Type header in the specification: https://tools.ietf.org/html/rfc7231#section-3.1.1.1

Here is how you read the ABNF specs in the RFC:

The following is the ABF for the contents of the Content-Type header:

     media-type = type "/" subtype *( OWS ";" OWS parameter )
     type       = token
     subtype    = token

So you are talking about what is in the parameter token, which you can follow right in the next section:

     parameter      = token "=" ( token / quoted-string )

You can see that a parameter's value is specified to be either a token or a quoted-string. So you're saying that / should be allowed in an unquoted value, so let's see what token is defined as. You can find it in Appendix D. Collected ABNF:

   token = <token, see [RFC7230], Section 3.2.6>

So this says that the token ABF is defined in RFC 7230, section 3.2.6. Here it is:

     token          = 1*tchar

So, one or more tchar:

     tchar          = "!" / "#" / "$" / "%" / "&" / "'" / "*"
                    / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
                    / DIGIT / ALPHA
                    ; any VCHAR, except delimiters

And as you can see, / is not a valid tchar.

dougwilson commented 7 years ago

P.S. I see you mentioned Reactive Native on iOS. I remember a previous issue about this somewhere, and it was deemed a bug in that library, and I feel like they even fixed it. Let me see if I can dig that up.

dougwilson commented 7 years ago

So this is the commit I'm thinking of https://github.com/facebook/react-native/commit/8ec774396c03f136104f1e916200d1fc359cda0d . If I'm reading their history right, looks like it first made it into Reactive Native v0.41.0

srolija commented 7 years ago

Thank you very much! It was the root cause of the problem!

I'm sorry if I am going off topic now, but I just have question regarding https://github.com/jshttp/type-is , I saw that you were also maintaining it. Reason why I was creating pull request here is because I saw it depended on media-typer which in latest version removed parameter handling rendering multipart queries like this invalid (regardless of forwardslash). So I suposed its implementation would sooner or later replace it with this library. Would you like me to create pull request for required changes?

dougwilson commented 7 years ago

Would you like me to create pull request for required changes?

There is still churn around those modules, so it's hard to say what exactly the change will actually be in type-is at this time.