Author:Jaikiran Pai
URL:
https://git.openjdk.org/jdk/commit/b5cfd76c047392788b6a5c25ebadc463b2c8ce90
Stats: 132 lines in 2 files changed: 124 ins; 0 del; 8 mod
8358456: ZipFile.getInputStream(ZipEntry) throws unspecified
IllegalArgumentException
Reviewed-by: lancea
-
PR: https://git.openjdk.org/jdk/pull/25606
On Tue, 3 Jun 2025 06:35:09 GMT, Jaikiran Pai wrote:
> Can I please get a review of this change which addresses the issue noted in
> https://bugs.openjdk.org/browse/JDK-8358456?
>
> In Java 24, through https://bugs.openjdk.org/browse/JDK-8341597 we did a
> change which started using the "compr
On Tue, 3 Jun 2025 06:35:09 GMT, Jaikiran Pai wrote:
> Can I please get a review of this change which addresses the issue noted in
> https://bugs.openjdk.org/browse/JDK-8358456?
>
> In Java 24, through https://bugs.openjdk.org/browse/JDK-8341597 we did a
> change which started using the "compr
On Tue, 3 Jun 2025 06:35:09 GMT, Jaikiran Pai wrote:
> Can I please get a review of this change which addresses the issue noted in
> https://bugs.openjdk.org/browse/JDK-8358456?
>
> In Java 24, through https://bugs.openjdk.org/browse/JDK-8341597 we did a
> change which started using the "compr
On Tue, 3 Jun 2025 06:35:09 GMT, Jaikiran Pai wrote:
> Can I please get a review of this change which addresses the issue noted in
> https://bugs.openjdk.org/browse/JDK-8358456?
>
> In Java 24, through https://bugs.openjdk.org/browse/JDK-8341597 we did a
> change which started using the "compr
On Tue, 3 Jun 2025 06:35:09 GMT, Jaikiran Pai wrote:
> Can I please get a review of this change which addresses the issue noted in
> https://bugs.openjdk.org/browse/JDK-8358456?
>
> In Java 24, through https://bugs.openjdk.org/browse/JDK-8341597 we did a
> change which started using the "compr
size of the entry to decide the input buffer size.
-
Commit messages:
- 8358456: ZipFile.getInputStream(ZipEntry) throws unspecified
IllegalArgumentException
Changes: https://git.openjdk.org/jdk/pull/25606/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=25606&range=00
Issue: https
a CRC check for that entry. The change in this PR addresses
> that and introduces a new jtreg test which reproduces the issue and verifies
> the fix.
>
> tier testing is currently in progress.
I found «empty ZipEntry» in the title here somewhat confusing since that’s an
API class a
On Thu, 8 May 2025 13:29:36 GMT, Alan Bateman wrote:
> > I'll run some experiments and see what it shows up.
>
> Thanks as that will help inform as to whether this will need a compatibility
> knob.
The crc is calculated as part of the write of the entry and I have not seen any
cases, where th
ts from ZipDetails will
be accurate.
Again, not a big deal either way, but wanted to mention it
test/jdk/java/util/zip/ZipInputStream/ZipInputStreamCRCCheck.java line 99:
> 97: final ZipEntry entry = zis.getNextEntry();
> 98: assertEquals(entryName, entry.getName(),
On Thu, 8 May 2025 13:10:57 GMT, Jaikiran Pai wrote:
> I'll run some experiments and see what it shows up.
Thanks as that will help inform as to whether this will need a compatibility
knob.
-
PR Comment: https://git.openjdk.org/jdk/pull/25116#issuecomment-2863078781
On Thu, 8 May 2025 13:01:38 GMT, Alan Bateman wrote:
> I'm just wondering if there are any wonky tools or plugins in the eco system,
> the output of which could be impacted by the more strict check.
I'll run some experiments and see what it shows up.
-
PR Comment: https://git.open
On Thu, 8 May 2025 12:08:09 GMT, Jaikiran Pai wrote:
> Can I please get a review of this change which addresses the issue noted in
> https://bugs.openjdk.org/browse/JDK-8354799?
>
> `java.util.zip.ZipInputStream` when dealing with a `STORED` entry of zero
> size was missing a CRC check for tha
new jtreg test which reproduces the issue and verifies the fix.
tier testing is currently in progress.
-
Commit messages:
- 8354799: ZipInputStream doesn't verify CRC for empty ZipEntry
- 8354799: add test
Changes: https://git.openjdk.org/jdk/pull/25116/files
Webrev:
On Wed, 16 Oct 2024 17:40:36 GMT, Lance Andersen wrote:
> Please review the changes for
> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025) which
> addresses that
>
>
On Sat, 19 Oct 2024 16:28:34 GMT, Lance Andersen wrote:
>> Please review the changes for
>> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
>> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025)
>> which addresses that
On Sat, 19 Oct 2024 15:38:26 GMT, Claes Redestad wrote:
>> Lance Andersen has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add Combined test
>
> src/java.base/share/classes/java/util/zip/ZipEntry.java line 723:
>
>> 721: * @return t
On Sat, 19 Oct 2024 16:28:34 GMT, Lance Andersen wrote:
>> Please review the changes for
>> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
>> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025)
>> which addresses that
On Sat, 19 Oct 2024 15:52:59 GMT, Lance Andersen wrote:
>> src/java.base/share/classes/java/util/zip/ZipEntry.java line 723:
>>
>>> 721: * @return true if valid CEN Header size; false otherwise
>>> 722: */
>>> 723: static boolean isCENHeaderValid(String name, byte[] extra, String
> Please review the changes for
> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025) which
> addresses that
>
> - ZipEntry(String)
> - ZipEntry::setComment
> - ZipEntry:
On Fri, 18 Oct 2024 21:24:18 GMT, Lance Andersen wrote:
>> Please review the changes for
>> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
>> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025)
>> which addresses that
On Fri, 18 Oct 2024 21:24:18 GMT, Lance Andersen wrote:
>> Please review the changes for
>> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
>> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025)
>> which addresses that
> Please review the changes for
> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025) which
> addresses that
>
> - ZipEntry(String)
> - ZipEntry::setComment
> - ZipEntry:
On Fri, 18 Oct 2024 20:42:47 GMT, Eirik Bjørsnøs wrote:
>> I am going to leat the comment as is
>
> Sorry, this was not to comment the summary, just pointing out that we have no
> test verifying the combined length exceeding the limit when each component
> alone does not.
Thanks for the clarif
> Please review the changes for
> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025) which
> addresses that
>
> - ZipEntry(String)
> - ZipEntry::setComment
> - ZipEntry:
On Fri, 18 Oct 2024 20:29:00 GMT, Eirik Bjørsnøs wrote:
>> Lance Andersen has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> change field -> fields
>
> test/jdk/java/util/zip/ZipEntry/MaxZipEntryFie
On Fri, 18 Oct 2024 20:38:31 GMT, Lance Andersen wrote:
>> test/jdk/java/util/zip/ZipEntry/MaxZipEntryFieldSizeTest.java line 28:
>>
>>> 26: * @summary Verify that ZipEntry(String), ZipEntry::setComment, and
>>> 27: * ZipEntry::setExtra throws a IllegalArg
On Fri, 18 Oct 2024 20:06:14 GMT, Lance Andersen wrote:
>> Please review the changes for
>> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
>> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025)
>> which addresses that
> Please review the changes for
> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025) which
> addresses that
>
> - ZipEntry(String)
> - ZipEntry::setComment
> - ZipEntry:
ds" intentional? Question also applies to
> `setComment`. The constructor does not have a comma at this position.
>
> Suggestion:
>
> * {@linkplain #CENHDR CEN Header size} exceeds 65,535 bytes.
removed
> test/jdk/java/util/zip/ZipEntry/MaxZipEntryFieldSizeTest.java line 28
> Please review the changes for
> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025) which
> addresses that
>
> - ZipEntry(String)
> - ZipEntry::setComment
> - ZipEntry:
On Fri, 18 Oct 2024 14:02:09 GMT, Lance Andersen wrote:
>> Please review the changes for
>> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
>> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025)
>> which addresses that
> Please review the changes for
> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025) which
> addresses that
>
> - ZipEntry(String)
> - ZipEntry::setComment
> - ZipEntry:
On Fri, 18 Oct 2024 13:30:37 GMT, Eirik Bjørsnøs wrote:
> Looks good. See comment about `headerSize` overflow plus some other minor
> comments.
Thank you
>
> Would this change in the `ZipEntry` API description require a re-approval of
> the CSR, even if just a formality?
Y
On Fri, 18 Oct 2024 13:24:08 GMT, Eirik Bjørsnøs wrote:
>> Lance Andersen has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Added additional validation to ZipEntry
>
> src/java.base/share/classes/java/u
On Fri, 18 Oct 2024 12:58:17 GMT, Lance Andersen wrote:
>> Please review the changes for
>> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
>> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025)
>> which addresses that
On Fri, 18 Oct 2024 12:58:17 GMT, Lance Andersen wrote:
>> Please review the changes for
>> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
>> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025)
>> which addresses that
dress the overall change in
>>> validation.
>>>
>>> So after we fork JDK 24, happyt to revisit.
>>
>> If the route we're taking ends up with having `ZipEntry` manage its own
>> invariant here, then I'm only lukewarm to including this solution in 24
&g
> Please review the changes for
> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025) which
> addresses that
>
> - ZipEntry(String)
> - ZipEntry::setComment
> - ZipEntry:
On Thu, 17 Oct 2024 18:45:01 GMT, Eirik Bjørsnøs wrote:
> But that's my subjective opinion, it's understandable and fine that others
> see it differently.
Again, I understand your suggestion and will give it some additional thought.
The original intent was to address the incorrect max value t
; validation.
>
> So after we fork JDK 24, happyt to revisit.
If the route we're taking ends up with having `ZipEntry` manage its own
invariant here, then I'm only lukewarm to including this solution in 24 which
only takes us half way and has weaker validation than what's alre
null, null, "entry name too long");
>
>
> setExtra:
>
>
> checkCombinedLength(name, extra, comment, "invalid extra field length");
>
>
> setComment:
>
> checkCombinedLength(name, extra, comment, "entry comment too long");
>
&g
On Thu, 17 Oct 2024 11:41:05 GMT, Eirik Bjørsnøs wrote:
>> Lance Andersen has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add back missing putNextEntry call
>
> test/jdk/java/util/zip/ZipOutputStream/ZipOutputStreamMaxCenHdrTest.java lin
> Please review the changes for
> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025) which
> addresses that
>
> - ZipEntry(String)
> - ZipEntry::setComment
> - ZipEntry:
On Thu, 17 Oct 2024 10:44:51 GMT, Lance Andersen wrote:
>> Please review the changes for
>> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
>> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025)
>> which addresses that
On Thu, 17 Oct 2024 10:44:51 GMT, Lance Andersen wrote:
>> Please review the changes for
>> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
>> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025)
>> which addresses that
On Thu, 17 Oct 2024 10:44:51 GMT, Lance Andersen wrote:
>> Please review the changes for
>> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
>> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025)
>> which addresses that
Lance, with this change, it looks like this now misses calling the
> `zos.putNextEntry(zipEntry)` when `expectZipException` is `false`. Is that an
> oversight?
Yes, an oversight as sometimes you see what you want to see.
added a call to `zos.putNextEntry(zipEntry) `when when `expectZipException` is
> Please review the changes for
> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025) which
> addresses that
>
> - ZipEntry(String)
> - ZipEntry::setComment
> - ZipEntry:
On Wed, 16 Oct 2024 17:40:36 GMT, Lance Andersen wrote:
> Please review the changes for
> [JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
> follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025) which
> addresses that
>
>
Please review the changes for
[JDK-8340553](https://bugs.openjdk.org/browse/JDK-8340553), which is a
follow-on to [JDK-8336025](https://bugs.openjdk.org/browse/JDK-8336025) which
addresses that
- ZipEntry(String)
- ZipEntry::setComment
- ZipEntry::setExtra
currently validate that the max
On Tue, 24 Sep 2024 00:21:57 GMT, Lei Zhu wrote:
> 8340553: ZipEntry field validation does not take into account the size of a
> CEN header
This pull request has been closed without being integrated.
-
PR: https://git.openjdk.org/jdk/pull/21147
On Tue, 24 Sep 2024 02:06:56 GMT, Jaikiran Pai wrote:
> Hello @Korov, Lance is already working on this change and is planning to open
> a PR for the change shortly.
OK, thanks for replay!
-
PR Comment: https://git.openjdk.org/jdk/pull/21147#issuecomment-2370010562
On Tue, 24 Sep 2024 01:51:56 GMT, Lei Zhu wrote:
>> 8340553: ZipEntry field validation does not take into account the size of a
>> CEN header
>
> Lei Zhu has updated the pull request incrementally with one additional commit
> since the last revision:
>
> 8340553
> 8340553: ZipEntry field validation does not take into account the size of a
> CEN header
Lei Zhu has updated the pull request incrementally with one additional commit
since the last revision:
8340553: ZipEntry field validation does not take into account the size of a
CEN
> 8340553: ZipEntry field validation does not take into account the size of a
> CEN header
Lei Zhu has updated the pull request with a new target base due to a merge or a
rebase. The pull request now contains one commit:
8340553: ZipEntry field validation does not take into account th
8340553: ZipEntry field validation does not take into account the size of a CEN
header
-
Commit messages:
- 8340553: ZipEntry field validation does not take into account the size of a
CEN header
- 8340553: ZipEntry field validation does not take into account the size of a
CEN
My immediate interest is symlinks.
Alan
> On Dec 13, 2023, at 1:55 AM, Alan Bateman wrote:
>
> On 12/12/2023 20:17, Alan Snyder wrote:
>> ZipEntry is a public class and I am aware that it is used outside the JDK.
>> Presumably that is not a problem.
>>
>> I
On 12/12/2023 20:17, Alan Snyder wrote:
ZipEntry is a public class and I am aware that it is used outside the JDK.
Presumably that is not a problem.
I’m wondering why the class stores the external file attributes field but does
not provide public accessors for it.
I would find it useful to
produce. So while exposing the "external file attributes" field
(which was incorrectly named "extraAttributes" in ZipEntry) is perhaps not
harmful in itself, the question is: Where do we stop?
Right now, I’m more interested in reading zip files, but to answer your
question:
ng the "external file attributes"
> field (which was incorrectly named "extraAttributes" in ZipEntry) is perhaps
> not harmful in itself, the question is: Where do we stop?
>
Right now, I’m more interested in reading zip files, but to answer your
question: we can
aAttributes" in
ZipEntry) is perhaps not harmful in itself, the question is: Where do we
stop?
It's worth noting that the ZipFileSystem API has the
"enablePosixFileAttributes" env option which lets you set POSIX permissions
on ZIP file entries using Files.setPosixFilePermissions()
T
ZipEntry is a public class and I am aware that it is used outside the JDK.
Presumably that is not a problem.
I’m wondering why the class stores the external file attributes field but does
not provide public accessors for it.
I would find it useful to have access to this field. I would rather
n integrated.
Changeset: a8f0f575
Author:Lance Andersen
URL:
https://git.openjdk.org/jdk/commit/a8f0f575abab53e89fc315a68394b556543cbb2e
Stats: 30 lines in 1 file changed: 27 ins; 0 del; 3 mod
8278165: Clarify that ZipInputStream does not access the CEN fields for a
ZipEntry
On Fri, 16 Sep 2022 11:01:02 GMT, Lance Andersen wrote:
>> Hi,
>>
>> Please review this update to the ZipInputStream class description to clarify
>> that ZipInputStream walks sequentially through each Zip Entry contained
>> within the Zip File. As a result, the CEN header for the Zip file ent
> Hi,
>
> Please review this update to the ZipInputStream class description to clarify
> that ZipInputStream walks sequentially through each Zip Entry contained
> within the Zip File. As a result, the CEN header for the Zip file entries
> are not read resulting in ZipInputStream not having acc
On Wed, 14 Sep 2022 16:54:56 GMT, Lance Andersen wrote:
>> Hi,
>>
>> Please review this update to the ZipInputStream class description to clarify
>> that ZipInputStream walks sequentially through each Zip Entry contained
>> within the Zip File. As a result, the CEN header for the Zip file ent
On Wed, 14 Sep 2022 11:57:17 GMT, Alan Bateman wrote:
> One other comment on the snippet is that the type of "jar" may not be obvious
> to readers. I think you'll need Path jar = ... in which case changing it
> Files.newInputStream(jar) might be simpler.
Updated per your suggestion
> I wonder
> Hi,
>
> Please review this update to the ZipInputStream class description to clarify
> that ZipInputStream walks sequentially through each Zip Entry contained
> within the Zip File. As a result, the CEN header for the Zip file entries
> are not read resulting in ZipInputStream not having acc
On Tue, 13 Sep 2022 21:13:15 GMT, Lance Andersen wrote:
>> Hi,
>>
>> Please review this update to the ZipInputStream class description to clarify
>> that ZipInputStream walks sequentially through each Zip Entry contained
>> within the Zip File. As a result, the CEN header for the Zip file ent
> Hi,
>
> Please review this update to the ZipInputStream class description to clarify
> that ZipInputStream walks sequentially through each Zip Entry contained
> within the Zip File. As a result, the CEN header for the Zip file entries
> are not read resulting in ZipInputStream not having acc
> Hi,
>
> Please review this update to the ZipInputStream class description to clarify
> that ZipInputStream walks sequentially through each Zip Entry contained
> within the Zip File. As a result, the CEN header for the Zip file entries
> are not read resulting in ZipInputStream not having acc
On Tue, 13 Sep 2022 19:30:01 GMT, Alan Bateman wrote:
>> Lance Andersen has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Address sample indentation and typo
>
> src/java.base/share/classes/java/util/zip/ZipInputStream.java line 65:
>
>>
On Tue, 13 Sep 2022 18:12:44 GMT, Lance Andersen wrote:
>> Hi,
>>
>> Please review this update to the ZipInputStream class description to clarify
>> that ZipInputStream walks sequentially through each Zip Entry contained
>> within the Zip File. As a result, the CEN header for the Zip file ent
On Tue, 13 Sep 2022 18:12:44 GMT, Lance Andersen wrote:
>> Hi,
>>
>> Please review this update to the ZipInputStream class description to clarify
>> that ZipInputStream walks sequentially through each Zip Entry contained
>> within the Zip File. As a result, the CEN header for the Zip file ent
On Tue, 13 Sep 2022 17:38:29 GMT, Brian Burkhalter wrote:
>> Lance Andersen has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Address sample indentation and typo
>
> src/java.base/share/classes/java/util/zip/ZipInputStream.java line 49:
>
> Hi,
>
> Please review this update to the ZipInputStream class description to clarify
> that ZipInputStream walks sequentially through each Zip Entry contained
> within the Zip File. As a result, the CEN header for the Zip file entries
> are not read resulting in ZipInputStream not having acc
On Wed, 31 Aug 2022 16:06:57 GMT, Lance Andersen wrote:
> Hi,
>
> Please review this update to the ZipInputStream class description to clarify
> that ZipInputStream walks sequentially through each Zip Entry contained
> within the Zip File. As a result, the CEN header for the Zip file entries
Hi,
Please review this update to the ZipInputStream class description to clarify
that ZipInputStream walks sequentially through each Zip Entry contained within
the Zip File. As a result, the CEN header for the Zip file entries are not
read resulting in ZipInputStream not having access to infor
79 matches
Mail list logo
