Compressed, chunk encoded streams are not flushed immediately

[matthiasblaesing]

It was observed, that chunk encoded streams were not correctly passed
through the proxy servlet. Analysis showed, that the stream was gzip
compressed.

The issue is located in the Apache http client/JDK: if the stream from
client is compressed, apache http client will delegate to
GzipInputStream. The #available implementation of InflaterInputStream
(parent of GzipInputStream) returns 1 until EOF is reached.

This is not consistent with InputStream#available, which defines:

A single read or skip of this many bytes will not block, but may
read or skip fewer bytes.

To work around this, the optimization to check for available, a flush
is issued always.

[matthiasblaesing]

I found an alternative - disable handling of decompression in client, then the stream can be directly passed through:

  /**
   * Called from {@link #init(javax.servlet.ServletConfig)}.
   * HttpClient offers many opportunities for customization.
   * In any case, it should be thread-safe.
   */
  protected HttpClient createHttpClient() {
    HttpClientBuilder clientBuilder = getHttpClientBuilder()
                                        .setDefaultRequestConfig(buildRequestConfig())
                                        .setDefaultSocketConfig(buildSocketConfig());
    
    clientBuilder.setMaxConnTotal(maxConnections);
    clientBuilder.setMaxConnPerRoute(maxConnections);
    clientBuilder.disableContentCompression();

    if (useSystemProperties)
      clientBuilder = clientBuilder.useSystemProperties();
    return buildHttpClient(clientBuilder);
  }

This has the drawback, that compression filters running on the webcontainer need respect already compressed data. This sounds reasonable, but ...

[dsmiley]

Passing the stream directly through makes sense to me. Why decompress then re-compress after all?

[matthiasblaesing]

Updated in place. The benefit is, that compression schemes other than gzip and deflate are no supported, if the backend server can handle them.

The potential problem I see: if a user has very simple compression filter active, that encodes the reply regardless of Content-Encoding headers already set, this will break. I checked the Jetty Gzip Handler and that correctly checks an existing Content-Encoding header to see if it should compress the stream coming from the servlet.

[dsmiley]

The potential problem I see: if a user has very simple compression filter active, that encodes the reply regardless of Content-Encoding headers already set, this will break.

I'd argue it would be the fault of the hypothetical filter you speak of. I'm glad Jetty does this right; thanks for investigating.

Let's add a changes entry to mention GZip isn't done any more; the compression (or lack-there-of) is passed right through back to the origin. Users could configure it back if they want it.

[matthiasblaesing]

I updated the change with a configuration parameter to return to the old behavior and a unittest, that verifies sane behavior for the passed on Accept-Encoding header. The change is documented in the CHANGES.md file.

[matthiasblaesing]

And another update - ChunkedTransferTest is modified, to test all combinations of:

• backend supports compression true/false
• compression handling is delegated to apache http client true/false

To complete the image, the work-around for the invalid implementation of InputStream#available in the JDK is also present. This should protect all possible combinations, with minimal impact.

[dsmiley]

+1 LGTM. Fantastic testing! Ready to merge?

[matthiasblaesing]

Yes I think this is ready - I just did an additional manual test with my problematic source and transfers look sane. I pushed a minimal update, that adjusts the name of the unittest for the Accept-Encoding header and does a slight adjustment to the CHANGES.md entry.