Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8189789

tomcat gzip-compressed response bodies appear to be broken in update 151

    Details

    • Subcomponent:
    • Understanding:
      Fix Understood
    • Introduced In Version:
    • CPU:
      x86_64
    • OS:
      generic

      Backports

        Description

        FULL PRODUCT VERSION :
        java version "1.7.0_80"
        Java(TM) SE Runtime Environment (build 1.7.0_80-b15)
        Java HotSpot(TM) 64-Bit Server VM (build 24.80-b11, mixed mode)


        ADDITIONAL OS VERSION INFORMATION :
        Windows 7

        A DESCRIPTION OF THE PROBLEM :
        After updating to build 151 of the Windows 1.8 JRE, our users all started reporting issues working with our embedded server - which is running the apache-tomcat-7.0.70 webserver.

        Disabling compression in the tomcat server.xml or reverting to u144 remedies the issue. Specifically:

        java version "1.8.0_144"
        Java(TM) SE Runtime Environment (build 1.8.0_144-b01)
        Java HotSpot(TM) 64-Bit Server VM (build 25.144-b01, mixed mode)

        REGRESSION. Last worked in version 8u144

        ADDITIONAL REGRESSION INFORMATION:
        java version "1.8.0_151"
        Java(TM) SE Runtime Environment (build 1.8.0_151-b12)
        Java HotSpot(TM) 64-Bit Server VM (build 25.151-b12, mixed mode)

        STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
        Install JRE 1.8 b151 on a Windows system running Tomcat with "compression" set to "true" for affected connectors.

        The response header will indicate that the transfer encoding is "gzip", but will report an error uncompressing the data.

        e.g. Chrome returns "ERR_CONTENT_DECODING_FAILED"

        EXPECTED VERSUS ACTUAL BEHAVIOR :
        EXPECTED -
        A valid, decompressed response body
        ACTUAL -
        Error decompressing the response body (e.g. ERR_CONTENT_DECODING_FAILED)

        REPRODUCIBILITY :
        This bug can be reproduced always.

        CUSTOMER SUBMITTED WORKAROUND :
        Disabling compression in Tomcat (set "compression" to "false" in server.xml)


          Issue Links

            Activity

            Hide
            phh Paul Hohensee added a comment -
            We (Amazon) have a reproducer, but unfortunately not yet in a form we can publish. We've seen it using tomcat7 and derivatives such as bobcat.
            Show
            phh Paul Hohensee added a comment - We (Amazon) have a reproducer, but unfortunately not yet in a form we can publish. We've seen it using tomcat7 and derivatives such as bobcat.
            Hide
            sherman Xueming Shen added a comment -
            it's reproducible on tomcat 7.0.82 with the test case included in JDKJDK-8190678. And Sean's 8u151/8u152 binary with the patch below appears to make the problem go away.
            http://cr.openjdk.java.net/~sherman/8189789/webrev
            Show
            sherman Xueming Shen added a comment - it's reproducible on tomcat 7.0.82 with the test case included in JDKJDK-8190678. And Sean's 8u151/8u152 binary with the patch below appears to make the problem go away. http://cr.openjdk.java.net/~sherman/8189789/webrev
            Hide
            sherman Xueming Shen added a comment - - edited
            The tomcat 8.5.23 works fine, the issue is not reproducible. Looked into the tomcat 7.0..82 source code, it appears its gzip handling code has something showed below (to switch the compression level to 0 and flush) to workaround the "flush" issue in our GZIPOutputStream. Tomcat 8 & 9 use the new GZIPOutputStream "flush" flag we introduced in jdk1.7 so they no longer need to switch the compression level. As posted at https://github.com/madler/zlib/issues/305 zlib does have an incompatible change in its deflater code when handling situation someone tries to switch compression level during the compression in its latest releases (since 1.2.9), I attached the details here just for the convenience.

            public class FlushableGZIPOutputStream extends GZIPOutputStream {
             ...
             @Override
                public synchronized void flush() throws IOException {
                    if (hasLastByte) {
                        // - do not allow the gzip header to be flushed on its own
                        // - do not do anything if there is no data to send

                        // trick the deflater to flush
                        /**
                         * Now this is tricky: We force the Deflater to flush its data by
                         * switching compression level. As yet, a perplexingly simple workaround
                         * for
                         * http://developer.java.sun.com/developer/bugParade/bugs/4255743.html
                         */
                        if (!def.finished()) {
                            def.setLevel(Deflater.NO_COMPRESSION);
                            flushLastByte();
                            flagReenableCompression = true;
                        }
                    }
                    out.flush();
                }
            ...
            }

            ------------------------------
            I took a quick looks at all releases and were trying to have a "unified" workaround. Seems like it's "difficult" to workaround for the releases 1.2.3 and earlier, which does not appears to work well with the "strategy-only-change-in-the-middle-of compression", even tried to use "partial-flush" to replace the z_block. I know these are decade-old releases but they are still showing up on various linux distributions :-(

            1.2.3:

            int ZEXPORT deflateParams(strm, level, strategy)

            ...
            if (func != configuration_table[level].func && strm->total_in != 0) {
                /* Flush the last buffer: */
                err = deflate(strm, Z_PARTIAL_FLUSH);
            }
            if (s->level != level) {
            ...
            }

            1.2.4
            int ZEXPORT deflateParams(strm, level, strategy)

            ...

            if ((strategy != s->strategy || func != configuration_table[level].func) &&
                strm->total_in != 0) {
                /* Flush the last buffer: */
                err = deflate(strm, Z_BLOCK);
            }
            if (s->level != level) {
            ...
            1.2.5:

            int ZEXPORT deflateParams(strm, level, strategy) {

            ...

            if ((strategy != s->strategy || func != configuration_table[level].func) &&
                strm->total_in != 0) {
                /* Flush the last buffer: */
                err = deflate(strm, Z_BLOCK);
            }
            if (s->level != level) {
            ...
            }

            1.2.8:

            int ZEXPORT deflateParams(strm, level, strategy) {

            ...

            if ((strategy != s->strategy || func != configuration_table[level].func) &&
                strm->total_in != 0) {
                /* Flush the last buffer: */
                err = deflate(strm, Z_BLOCK);
                if (err == Z_BUF_ERROR && s->pending == 0)
                    err = Z_OK;
            }
            if (s->level != level) {
            ...
            }

            1.2.9/10:

            int ZEXPORT deflateParams(strm, level, strategy) {

            ...
            if ((strategy != s->strategy || func != configuration_table[level].func)) {
                /* Flush the last buffer: */
                int err = deflate(strm, Z_BLOCK);
                if (err == Z_STREAM_ERROR)
                    return err;
                if (strm->avail_out == 0)
                    return Z_BUF_ERROR;
            }
            if (s->level != level) {
            ...
            }

            1.2.11

            int ZEXPORT deflateParams(strm, level, strategy) {
            ....

            if ((strategy != s->strategy || func != configuration_table[level].func) &&
                s->high_water) {
                /* Flush the last buffer: */
                int err = deflate(strm, Z_BLOCK);
                if (err == Z_STREAM_ERROR)
                    return err;
                if (strm->avail_out == 0)
                    return Z_BUF_ERROR;
            }
            if (s->level != level) {
            ...
            }

            1.2.12 (? latest repo)

            int ZEXPORT deflateParams(strm, level, strategy) {
            ....

            if ((strategy != s->strategy || func != configuration_table[level].func) &&
                s->last_flush != -2) {
                /* Flush the last buffer: */
                int err = deflate(strm, Z_BLOCK);
                if (err == Z_STREAM_ERROR)
                    return err;
                if (strm->avail_out == 0)
                    return Z_BUF_ERROR;
            }
            if (s->level != level) {
            ...
            }
            Show
            sherman Xueming Shen added a comment - - edited The tomcat 8.5.23 works fine, the issue is not reproducible. Looked into the tomcat 7.0..82 source code, it appears its gzip handling code has something showed below (to switch the compression level to 0 and flush) to workaround the "flush" issue in our GZIPOutputStream. Tomcat 8 & 9 use the new GZIPOutputStream "flush" flag we introduced in jdk1.7 so they no longer need to switch the compression level. As posted at https://github.com/madler/zlib/issues/305 zlib does have an incompatible change in its deflater code when handling situation someone tries to switch compression level during the compression in its latest releases (since 1.2.9), I attached the details here just for the convenience. public class FlushableGZIPOutputStream extends GZIPOutputStream {  ...  @Override     public synchronized void flush() throws IOException {         if (hasLastByte) {             // - do not allow the gzip header to be flushed on its own             // - do not do anything if there is no data to send             // trick the deflater to flush             /**              * Now this is tricky: We force the Deflater to flush its data by              * switching compression level. As yet, a perplexingly simple workaround              * for              * http://developer.java.sun.com/developer/bugParade/bugs/4255743.html              */             if (!def.finished()) {                 def.setLevel(Deflater.NO_COMPRESSION);                 flushLastByte();                 flagReenableCompression = true;             }         }         out.flush();     } ... } ------------------------------ I took a quick looks at all releases and were trying to have a "unified" workaround. Seems like it's "difficult" to workaround for the releases 1.2.3 and earlier, which does not appears to work well with the "strategy-only-change-in-the-middle-of compression", even tried to use "partial-flush" to replace the z_block. I know these are decade-old releases but they are still showing up on various linux distributions :-( 1.2.3: int ZEXPORT deflateParams(strm, level, strategy) ... if (func != configuration_table[level].func && strm->total_in != 0) {     /* Flush the last buffer: */     err = deflate(strm, Z_PARTIAL_FLUSH); } if (s->level != level) { ... } 1.2.4 int ZEXPORT deflateParams(strm, level, strategy) ... if ((strategy != s->strategy || func != configuration_table[level].func) &&     strm->total_in != 0) {     /* Flush the last buffer: */     err = deflate(strm, Z_BLOCK); } if (s->level != level) { ... 1.2.5: int ZEXPORT deflateParams(strm, level, strategy) { ... if ((strategy != s->strategy || func != configuration_table[level].func) &&     strm->total_in != 0) {     /* Flush the last buffer: */     err = deflate(strm, Z_BLOCK); } if (s->level != level) { ... } 1.2.8: int ZEXPORT deflateParams(strm, level, strategy) { ... if ((strategy != s->strategy || func != configuration_table[level].func) &&     strm->total_in != 0) {     /* Flush the last buffer: */     err = deflate(strm, Z_BLOCK);     if (err == Z_BUF_ERROR && s->pending == 0)         err = Z_OK; } if (s->level != level) { ... } 1.2.9/10: int ZEXPORT deflateParams(strm, level, strategy) { ... if ((strategy != s->strategy || func != configuration_table[level].func)) {     /* Flush the last buffer: */     int err = deflate(strm, Z_BLOCK);     if (err == Z_STREAM_ERROR)         return err;     if (strm->avail_out == 0)         return Z_BUF_ERROR; } if (s->level != level) { ... } 1.2.11 int ZEXPORT deflateParams(strm, level, strategy) { .... if ((strategy != s->strategy || func != configuration_table[level].func) &&     s->high_water) {     /* Flush the last buffer: */     int err = deflate(strm, Z_BLOCK);     if (err == Z_STREAM_ERROR)         return err;     if (strm->avail_out == 0)         return Z_BUF_ERROR; } if (s->level != level) { ... } 1.2.12 (? latest repo) int ZEXPORT deflateParams(strm, level, strategy) { .... if ((strategy != s->strategy || func != configuration_table[level].func) &&     s->last_flush != -2) {     /* Flush the last buffer: */     int err = deflate(strm, Z_BLOCK);     if (err == Z_STREAM_ERROR)         return err;     if (strm->avail_out == 0)         return Z_BUF_ERROR; } if (s->level != level) { ... }
            Hide
            phh Paul Hohensee added a comment - - edited
            I've attached a reproducer.
            Does the CPU18_01-defer-approved tag mean that this fix will not be in the January release?
            Show
            phh Paul Hohensee added a comment - - edited I've attached a reproducer. Does the CPU18_01-defer-approved tag mean that this fix will not be in the January release?
            Hide
            coffeys Sean Coffey added a comment -
            Thanks for test pointers [~phh]

            Code review started at http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-November/050118.html
            Show
            coffeys Sean Coffey added a comment - Thanks for test pointers [~phh] Code review started at http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-November/050118.html

              People

              • Assignee:
                coffeys Sean Coffey
                Reporter:
                webbuggrp Webbug Group
              • Votes:
                1 Vote for this issue
                Watchers:
                9 Start watching this issue

                Dates

                • Created:
                  Updated: