[jira] [Resolved] (HTTPCLIENT-2005) ContentCompressionExec is not thread-safe

["Oleg Kalnichevski (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/HTTPCLIENT-2005?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski resolved HTTPCLIENT-2005.
-------------------------------------------
       Resolution: Fixed
    Fix Version/s: 5.0 Beta6

Merged to master.

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
>             Fix For: 5.0 Beta6
>
>
> 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org