On 14/01/2016 13:07, Konstantin Kolinko wrote:
> 2016-01-14 11:43 GMT+03:00 Mark Thomas <[email protected]>:
>> On 14/01/2016 03:08, Konstantin Kolinko wrote:
>>> 2016-01-13 18:02 GMT+03:00 <[email protected]>:
>>>> Author: markt
>>>> Date: Wed Jan 13 15:02:00 2016
>>>> New Revision: 1724437
>>
>>> Looking into Parameters.handleQueryParameters() on how it deals with
>>> MessageBytes that wraps a String. It calls "data.toBytes()" which
>>> uses System Default encoding (see MessageBytes.toBytes() in Tomcat 6
>>> and Tomcat 7, Tomcat 8 onward is different). The
>>> RequestUtil.parseParameters() method called by the old code uses the
>>> specified encoding.
>>
>> Nice catch. Fixed.
>
> The hard part (backporting MessageBytes charset support) - done.
>
> The easy part - setting the charset on MessageBytes - not yet.
>
> Setting charset on MessageBytes queryMB won't help. In
> paramParser.handleQueryParameters(); it is copied and that does not
> inherit the charset:
I've fixed that. Looking at where MessageBytes.duplicate() is used,
mostly it won't matter but there are a few places - mainly parameter
processing - where it could have an impact so fixing the issue at source
seemed like the best option.
>> decodedQuery.duplicate( queryMB );
>
>
> Also the following block:
> + String dispParamName = dispParamNames.nextElement();
> + String[] dispParamValues =
> paramParser.getParameterValues(dispParamName);
> + String[] originalValues = queryParameters.get(dispParamName);
> + if (originalValues == null) {
> + queryParameters.put(dispParamName, dispParamValues);
> continue;
> }
> - queryParameters.put
> - (key, mergeValues(value, parameters.get(key)));
> + queryParameters.put(dispParamName,
> mergeValues(dispParamValues, originalValues));
>
> The mergeValues() method arguments are (Object, Object), but here the
> both arguments are String[]. I think that the mergeValues() method can
> be simplified in Tomcat 9.
I'll take a look.
> The "parameters" field in Tomcat 6 is returned by
> ApplicationHttpRequest.getParameterMap() and Javadoc for that method
> in ServletRequest says "The values in the parameter map are of type
> String array." so I think that we are OK to assume that the original
> values are String[]. It would be safer though to define
> queryParameters as Map<String, Object> in Tomcat 6.
Given that is all internal code and a specification requirement to
return String[] I'm happy leaving it as is for now.
Thanks again for the review.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
