2012/3/23 Mark Thomas <ma...@apache.org>:
> On 23/03/2012 01:41, Konstantin Kolinko wrote:
>> 2012/3/13 <ma...@apache.org>:
>>> Author: markt
>>> Date: Tue Mar 13 14:39:24 2012
>>> New Revision: 1300154
>
>> 1) It seems that quoted text handling ("<IN_QUOTED_TEXT> TOKEN"
>> construct) is slightly wrong in its handling of backslash character.
>
> Agreed. Fixed.
>
OK
>> 2) RFC2616: ch.3.7 Media Types
>
>> "Linear white space
>> (LWS) MUST NOT be used between the type and subtype, nor between an
>> attribute and its value."
>
>> I am OK with it though, because I do not see any problem from it, but
>> maybe LWS could be removed from TOKEN and QUOTED_STRING constructs
>> along with those trim() calls.
>
> Keeping the code as is should make re-use of the parser for other
> headers easier. As long as the LWS does not cause an issue, I'm OK with
> the current approach. After all, servers are meant to be tolerant where
> they can be.
>
ACK
>> 3)
>> I do not see where we allow "charset" parameter name to be
>> case-insensitive. We allow lowercase only.
>
> Agreed. Fixed.
>
toLowerCase()? We are passing Locale.ENGLISH to such calls elsewhere.
I think that String.equalsIgnoreCase() would be faster (no additional
object creation and stops on first mismatch)
It does not matter for charset though.
>> 4) The "value" of a parameter can be quoted-string. We do not handle
>> unquoting of charset parameter value. I am OK with leaving as is,
>> because nobody asked.
>
> We do handle this. jjtGetValue() will unquote the value if it is quoted.
> toString() will return the original value.
>
OK, I see - in AstValue.java
Unquoting is actually not as trivial as just replacing \" with ", but
for our use case (charset value) it is OK.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
