Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Wed, 3 Jun 2026 13:43:03 GMT, Jaikiran Pai wrote: >>> @jaikiran, is my understanding correct that by "reintroduce the change that >>> was done for JDK-7036144" you implicitly also mean to reintroduce the fix >>> [JDK-8374644](https://bugs.openjdk.org/browse/JDK-8374644) for the >>> performance regression >> >> Correct. > >> Expand the test coverage. It's always risky to change something in code from >> JDK 1.0/1.1 so having tests that capture the current behavior (even if >> unspecified) is important before making any change. > > A new test has been proposed https://bugs.openjdk.org/browse/JDK-8385891 to > help increase the coverage. Over time, more will be added. > Jai, Lance and I met today to go over the issues with GZIPInputStream. Here's > a summary of where we got to: > >For JDK 28 or later, introduce a new constructor to create a > GZIPInputStream that works like it should have done in JDK 1.1 when the API > was originally introduced. We discussed two possible sets of parameters. We > also discussed maybe deprecating the two existing constructors due to the > issue of treating available==0 as EOF and the issue of swallowing exceptions. I've filed https://bugs.openjdk.org/browse/JDK-8388919 to track this. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3643464958
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Fri, 29 May 2026 10:39:49 GMT, Jaikiran Pai wrote: >>> How do you plan to solve point "3. For JDK 27, introduce a system >>> property..."? Do you want to do this with the initial fix for >>> [JDK-7036144](https://bugs.openjdk.org/browse/JDK-7036144), by wrapping the >>> input stream and overriding available() or in a completely different way? >> >> The plan is to reintroduce the change that was done for JDK-7036144 and >> enable that new behaviour only when the new system property is enabled. When >> that system property isn't set (or disabled), which would be the default >> case, then the implementation will continue with the long standing behaviour >> of using `InputStream.available()`. > >> @jaikiran, is my understanding correct that by "reintroduce the change that >> was done for JDK-7036144" you implicitly also mean to reintroduce the fix >> [JDK-8374644](https://bugs.openjdk.org/browse/JDK-8374644) for the >> performance regression > > Correct. > Expand the test coverage. It's always risky to change something in code from > JDK 1.0/1.1 so having tests that capture the current behavior (even if > unspecified) is important before making any change. A new test has been proposed https://bugs.openjdk.org/browse/JDK-8385891 to help increase the coverage. Over time, more will be added. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3348948353
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Fri, 29 May 2026 06:36:35 GMT, Jaikiran Pai wrote: >> Thanks you all for your support! This sounds like a reasonable plan to me. >> >> How do you plan to solve point "3. For JDK 27, introduce a system >> property..."? Do you want to do this with the initial fix for >> [JDK-7036144](https://bugs.openjdk.org/browse/JDK-7036144), by wrapping the >> input stream and overriding available() or in a completely different way? >> >> It's also just a week away from Rampdown Phase One for JDK 27, so we have to >> hurry up. > >> How do you plan to solve point "3. For JDK 27, introduce a system >> property..."? Do you want to do this with the initial fix for >> [JDK-7036144](https://bugs.openjdk.org/browse/JDK-7036144), by wrapping the >> input stream and overriding available() or in a completely different way? > > The plan is to reintroduce the change that was done for JDK-7036144 and > enable that new behaviour only when the new system property is enabled. When > that system property isn't set (or disabled), which would be the default > case, then the implementation will continue with the long standing behaviour > of using `InputStream.available()`. > @jaikiran, is my understanding correct that by "reintroduce the change that > was done for JDK-7036144" you implicitly also mean to reintroduce the fix > [JDK-8374644](https://bugs.openjdk.org/browse/JDK-8374644) for the > performance regression Correct. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3323772824
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Fri, 29 May 2026 09:33:58 GMT, Volker Simonis wrote: >>> See the notes above about next steps which will propose a new constructor >>> and maybe deprecate the existing constructors. >> >> Yes, sure, I'm fine with those >> >> My concern was more about any system property and that they should be simple >> switches which picks the default mode. They should not hold independent >> semantics which needs to be documented in other terms than the modes >> documented in the API. Any system property introduced now should account for >> the modes/constructors we want to introduce later. They will co-exist for a >> long time. >> >> Maybe this is just a given and my comment here is superfluous. My concern >> was usability and future users having to understand two different ways of >> controlling behavior and resolve any conflicts between them. >> >> I think I'm happy if we avoid introducing a system propery now which later >> proves difficult to define in terms of behavior specified by the new >> constructors / APIs . > > My understanding is that we only need the system property for JDK 27 and > earlier releases. Once we come up with a real fix which will probably contain > API changes which can't be backported, the system property wouldn't be > required any more and deprecated/removed as well. > The plan is to reintroduce the change that was done for JDK-7036144 and > enable that new behaviour only when the new system property is enabled. When > that system property isn't set (or disabled), which would be the default > case, then the implementation will continue with the long standing behaviour > of using InputStream.available(). @jaikiran, is my understanding correct that by "reintroduce the change that was done for JDK-7036144" you implicitly also mean to reintroduce the fix [JDK-8374644](https://bugs.openjdk.org/browse/JDK-8374644) for the performance regression introduced by [JDK-7036144](https://bugs.openjdk.org/browse/JDK-7036144)? If yes, then I think this is a good idea and agree to the plan. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3323485540
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Fri, 29 May 2026 08:28:19 GMT, Eirik Bjørsnøs wrote: >>> Just wanted to chime in to say that if we plan to introduce some form of >>> modality in behavior, I wonder if it would be beneficial to try and avoid >>> having a system property control behavior which is otherwise not reachable >>> programmatically. >> >> See the notes above about next steps which will propose a new constructor >> and maybe deprecate the existing constructors. > >> See the notes above about next steps which will propose a new constructor >> and maybe deprecate the existing constructors. > > Yes, sure, I'm fine with those > > My concern was more about any system property and that they should be simple > switches which picks the default mode. They should not hold independent > semantics which needs to be documented in other terms than the modes > documented in the API. Any system property introduced now should account for > the modes/constructors we want to introduce later. They will co-exist for a > long time. > > Maybe this is just a given and my comment here is superfluous. My concern was > usability and future users having to understand two different ways of > controlling behavior and resolve any conflicts between them. > > I think I'm happy if we avoid introducing a system propery now which later > proves difficult to define in terms of behavior specified by the new > constructors / APIs . My understanding is that we only need the system property for JDK 27 and earlier releases. Once we come up with a real fix which will probably contain API changes which can't be backported, the system property wouldn't be required any more and deprecated/removed as well. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3323453661
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Fri, 29 May 2026 08:07:07 GMT, Alan Bateman wrote: > See the notes above about next steps which will propose a new constructor and > maybe deprecate the existing constructors. Yes, sure, I'm fine with those My concern was more about any system property and that they should be simple switches which picks the default mode. They should not hold independent semantics which needs to be documented in other terms than the modes documented in the API. Any system property introduced now should account for the modes/constructors we want to introduce later. They will co-exist for a long time. Maybe this is just a given and my comment here is superfluous. My concern was usability and future users having to understand two different ways of controlling behavior and resolve any conflicts between them. I think I'm happy if we avoid introducing a system propery now which later proves difficult to define in terms of behavior specified by the new constructors / APIs . - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3323123373
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Fri, 29 May 2026 07:59:45 GMT, Eirik Bjørsnøs wrote: > Just wanted to chime in to say that if we plan to introduce some form of > modality in behavior, I wonder if it would be beneficial to try and avoid > having a system property control behavior which is otherwise not reachable > programmatically. See the notes above about next steps which will propose a new constructor and maybe deprecate the existing constructors. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3323007477
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Fri, 29 May 2026 06:36:35 GMT, Jaikiran Pai wrote: >> Thanks you all for your support! This sounds like a reasonable plan to me. >> >> How do you plan to solve point "3. For JDK 27, introduce a system >> property..."? Do you want to do this with the initial fix for >> [JDK-7036144](https://bugs.openjdk.org/browse/JDK-7036144), by wrapping the >> input stream and overriding available() or in a completely different way? >> >> It's also just a week away from Rampdown Phase One for JDK 27, so we have to >> hurry up. > >> How do you plan to solve point "3. For JDK 27, introduce a system >> property..."? Do you want to do this with the initial fix for >> [JDK-7036144](https://bugs.openjdk.org/browse/JDK-7036144), by wrapping the >> input stream and overriding available() or in a completely different way? > > The plan is to reintroduce the change that was done for JDK-7036144 and > enable that new behaviour only when the new system property is enabled. When > that system property isn't set (or disabled), which would be the default > case, then the implementation will continue with the long standing behaviour > of using `InputStream.available()`. > Jai, Lance and I met today to go over the issues with GZIPInputStream. Here's > a summary of where we got to: I'll admit to having lost track of the details of the functional root causes/issues here. Just wanted to chime in to say that if we plan to introduce some form of modality in behavior, I wonder if it would be beneficial to try and avoid having a system property control behavior which is otherwise not reachable programmatically. That is, all modalities should be reachable programmatically (choice of constructor/constuctor param), and any system property should simply flip the default mode for legacy code. Then the modes can be documented independently of the system property which won't need to hold much semantics. Again, I don't know the details here, just a high-level input which may or may not be useful :-) - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3322971085
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Thu, 28 May 2026 16:28:16 GMT, Volker Simonis wrote: > How do you plan to solve point "3. For JDK 27, introduce a system > property..."? Do you want to do this with the initial fix for > [JDK-7036144](https://bugs.openjdk.org/browse/JDK-7036144), by wrapping the > input stream and overriding available() or in a completely different way? The plan is to reintroduce the change that was done for JDK-7036144 and enable that new behaviour only when the new system property is enabled. When that system property isn't set (or disabled), which would be the default case, then the implementation will continue with the long standing behaviour of using `InputStream.available()`. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3322570189
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Thu, 28 May 2026 14:24:30 GMT, Alan Bateman wrote: >> JDK 25u is especially sensitive here, because JDK 25 was released with the >> fix for [JDK-7036144](https://bugs.openjdk.org/browse/JDK-7036144). As I >> wrote, we got complains after backing it out, so we will most probably leave >> it in our 25u releases. >> >> I also thought about using properties to control the behavior. It might be >> good if we could agree on a common property for Oracle, OpenJDK and >> VendorJDKs especially when it comes to downports. >> >> One simple solution might be to wrap the incoming `InputStream` into another >> `InputStream` which overrides `available()` to make sure it doesn't return >> `0` if there are more bytes to read. This wrapping could be controlled by a >> system property. What do you think? > > Jai, Lance and I met today to go over the issues with GZIPInputStream. Here's > a summary of where we got to: > > 1. For JDK 28 or later, introduce a new constructor to create a > GZIPInputStream that works like it should have done in JDK 1.1 when the API > was originally introduced. We discussed two possible sets of parameters. We > also discussed maybe deprecating the two existing constructors due to the > issue of treating available==0 as EOF and the issue of swallowing exceptions. > > 2. For JDK 27 or later, replace the class description as per the current > proposal. We can drop the spec update for the read method. The issue of an > I/O exception when reading beyond a trailer is problematic too so that might > have to be removed from the current wording. > > 3. For JDK 27, introduce a system property that controls the reliable > handling of streams with more than one member. Long standing behavior does > not handle the case where the underlying stream doesn't have a useful > avaialble() method and it additionally swallows any exception when reading > ahead. The system property can be documented as an implNote in the 2-arg > constructor or class description. The property will need default to long > standing behavior (not the more recent/changed behavior). > > 4. JDK 25u, system property to control behavior could be back ported to align > with 27. We understand the OpenJDK updates project has back ported > JDK-7036144 to other releases but we did not discuss this. > > 5. Expand the test coverage. It's always risky to change something in code > from JDK 1.0/1.1 so having tests that capture the current behavior (even if > unspecified) is important before making any change. Thanks you all for your support! This sounds like a reasonable plan to me. How do you plan to solve point "3. For JDK 27, introduce a system property..."? Do you want to do this with the initial fix for [JDK-7036144](https://bugs.openjdk.org/browse/JDK-7036144), by wrapping the input stream and overriding available() or in a completely different way? It's also just a week away from Rampdown Phase One for JDK 27, so we have to hurry up. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3319258726
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Wed, 27 May 2026 13:24:22 GMT, Volker Simonis wrote: >>> I'm also not so much concerned about JDK 27, so although I disagree with >>> this PR, I don't think it will create a lot of harm because nobody will >>> using it in production anyway. We still have enough time to fix this until >>> the next LTS will be released, but I **strongly** advocate for a solution >>> that can be downported to JDK 17, 21 and 25 as well. >> >> Our focus has to be forward looking. If fixing the issue with streams that >> contain a series of members, in conjunction with an underlying stream that >> doesn't have a useful available() implementation, can't be fixed without >> behavioural compatibility issues then we have the option of deprecating the >> existing constructors, and adding a new constructor or factory method. >> >> The backports are unfortunate but should not be our focus right now. It >> might be that they need a system property or some knob to deal with the >> issue on JDK 25u at least. > > JDK 25u is especially sensitive here, because JDK 25 was released with the > fix for [JDK-7036144](https://bugs.openjdk.org/browse/JDK-7036144). As I > wrote, we got complains after backing it out, so we will most probably leave > it in our 25u releases. > > I also thought about using properties to control the behavior. It might be > good if we could agree on a common property for Oracle, OpenJDK and > VendorJDKs especially when it comes to downports. > > One simple solution might be to wrap the incoming `InputStream` into another > `InputStream` which overrides `available()` to make sure it doesn't return > `0` if there are more bytes to read. This wrapping could be controlled by a > system property. What do you think? Jai, Lance and I met today to go over the issues with GZIPInputStream. Here's a summary of where we got to: 1. For JDK 28 or later, introduce a new constructor to create a GZIPInputStream that works like it should have done in JDK 1.1 when the API was originally introduced. We discussed two possible sets of parameters. We also discussed maybe deprecating the two existing constructors due to the issue of treating available==0 as EOF and the issue of swallowing exceptions. 2. For JDK 27 or later, replace the class description as per the current proposal. We can drop the spec update for the read method. The issue of an I/O exception when reading beyond a trailer is problematic too so that might have to be removed from the current wording. 3. For JDK 27, introduce a system property that controls the reliable handling of streams with more than one member. Long standing behavior does not handle the case where the underlying stream doesn't have a useful avaialble() method and it additionally swallows any exception when reading ahead. The system property can be documented as an implNote in the 2-arg constructor or class description. The property will need default to long standing behavior (not the more recent/changed behavior). 4. JDK 25u, system property to control behavior could be back ported to align with 27. We understand the OpenJDK updates project has back ported JDK-7036144 to other releases but we did not discuss this. 5. Expand the test coverage. It's always risky to change something in code from JDK 1.0/1.1 so having tests that capture the current behavior (even if unspecified) is important before making any change. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3318466104
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Wed, 27 May 2026 10:21:33 GMT, Alan Bateman wrote: >> I'm not so much concerned about JDK 1.1 than I am about JDK 25. Notice that >> we had already downported and shipped "[JDK-8381670: Revert the changes to >> GZIPInputStream related to InputStream.available() >> usage](https://bugs.openjdk.org/browse/JDK-8381670)" in our Corretto 25.0.3 >> release in April and almost instantly got a customer escalation from a huge >> client who got affected by the silent drop of concatenated GZIP members. >> I've just seen that you have now also backported >> "[JDK-8381670](https://bugs.openjdk.org/browse/JDK-8381670)" to Oracle JDK >> 25.0.5. Just be aware that this might cause more trouble than it solves. >> >> I'm also not so much concerned about JDK 27, so although I disagree with >> this PR, I don't think it will create a lot of harm because nobody will >> using it in production anyway. We still have enough time to fix this until >> the next LTS will be released, but I **strongly** advocate for a solution >> that can be downported to JDK 17, 21 and 25 as well. > >> I'm also not so much concerned about JDK 27, so although I disagree with >> this PR, I don't think it will create a lot of harm because nobody will >> using it in production anyway. We still have enough time to fix this until >> the next LTS will be released, but I **strongly** advocate for a solution >> that can be downported to JDK 17, 21 and 25 as well. > > Our focus has to be forward looking. If fixing the issue with streams that > contain a series of members, in conjunction with an underlying stream that > doesn't have a useful available() implementation, can't be fixed without > behavioural compatibility issues then we have the option of deprecating the > existing constructors, and adding a new constructor or factory method. > > The backports are unfortunate but should not be our focus right now. It might > be that they need a system property or some knob to deal with the issue on > JDK 25u at least. JDK 25u is especially sensitive here, because JDK 25 was released with the fix for [JDK-7036144](https://bugs.openjdk.org/browse/JDK-7036144). As I wrote, we got complains after backing it out, so we will most probably leave it in our 25u releases. I also thought about using properties to control the behavior. It might be good if we could agree on a common property for Oracle, OpenJDK and VendorJDKs especially when it comes to downports. One simple solution might be to wrap the incoming `InputStream` into another `InputStream` which overrides `available()` to make sure it doesn't return `0` if there are more bytes to read. This wrapping could be controlled by a system property. What do you think? - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3311041231
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Tue, 26 May 2026 19:17:44 GMT, Volker Simonis wrote: > I'm also not so much concerned about JDK 27, so although I disagree with this > PR, I don't think it will create a lot of harm because nobody will using it > in production anyway. We still have enough time to fix this until the next > LTS will be released, but I **strongly** advocate for a solution that can be > downported to JDK 17, 21 and 25 as well. Our focus has to be forward looking. If fixing the issue with streams that contain a series of members, in conjunction with an underlying stream that doesn't have a useful available() implementation, can't be fixed without behavioural compatibility issues then we have the option of deprecating the existing constructors, and adding a new constructor or factory method. The backports are unfortunate but should not be our focus right now. It might be that they need a system property or some knob to deal with the issue on JDK 25u at least. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3310172782
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Tue, 26 May 2026 17:32:23 GMT, Alan Bateman wrote: >>> I think everyone involved understands this. >> >> If by "*everyone involved*" you mean the people working on this PR I agree, >> however if you mean the users of `GZIPInputStream` I completely disagree. >> >>> There is work required to find a solution that does not impact existing >>> usages. We can't rule out deprecating the existing constructors. >> >> I understand, but that doesn't explain (at least to me) why it seems >> acceptable to you to backout a correctness fix in favor of fixing a >> performance regression and at the same time are so hesitant to explicitly >> mention the correctness issue in the documentation? > >> If by "_everyone involved_" you mean the people working on this PR I agree > > I mean everyone involved in the current cluster of issues. > >> I understand, but that doesn't explain (at least to me) why it seems >> acceptable to you to backout a correctness fix in favor of fixing a >> performance regression and at the same time are so hesitant to explicitly >> mention the correctness issue in the documentation? > > GZIPInputStream dates from JDK 1.1 so close to 30 years of existing usage. We > have to be super cautious with changes as we have no idea what might be > depending on existing behavior. The change has been backed out, and Jai is > improving the API docs before re-visiting the issue of readTrailer. I'm not so much concerned about JDK 1.1 than I am about JDK 25. Notice that we had already downported and shipped "[JDK-8381670: Revert the changes to GZIPInputStream related to InputStream.available() usage](https://bugs.openjdk.org/browse/JDK-8381670)" in our Corretto 25.0.3 release in April and almost instantly got a customer escalation from a huge client who got affected by the silent drop of concatenated GZIP members. I've just seen that you have now also backported "[JDK-8381670](https://bugs.openjdk.org/browse/JDK-8381670)" to Oracle JDK 25.0.5. Just be aware that this might cause more trouble than it solves. I'm also not so much concerned about JDK 27, so although I disagree with this PR, I don't think it will create a lot of harm because nobody will using it in production anyway. We still have enough time to fix this until the next LTS will be released, but I **strongly** advocate for a solution that can be downported to JDK 17, 21 and 25 as well. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3306281158
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Tue, 26 May 2026 16:59:10 GMT, Volker Simonis wrote: >>> @AlanBateman, but if `available()` returns 0, `readTrailer()` will return >>> `true` which will set `eos` in `read()` which will let `read()` return `-1` >>> (or "_end of the compressed input stream_") the next time it will be >>> called. And this is simply wrong if there exists more compressed members in >>> the underlying stream! >> >> I think everyone involved understands this. There is work required to find a >> solution that does not impact existing usages. We can't rule out deprecating >> the existing constructors. > >> I think everyone involved understands this. > > If by "*everyone involved*" you mean the people working on this PR I agree, > however if you mean the users of `GZIPInputStream` I completely disagree. > >> There is work required to find a solution that does not impact existing >> usages. We can't rule out deprecating the existing constructors. > > I understand, but that doesn't explain (at least to me) why it seems > acceptable to you to backout a correctness fix in favor of fixing a > performance regression and at the same time are so hesitant to explicitly > mention the correctness issue in the documentation? > If by "_everyone involved_" you mean the people working on this PR I agree I mean everyone involved in the current cluster of issues. > I understand, but that doesn't explain (at least to me) why it seems > acceptable to you to backout a correctness fix in favor of fixing a > performance regression and at the same time are so hesitant to explicitly > mention the correctness issue in the documentation? GZIPInputStream dates from JDK 1.1 so close to 30 years of existing usage. We have to be super cautious with changes as we have no idea what might be depending on existing behavior. The change has been backed out, and Jai is improving the API docs before re-visiting the issue of readTrailer. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3305674300
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Tue, 26 May 2026 15:36:47 GMT, Alan Bateman wrote: >> I'm not sure if it is the right thing to **mandate** a knowingly wrong >> implementation in the API doc which is what we effectively do in the current >> form. From my point of view, the only acceptable tag for such a sentence >> would be `@implNote` because it described implementation details which "*are >> allowed to vary across platforms, vendors, and versions*" (although this is >> unlikely nowadays where we more or less only have a single JDK >> implementation). > > @simonis It's okay to specify that it invokes the underlying stream's > available method. It is possible that you are reading this to mean that it > interprets 0 as EOF? That is not the intention. As per the discussion in > JBS, the general plan is re-visit the issue of how readTrailer behaves once > GZIPInputStream behavior is restored (already done), and once its > specification is updated to document long standing behavior. > @AlanBateman, but if `available()` returns 0, `readTrailer()` will return > `true` which will set `eos` in `read()` which will let `read()` return `-1` > (or "_end of the compressed input stream_") the next time it will be called. > And this is simply wrong if there exists more compressed members in the > underlying stream! I think everyone involved understands this. There is work required to find a solution that does not impact existing usages. We can't rule out deprecating the existing constructors. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3305345746
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Tue, 26 May 2026 15:36:47 GMT, Alan Bateman wrote: >> I'm not sure if it is the right thing to **mandate** a knowingly wrong >> implementation in the API doc which is what we effectively do in the current >> form. From my point of view, the only acceptable tag for such a sentence >> would be `@implNote` because it described implementation details which "*are >> allowed to vary across platforms, vendors, and versions*" (although this is >> unlikely nowadays where we more or less only have a single JDK >> implementation). > > @simonis It's okay to specify that it invokes the underlying stream's > available method. It is possible that you are reading this to mean that it > interprets 0 as EOF? That is not the intention. As per the discussion in > JBS, the general plan is re-visit the issue of how readTrailer behaves once > GZIPInputStream behavior is restored (already done), and once its > specification is updated to document long standing behavior. @AlanBateman, but if `available()` returns 0, `readTrailer()` will return `true` which will set `eos` in `read()` which will let `read()` return `-1` (or "*end of the compressed input stream*") the next time it will be called. And this is simply wrong if there exists more compressed members in the underlying stream! If you absolutely want to back out the fix and update the API doc, then you should at least put a big, bold warning in the documentation which clearly states that `GZIPInputStream` is **not** fully RFC 1952 compatible and may **not** reliably read concatenated GZIP streams if the implementation of `available()` in the underlying input stream is not returning the correct number of remaining bytes in the stream (which it is not required to do according to the specification). This would be the correct way to "*document long standing behavior*". Instead, the current update of the API doc just mentions the use of `available()` without making the consequences clear - namely the inability to correctly read concatenated GZIP streams. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3305226749
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Tue, 26 May 2026 14:37:58 GMT, Volker Simonis wrote: >> Fixed. > > I'm not sure if it is the right thing to **mandate** a knowingly wrong > implementation in the API doc which is what we effectively do in the current > form. From my point of view, the only acceptable tag for such a sentence > would be `@implNote` because it described implementation details which "*are > allowed to vary across platforms, vendors, and versions*" (although this is > unlikely nowadays where we more or less only have a single JDK > implementation). @simonis It's okay to specify that it invokes the underlying stream's available method. It is possible that you are reading this to mean that it interprets 0 as EOF? That is not the intention. As per the discussion in JBS, the general plan is re-visit the issue of how readTrailer behaves once GZIPInputStream behavior is restored (already done), and once its specification is updated to document long standing behavior. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3304967667
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Mon, 25 May 2026 13:02:28 GMT, Jaikiran Pai wrote: >> For this case, I think you can drop the "implSpec" tag so that it's part of >> the method description. > > Fixed. I'm not sure if it is the right thing to **mandate** a knowingly wrong implementation in the API doc which is what we effectively do in the current form. From my point of view, the only acceptable tag for such a sentence would be `@implNote` because it described implementation details which "*are allowed to vary across platforms, vendors, and versions*" (although this is unlikely nowadays where we more or less only have a single JDK implementation). - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3304550993
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Mon, 18 May 2026 09:06:46 GMT, Alan Bateman wrote: >> It's for both, the users/callers of the GZIPInputStream who pass in the >> underlying InputStream when constructing the GZIPInputStream instance and >> for any libraries which may subclass this class. >> >> But to me it's always a bit of a dilemma when it comes to the usage of >> `@apiNote`, `@implSpec` (and sometimes `@implNote`). Do you suggest using >> `@apiNote` here? > > For this case, I think you can drop the "implSpec" tag so that it's part of > the method description. Fixed. - PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3298289132
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Sat, 16 May 2026 07:20:03 GMT, Jaikiran Pai wrote:
>> src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 182:
>>
>>> 180: * @implSpec After reading a member trailer, this method calls
>>> {@link InputStream#available()}
>>> 181: * on the underlying stream to determine whether additional data
>>> is available that may represent
>>> 182: * a subsequent member.
>>
>> Who is the audience for this? Usually implSpec is aim at
>> implementers/subclasses. Here I think this note is aimed at the user of the
>> API that provides the input stream.
>
> It's for both, the users/callers of the GZIPInputStream who pass in the
> underlying InputStream when constructing the GZIPInputStream instance and for
> any libraries which may subclass this class.
>
> But to me it's always a bit of a dilemma when it comes to the usage of
> `@apiNote`, `@implSpec` (and sometimes `@implNote`). Do you suggest using
> `@apiNote` here?
For this case, I think you can drop the "implSpec" tag so that it's part of the
method description.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3257643309
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Wed, 13 May 2026 10:57:12 GMT, Alan Bateman wrote:
>> Jaikiran Pai has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 17 additional
>> commits since the last revision:
>>
>> - merge latest from master branch
>> - merge latest from master branch
>> - minor adjustments
>> - Lance's review
>> - merge latest from master branch
>> - introduce test to verify read() throws IOException when invoked on closed
>> stream
>> - specify existing behaviour of throwing IOException when stream is already
>> closed
>> - merge latest from master branch
>> - specify that GZIPInputStream is not thread-safe
>> - refer to section 2.2 of the RFC
>> - ... and 7 more: https://git.openjdk.org/jdk/compare/80a78b38...a22c1b83
>
> src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 182:
>
>> 180: * @implSpec After reading a member trailer, this method calls
>> {@link InputStream#available()}
>> 181: * on the underlying stream to determine whether additional data is
>> available that may represent
>> 182: * a subsequent member.
>
> Who is the audience for this? Usually implSpec is aim at
> implementers/subclasses. Here I think this note is aimed at the user of the
> API that provides the input stream.
It's for both, the users/callers of the GZIPInputStream who pass in the
underlying InputStream when constructing the GZIPInputStream instance and for
any libraries which may subclass this class.
But to me it's always a bit of a dilemma when it comes to the usage of
`@apiNote`, `@implSpec` (and sometimes `@implNote`). Do you suggest using
`@apiNote` here?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3252434152
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Wed, 13 May 2026 10:53:23 GMT, Alan Bateman wrote:
>> Jaikiran Pai has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 17 additional
>> commits since the last revision:
>>
>> - merge latest from master branch
>> - merge latest from master branch
>> - minor adjustments
>> - Lance's review
>> - merge latest from master branch
>> - introduce test to verify read() throws IOException when invoked on closed
>> stream
>> - specify existing behaviour of throwing IOException when stream is already
>> closed
>> - merge latest from master branch
>> - specify that GZIPInputStream is not thread-safe
>> - refer to section 2.2 of the RFC
>> - ... and 7 more: https://git.openjdk.org/jdk/compare/f7548393...a22c1b83
>
> src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 40:
>
>> 38: *
>> 39: * GZIP file format
>> 40: * Section 2.2 of RFC-1952 specifies GZIP file format as consisting of a
>> series of "members"
>
> The RFC specifies the GZIP file format and it might be better to link to that
> first before jumping to section 2.2. Also it might be better to say that the
> members appear one after another before discussing what is in each member,
> e.g.
>
> "The GZIP file format is specified by RFC 1952. The format consists of a
> series of "members" that appear one after another in the stream with with no
> additional information before, between, or after. Each member consists of a
> header, followed by ...".
Done, I've adjusted this class level doc to follow your suggestion. Let me know
if you would like additional changes.
> src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 121:
>
>> 119: * @throwsNullPointerException if {@code in} is null
>> 120: * @throwsIOException if an I/O error occurs when reading the
>> member header
>> 121: *from the underlying stream
>
> Are we guaranteeing that this will not attempt to read beyond the header?
Hello Alan, yes, we do guarantee that when reading the header we will not read
additional bytes past the header. Infact, when reading the header or the
trailer of a member, we don't read or buffer additional bytes. It's only during
decompressing the deflated data of a member, through the InflaterInputStream,
we may read and buffer additional data that may go past a member's trailer. The
only other time we read past a trailer is when we go looking for the header of
the next member, after the previous member's trailer is read.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3252430207
PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3252429404
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
On Wed, 13 May 2026 06:53:28 GMT, Jaikiran Pai wrote:
>> Can I please get a review of this doc and test-only change for
>> https://bugs.openjdk.org/browse/JDK-8322256?
>>
>> The `java.util.zip.GZIPInputStream` has been in the JDK since Java 1.1.
>> However, its specification hasn't been clear on how it behaves, especially
>> when a `InputStream` consists of more than one GZIP member.
>>
>> The commit in this PR updates the specification of this class to match its
>> current (long standing) implementation.
>>
>> A new jtreg test has been introduced to verify this behaviour.
>>
>> I'll draft a CSR shortly.
>>
>> -
>> - [x] I confirm that I make this contribution in accordance with the
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Jaikiran Pai has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains 17 additional
> commits since the last revision:
>
> - merge latest from master branch
> - merge latest from master branch
> - minor adjustments
> - Lance's review
> - merge latest from master branch
> - introduce test to verify read() throws IOException when invoked on closed
> stream
> - specify existing behaviour of throwing IOException when stream is already
> closed
> - merge latest from master branch
> - specify that GZIPInputStream is not thread-safe
> - refer to section 2.2 of the RFC
> - ... and 7 more: https://git.openjdk.org/jdk/compare/494a174d...a22c1b83
src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 40:
> 38: *
> 39: * GZIP file format
> 40: * Section 2.2 of RFC-1952 specifies GZIP file format as consisting of a
> series of "members"
The RFC specifies the GZIP file format and it might be better to link to that
first before jumping to section 2.2. Also it might be better to say that the
members appear one after another before discussing what is in each member, e.g.
"The GZIP file format is specified by RFC 1952. The format consists of a series
of "members" that appear one after another in the stream with with no
additional information before, between, or after. Each member consists of a
header, followed by ...".
src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 121:
> 119: * @throwsNullPointerException if {@code in} is null
> 120: * @throwsIOException if an I/O error occurs when reading the
> member header
> 121: *from the underlying stream
Are we guaranteeing that this will not attempt to read beyond the header?
src/java.base/share/classes/java/util/zip/GZIPInputStream.java line 182:
> 180: * @implSpec After reading a member trailer, this method calls
> {@link InputStream#available()}
> 181: * on the underlying stream to determine whether additional data is
> available that may represent
> 182: * a subsequent member.
Who is the audience for this? Usually implSpec is aim at
implementers/subclasses. Here I think this note is aimed at the user of the API
that provides the input stream.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3233554996
PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3233567713
PR Review Comment: https://git.openjdk.org/jdk/pull/30925#discussion_r3233579371
Re: RFR: 8322256: Define and document GZIPInputStream concatenated stream semantics [v11]
> Can I please get a review of this doc and test-only change for > https://bugs.openjdk.org/browse/JDK-8322256? > > The `java.util.zip.GZIPInputStream` has been in the JDK since Java 1.1. > However, its specification hasn't been clear on how it behaves, especially > when a `InputStream` consists of more than one GZIP member. > > The commit in this PR updates the specification of this class to match its > current (long standing) implementation. > > A new jtreg test has been introduced to verify this behaviour. > > I'll draft a CSR shortly. > > - > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). Jaikiran Pai has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 17 additional commits since the last revision: - merge latest from master branch - merge latest from master branch - minor adjustments - Lance's review - merge latest from master branch - introduce test to verify read() throws IOException when invoked on closed stream - specify existing behaviour of throwing IOException when stream is already closed - merge latest from master branch - specify that GZIPInputStream is not thread-safe - refer to section 2.2 of the RFC - ... and 7 more: https://git.openjdk.org/jdk/compare/03f02d48...a22c1b83 - Changes: - all: https://git.openjdk.org/jdk/pull/30925/files - new: https://git.openjdk.org/jdk/pull/30925/files/a3c3f4d2..a22c1b83 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=30925&range=10 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=30925&range=09-10 Stats: 8288 lines in 126 files changed: 5833 ins; 1595 del; 860 mod Patch: https://git.openjdk.org/jdk/pull/30925.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/30925/head:pull/30925 PR: https://git.openjdk.org/jdk/pull/30925
