[
https://issues.apache.org/jira/browse/HTTPCLIENT-2005?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16894401#comment-16894401
]
Oleg Kalnichevski edited comment on HTTPCLIENT-2005 at 7/27/19 10:40 AM:
-------------------------------------------------------------------------
[~linton.miller] I must admit I have a hard time believing
{{Arrays#sort(Object[])}} not being thread safe. It is a static method without
any shared state I could spot in the implementation code. What is your
assumption based upon?
But what is absolutely wrong is that {{MessageSupport#formatTokens}} meddles
with input arguments that it ought to treat as immutable. There is actually no
reason of what so ever {{MessageSupport#formatTokens}} should be sorting the
tokens. I am going to push a fix to HttpCore master if I hear no objections.
https://github.com/apache/httpcomponents-core/commit/13c4291372f8a22eaa468a36cc4a5a79eda0a567
Oleg
was (Author: olegk):
[~linton.miller] I must admit I have a hard time believing
{{Arrays#sort(Object[])}} not being thread safe. It is a static method without
any shared state I could spot in the implementation code. What is your
assumption based upon?
But what is absolutely wrong is that {{MessageSupport#formatTokens}} meddles
with input arguments that it ought to treat as immutable. There is actually no
reason of what so ever {{MessageSupport#formatTokens}} should be sorting the
tokens. I am going to push a fix to HttpCore master if I hear no objections.
https://github.com/apache/httpcomponents-core/commit/13c4291372f8a22eaa468a36cc4a5a79eda0a567
Oleg
Oleg
> ContentCompressionExec is not thread-safe
> -----------------------------------------
>
> Key: HTTPCLIENT-2005
> URL: https://issues.apache.org/jira/browse/HTTPCLIENT-2005
> Project: HttpComponents HttpClient
> Issue Type: Bug
> Components: HttpClient (classic)
> Affects Versions: 5.0 Beta5
> Reporter: Linton Miller
> Priority: Minor
>
> org.apache.hc.client5.http.impl.classic.ContentCompressionExec is not
> thread-safe in its handling of the Accept-Encoding header, which can result
> in the header being mangled.
> The problem is in its creation of the header by using MessageSupport:
> {code:java}
> request.addHeader(MessageSupport.format(HttpHeaders.ACCEPT_ENCODING,
> acceptEncoding));
> {code}
> MessageSupport.format is *not* thread-safe, because it sorts its second
> argument:
> {code:java}
> public static Header format(final String name, final String... tokens) {
> ...
> formatTokens(buffer, tokens);
> public static void formatTokens(final CharArrayBuffer dst, final String...
> tokens) {
> Args.notNull(dst, "Destination");
> Arrays.sort(tokens); // <--- This is not thread-safe!!!
> {code}
> The result is that the Accept-Encoding header can end up being any random
> combination and subset of the acceptEncoding array elements.
> e.g. on the standard acceptEncoding array of new String[] \{"gzip", "x-gzip",
> "deflate"}, the request header may read
> Accept-Encoding: deflate, deflate, gzip
> or
> Accept-Encoding: deflate, deflate, deflate
> for example.
> It won't, however, introduce any elements not previously in the array, or
> corrupt any of the individual strings.
> It actually seems pretty nasty that MessageSupport isn't thread-safe, but
> given that it's not, it would require supplying a separate copy of the
> acceptEncoding array for each format call. e.g.
>
> {code:java}
> request.addHeader(MessageSupport.format(HttpHeaders.ACCEPT_ENCODING,
> acceptEncoding.clone()));
> {code}
> However, an even better alternative would seem to be to do the MessageFormat
> in the ContentCompressionExec constructor, and cache the formatted Header:
> {code:java}
> this.acceptEncoding =
> MessageSupport.format(HttpHeaders.ACCEPT_ENCODING,
> acceptEncoding != null ? acceptEncoding.toArray(
> new String[acceptEncoding.size()]) : new String[] {"gzip",
> "x-gzip", "deflate"});
> {code}
> and then just use it in the execute method:
>
> {code:java}
> request.addHeader(acceptEncoding);
> {code}
>
>
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
