RFR JDK-8191918: tomcat gzip-compressed response bodies appear to be broken in update 151

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR JDK-8191918: tomcat gzip-compressed response bodies appear to be broken in update 151

Xueming Shen
Hi,

Please help review the change for  JDK-8191918:

issue: https://bugs.openjdk.java.net/browse/JDK-8191918
webrev: http://cr.openjdk.java.net/~sherman/8191918/webrev

This is the backport/identical change we have already putback into
earlier update
releases for JDK-8189789. It includes two changes

(1) to replace the change we put into jdk9 for #8184306 with the "official"
change currently in zlib github repo
(https://github.com/madler/zlib/issues/275)

http://cr.openjdk.java.net/~sherman/8184306
https://bugs.openjdk.java.net/browse/JDK-8184306

(2) to change the way we handle the returned result Z_BUF_ERROR. Now the
implementation expects that there might be bytes read from the input and
bytes
written to the output buffer when the deflateParams()/deflate() returns
Z_BUF_ERROR. It is "interpreted as there is no enough buffer space for
all output,
but the deflater has decoded input bytes and write out output bytes as
many as
possible, come back with more available output space".

See related github/zlib discussions at #275 [1] and #305[2]

Thanks,
Sherman

[1] https://github.com/madler/zlib/issues/275
[2] https://github.com/madler/zlib/issues/305
Reply | Threaded
Open this post in threaded view
|

Re: RFR JDK-8191918: tomcat gzip-compressed response bodies appear to be broken in update 151

Paul Sandoz


> On 30 Nov 2017, at 14:46, Xueming Shen <[hidden email]> wrote:
>
> Hi,
>
> Please help review the change for  JDK-8191918:
>
> issue: https://bugs.openjdk.java.net/browse/JDK-8191918
> webrev: http://cr.openjdk.java.net/~sherman/8191918/webrev
>

InflateIn_DeflateOut.java


 174         GZIPInputStream gzis = new GZIPInputStream(byteInStream);
 175         ByteArrayOutputStream baos = new ByteArrayOutputStream();
 176         int numRead;
 177         byte[] b = new byte[4 * 1024];
 178         try {
 179             while ((numRead = gzis.read(buf)) >= 0) {
 180                 baos.write(b, 0, numRead);
 181             }
 182         } finally {
 183             baos.close();
 184         }

Can you use gzis.transferTo(baos)?

Paul.

> This is the backport/identical change we have already putback into earlier update
> releases for JDK-8189789. It includes two changes
>
> (1) to replace the change we put into jdk9 for #8184306 with the "official"
> change currently in zlib github repo (https://github.com/madler/zlib/issues/275)
>
> http://cr.openjdk.java.net/~sherman/8184306
> https://bugs.openjdk.java.net/browse/JDK-8184306
>
> (2) to change the way we handle the returned result Z_BUF_ERROR. Now the
> implementation expects that there might be bytes read from the input and bytes
> written to the output buffer when the deflateParams()/deflate() returns
> Z_BUF_ERROR. It is "interpreted as there is no enough buffer space for all output,
> but the deflater has decoded input bytes and write out output bytes as many as
> possible, come back with more available output space".
>
> See related github/zlib discussions at #275 [1] and #305[2]
>
> Thanks,
> Sherman
>
> [1] https://github.com/madler/zlib/issues/275
> [2] https://github.com/madler/zlib/issues/305

Reply | Threaded
Open this post in threaded view
|

Re: RFR JDK-8191918: tomcat gzip-compressed response bodies appear to be broken in update 151

Xueming Shen
On 12/1/17, 3:40 PM, Paul Sandoz wrote:

>
>> On 30 Nov 2017, at 14:46, Xueming Shen<[hidden email]>  wrote:
>>
>> Hi,
>>
>> Please help review the change for  JDK-8191918:
>>
>> issue: https://bugs.openjdk.java.net/browse/JDK-8191918
>> webrev: http://cr.openjdk.java.net/~sherman/8191918/webrev
>>
> InflateIn_DeflateOut.java
> —
>
>   174         GZIPInputStream gzis = new GZIPInputStream(byteInStream);
>   175         ByteArrayOutputStream baos = new ByteArrayOutputStream();
>   176         int numRead;
>   177         byte[] b = new byte[4 * 1024];
>   178         try {
>   179             while ((numRead = gzis.read(buf))>= 0) {
>   180                 baos.write(b, 0, numRead);
>   181             }
>   182         } finally {
>   183             baos.close();
>   184         }
>
> Can you use gzis.transferTo(baos)?
>
> Paul.
>
>

Thanks Paul!

webrev has been updated as suggested.

http://cr.openjdk.java.net/~sherman/8191918/webrev

-Sherman
Reply | Threaded
Open this post in threaded view
|

Re: RFR JDK-8191918: tomcat gzip-compressed response bodies appear to be broken in update 151

Paul Sandoz
+1

This is a good example of where “var” can really reduce the verbosity, up to you.

Also ByteArrayOutputStream.close is a no-op, you don’t need the finally block.

Paul.

> On 1 Dec 2017, at 17:04, Xueming Shen <[hidden email]> wrote:
>
> On 12/1/17, 3:40 PM, Paul Sandoz wrote:
>>
>>> On 30 Nov 2017, at 14:46, Xueming Shen<[hidden email]>  wrote:
>>>
>>> Hi,
>>>
>>> Please help review the change for  JDK-8191918:
>>>
>>> issue: https://bugs.openjdk.java.net/browse/JDK-8191918
>>> webrev: http://cr.openjdk.java.net/~sherman/8191918/webrev
>>>
>> InflateIn_DeflateOut.java
>> —
>>
>>  174         GZIPInputStream gzis = new GZIPInputStream(byteInStream);
>>  175         ByteArrayOutputStream baos = new ByteArrayOutputStream();
>>  176         int numRead;
>>  177         byte[] b = new byte[4 * 1024];
>>  178         try {
>>  179             while ((numRead = gzis.read(buf))>= 0) {
>>  180                 baos.write(b, 0, numRead);
>>  181             }
>>  182         } finally {
>>  183             baos.close();
>>  184         }
>>
>> Can you use gzis.transferTo(baos)?
>>
>> Paul.
>>
>>
>
> Thanks Paul!
>
> webrev has been updated as suggested.
>
> http://cr.openjdk.java.net/~sherman/8191918/webrev
>
> -Sherman

Reply | Threaded
Open this post in threaded view
|

Re: RFR JDK-8191918: tomcat gzip-compressed response bodies appear to be broken in update 151

Seán Coffey
In reply to this post by Xueming Shen
Looks good to me.

Regards,
Sean.

On 02/12/17 01:04, Xueming Shen wrote:

> On 12/1/17, 3:40 PM, Paul Sandoz wrote:
>>
>>> On 30 Nov 2017, at 14:46, Xueming Shen<[hidden email]>  wrote:
>>>
>>> Hi,
>>>
>>> Please help review the change for  JDK-8191918:
>>>
>>> issue: https://bugs.openjdk.java.net/browse/JDK-8191918
>>> webrev: http://cr.openjdk.java.net/~sherman/8191918/webrev
>>>
>> InflateIn_DeflateOut.java
>> —
>>
>>   174         GZIPInputStream gzis = new GZIPInputStream(byteInStream);
>>   175         ByteArrayOutputStream baos = new ByteArrayOutputStream();
>>   176         int numRead;
>>   177         byte[] b = new byte[4 * 1024];
>>   178         try {
>>   179             while ((numRead = gzis.read(buf))>= 0) {
>>   180                 baos.write(b, 0, numRead);
>>   181             }
>>   182         } finally {
>>   183             baos.close();
>>   184         }
>>
>> Can you use gzis.transferTo(baos)?
>>
>> Paul.
>>
>>
>
> Thanks Paul!
>
> webrev has been updated as suggested.
>
> http://cr.openjdk.java.net/~sherman/8191918/webrev
>
> -Sherman