[HTTPCLIENT-2005] ContentCompressionExec is not thread-safe

• Type: Bug
• Status: Resolved
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 5.0 Beta5
• Fix Version/s: 5.0 Beta6
• Component/s: HttpClient (classic)
• Assignee: Unassigned
• Reporter: Linton Miller
• Created: 27/Jul/19 05:38
• Updated: 27/Jul/19 21:46
• Resolved: 27/Jul/19 21:46

Description

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:

request.addHeader(MessageSupport.format(HttpHeaders.ACCEPT_ENCODING, acceptEncoding));

MessageSupport.format is not thread-safe, because it sorts its second argument:

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!!!

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.

 

  request.addHeader(MessageSupport.format(HttpHeaders.ACCEPT_ENCODING, acceptEncoding.clone()));

However, an even better alternative would seem to be to do the MessageFormat in the ContentCompressionExec constructor, and cache the formatted Header:

        this.acceptEncoding = MessageSupport.format(HttpHeaders.ACCEPT_ENCODING,
            acceptEncoding != null ? acceptEncoding.toArray(
                new String[acceptEncoding.size()]) : new String[] {"gzip", "x-gzip", "deflate"});

and then just use it in the execute method:

 

      request.addHeader(acceptEncoding);