From: Kyle J. McKay <mackyle@gmail.com>
Date: Thu, 22 May 2014 07:27:38 GMT
Subject: Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter
Message-ID: <21F11C1A-4C69-4633-9DC7-E9134D38856E@gmail.com>
In-Reply-To: <20140522060516.GB16587@sigill.intra.peff.net>

On May 21, 2014, at 23:05, Jeff King wrote:

> On Wed, May 21, 2014 at 05:07:38PM -0700, Kyle J. McKay wrote:
>
>>> +	p = skip_prefix(type->buf, "text/plain");
>>> +	if (!p || (*p && *p != ';'))
>>> +		return 0;
>>> +
>>> +	return 1;
>>> +}
>>> +
>>
>> I think that a strict reading of RFC 2616 allows "text/plain ;
>> charset=utf-8" as well as "text/plain;charset=utf-8" and "text/plain;
>> charset=utf-8".  So perhaps this if line instead:
>>
>> +	if (!p || (*p && *p != ';' && *p != ' ' && *p != '\t'))
>>
>> See RFC 2616 section 2.2 for the definition of linear white space  
>> (LWS) and
>> the end of section 2.1 for the "implied *LWS" rule that allows it.
>
> You're right. There are a few exceptions in 3.7:
>
>  Linear white space (LWS) MUST NOT be used between the type and
>  subtype, nor between an attribute and its value.
>
> but it does not include between the subtype and parameter. So the
> "find_parameter" also needs to accept the collapsed whitespace, too, I
> guess.

Yeah I think so too.  It's probably enough though just to just strip  
all " " and "\t" characters at the same time the content type is  
lowercased.  While that would cause invalid content types such as  
"text / plain" to be recognized it would keep the rest of the code  
simpler.  Since a producer of an invalid content type shouldn't really  
be depending on any particular behavior by a receiver of an invalid  
content type it's probably not an issue.

--Kyle