Uploaded image for project: 'HttpComponents HttpClient'
  1. HttpComponents HttpClient
  2. HTTPCLIENT-2005

ContentCompressionExec is not thread-safe

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 5.0 Beta5
    • Fix Version/s: 5.0 Beta6
    • Component/s: HttpClient (classic)
    • Labels:
      None

      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); 
      

       

       

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              linton.miller Linton Miller
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: