RFR: 8189789: tomcat gzip-compressed response bodies appear to be broken in update 151
Looking to fix a recent regression that appeared in 8u151. Thanks goes to Sherman for the src patch. Paul Hohensee has also helped by provided some test code. I've been able to modify an existing test to mimic the behaviour seen with FlushableGZIPOutputStream which is found in Tomcat 7.x. Sherman is working on a patch for JDK 10. For unix OSes, JDK 10 operates with the zlib version found on the OS. bug : https://bugs.openjdk.java.net/browse/JDK-8189789 webrev : http://cr.openjdk.java.net/~coffeys/webrev.8189789.jdk8u/webrev/ regards, Sean.
If the fix is from the zlib project, then ignore the following, since it’s their code. In Deflater.c: the existing significant code duplication between the arms of the if-else gives one pause. E.g., in the new file, compare lines 148/183 and 155/190. Might be bugs lurking at 183 and 190. In deflate.c: the old code had “s->last_flush = Z_NO_FLUSH”, the new has “-2”. Should “-2” be symbolic instead? Thanks, Paul On 11/22/17, 2:00 AM, "core-libs-dev on behalf of Seán Coffey" <core-libs-dev-bounces@openjdk.java.net on behalf of sean.coffey@oracle.com> wrote: Looking to fix a recent regression that appeared in 8u151. Thanks goes to Sherman for the src patch. Paul Hohensee has also helped by provided some test code. I've been able to modify an existing test to mimic the behaviour seen with FlushableGZIPOutputStream which is found in Tomcat 7.x. Sherman is working on a patch for JDK 10. For unix OSes, JDK 10 operates with the zlib version found on the OS. bug : https://bugs.openjdk.java.net/browse/JDK-8189789 webrev : http://cr.openjdk.java.net/~coffeys/webrev.8189789.jdk8u/webrev/ regards, Sean.
On 22/11/2017 15:46, Hohensee, Paul wrote:
If the fix is from the zlib project, then ignore the following, since it’s their code.
In Deflater.c: the existing significant code duplication between the arms of the if-else gives one pause. E.g., in the new file, compare lines 148/183 and 155/190. Might be bugs lurking at 183 and 190. There might be some room for optimizing alright. The different length variables are measuring the available input and output length values. Different action is taken if we encounter exceptions when setting up memory for related operations. Maybe we can bring more consistent length checks to both blocks. I'd have to check why we check for native exception handling in one block and not the other.
In deflate.c: the old code had “s->last_flush = Z_NO_FLUSH”, the new has “-2”. Should “-2” be symbolic instead? Sherman is following the code from the main zlib repo for that change [1]
regards, Sean. [1] https://github.com/madler/zlib/issues/275
Thanks,
Paul
On 11/22/17, 2:00 AM, "core-libs-dev on behalf of Seán Coffey" <core-libs-dev-bounces@openjdk.java.net on behalf of sean.coffey@oracle.com> wrote:
Looking to fix a recent regression that appeared in 8u151. Thanks goes to Sherman for the src patch. Paul Hohensee has also helped by provided some test code. I've been able to modify an existing test to mimic the behaviour seen with FlushableGZIPOutputStream which is found in Tomcat 7.x.
Sherman is working on a patch for JDK 10. For unix OSes, JDK 10 operates with the zlib version found on the OS.
bug : https://bugs.openjdk.java.net/browse/JDK-8189789 webrev : http://cr.openjdk.java.net/~coffeys/webrev.8189789.jdk8u/webrev/
regards, Sean.
I’d file an RFE to refactor the Deflater.c code. Following the zlib code is the right thing to do in order to stay consistent, even if it’s a bit rough. So, ship it. On 11/22/17, 9:43 AM, "Seán Coffey" <sean.coffey@oracle.com> wrote: On 22/11/2017 15:46, Hohensee, Paul wrote: > If the fix is from the zlib project, then ignore the following, since it’s their code. > > In Deflater.c: the existing significant code duplication between the arms of the if-else gives one pause. E.g., in the new file, compare lines 148/183 and 155/190. Might be bugs lurking at 183 and 190. There might be some room for optimizing alright. The different length variables are measuring the available input and output length values. Different action is taken if we encounter exceptions when setting up memory for related operations. Maybe we can bring more consistent length checks to both blocks. I'd have to check why we check for native exception handling in one block and not the other. > > In deflate.c: the old code had “s->last_flush = Z_NO_FLUSH”, the new has “-2”. Should “-2” be symbolic instead? Sherman is following the code from the main zlib repo for that change [1] regards, Sean. [1] https://github.com/madler/zlib/issues/275 > > Thanks, > > Paul > > On 11/22/17, 2:00 AM, "core-libs-dev on behalf of Seán Coffey" <core-libs-dev-bounces@openjdk.java.net on behalf of sean.coffey@oracle.com> wrote: > > Looking to fix a recent regression that appeared in 8u151. Thanks goes > to Sherman for the src patch. Paul Hohensee has also helped by provided > some test code. I've been able to modify an existing test to mimic the > behaviour seen with FlushableGZIPOutputStream which is found in Tomcat 7.x. > > Sherman is working on a patch for JDK 10. For unix OSes, JDK 10 operates > with the zlib version found on the OS. > > bug : https://bugs.openjdk.java.net/browse/JDK-8189789 > webrev : http://cr.openjdk.java.net/~coffeys/webrev.8189789.jdk8u/webrev/ > > regards, > Sean. > > >
+1 On 11/22/17, 1:59 AM, Seán Coffey wrote:
Looking to fix a recent regression that appeared in 8u151. Thanks goes to Sherman for the src patch. Paul Hohensee has also helped by provided some test code. I've been able to modify an existing test to mimic the behaviour seen with FlushableGZIPOutputStream which is found in Tomcat 7.x.
Sherman is working on a patch for JDK 10. For unix OSes, JDK 10 operates with the zlib version found on the OS.
bug : https://bugs.openjdk.java.net/browse/JDK-8189789 webrev : http://cr.openjdk.java.net/~coffeys/webrev.8189789.jdk8u/webrev/
regards, Sean.
participants (3)
-
Hohensee, Paul
-
Seán Coffey
-
Xueming Shen