Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File [v2]
On Thu, 11 Nov 2021 17:19:04 GMT, Lance Andersen wrote: >> Hi all, >> >> This patch addresses a regression introduced in JDK 15 via JDK-8242959 where >> you can no longer access a file entry contained within a Zip file when there >> is also a directory entry with the same name via ZipFile:getEntry(). >> >> Once fixed, the behavior will be consistent with earlier JDK releases. >> >> Mach5 tiers 1-3 have been run without failure >> >> Best >> Lance > > Lance Andersen has updated the pull request incrementally with one additional > commit since the last revision: > > Address minor review comments Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6342
Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File [v2]
On Thu, 11 Nov 2021 17:19:04 GMT, Lance Andersen wrote: >> Hi all, >> >> This patch addresses a regression introduced in JDK 15 via JDK-8242959 where >> you can no longer access a file entry contained within a Zip file when there >> is also a directory entry with the same name via ZipFile:getEntry(). >> >> Once fixed, the behavior will be consistent with earlier JDK releases. >> >> Mach5 tiers 1-3 have been run without failure >> >> Best >> Lance > > Lance Andersen has updated the pull request incrementally with one additional > commit since the last revision: > > Address minor review comments Marked as reviewed by redestad (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6342
Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File [v2]
On Thu, 11 Nov 2021 12:04:46 GMT, Alan Bateman wrote:
>> Lance Andersen has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Address minor review comments
>
> test/jdk/java/util/zip/ZipFile/ZipFileDuplicateEntryTest.java line 176:
>
>> 174: zos.closeEntry();
>> 175: } catch (IOException e) {
>> 176: e.printStackTrace();
>
> Did you mean to catch IOException here? If the setup fails then the test will
> still run.
Removed the catch clause
-
PR: https://git.openjdk.java.net/jdk/pull/6342
Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File [v2]
On Thu, 11 Nov 2021 12:50:30 GMT, Claes Redestad wrote: >> Thank you for that clarification. The "addSlash" param being "false" in the >> call below that comment is what made me think that the comment had a typo. I >> read that code in a bit more detail now and I see what that comment means. >> It essentially says that it's now issuing a call to find a entry position >> (if it exists) for the name ending with a "/" character. > > The comment is a bit too fuzzy. What's happening here is we searched for > `foo` and found a match for `foo/` - but we can't be sure there's no exact > entry `foo` later on in the hash bucket, so we (somewhat inefficiently) redo > the search for `foo` without the trailing slash parameter. > > I suggest clarifying to something like this: > ``` // name + "/" entry found, keep looking for and prefer an entry which > exactly matches name``` I just tweaked the comment slightly - PR: https://git.openjdk.java.net/jdk/pull/6342
Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File [v2]
> Hi all, > > This patch addresses a regression introduced in JDK 15 via JDK-8242959 where > you can no longer access a file entry contained within a Zip file when there > is also a directory entry with the same name via ZipFile:getEntry(). > > Once fixed, the behavior will be consistent with earlier JDK releases. > > Mach5 tiers 1-3 have been run without failure > > Best > Lance Lance Andersen has updated the pull request incrementally with one additional commit since the last revision: Address minor review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/6342/files - new: https://git.openjdk.java.net/jdk/pull/6342/files/2d5e5913..93b46c66 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6342=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6342=00-01 Stats: 5 lines in 2 files changed: 0 ins; 4 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6342.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6342/head:pull/6342 PR: https://git.openjdk.java.net/jdk/pull/6342
Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File
On Thu, 11 Nov 2021 12:25:38 GMT, Jaikiran Pai wrote: >> Hi Jaikiran, >> >> The comment is correct > > Thank you for that clarification. The "addSlash" param being "false" in the > call below that comment is what made me think that the comment had a typo. I > read that code in a bit more detail now and I see what that comment means. It > essentially says that it's now issuing a call to find a entry position (if it > exists) for the name ending with a "/" character. The comment is a bit too fuzzy. What's happening here is we searched for `foo` and found a match for `foo/` - but we can't be sure there's no exact entry `foo` later on in the hash bucket, so we (somewhat inefficiently) redo the search for `foo` without the trailing slash parameter. I suggest clarifying to something like this: ``` // name + "/" entry found, keep looking for and prefer an entry which exactly matches name``` - PR: https://git.openjdk.java.net/jdk/pull/6342
Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File
On Thu, 11 Nov 2021 11:51:31 GMT, Lance Andersen wrote:
>> src/java.base/share/classes/java/util/zip/ZipFile.java line 1642:
>>
>>> 1640: && entry.startsWith(name) &&
>>> 1641: entry.charAt(entryLen - 1) == '/') {
>>> 1642: // Now check for a match with a trailing
>>> slash
>>
>> Hello Lance,
>> Is this a typo in that comment? Should it instead say "... without a
>> trailing slash"?
>
> Hi Jaikiran,
>
> The comment is correct
Thank you for that clarification. The "addSlash" param being "false" in the
call below that comment is what made me think that the comment had a typo. I
read that code in a bit more detail now and I see what that comment means. It
essentially says that it's now issuing a call to find a entry position (if it
exists) for the name ending with a "/" character.
-
PR: https://git.openjdk.java.net/jdk/pull/6342
Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File
On Wed, 10 Nov 2021 20:51:20 GMT, Lance Andersen wrote:
> Hi all,
>
> This patch addresses a regression introduced in JDK 15 via JDK-8242959 where
> you can no longer access a file entry contained within a Zip file when there
> is also a directory entry with the same name via ZipFile:getEntry().
>
> Once fixed, the behavior will be consistent with earlier JDK releases.
>
> Mach5 tiers 1-3 have been run without failure
>
> Best
> Lance
test/jdk/java/util/zip/ZipFile/ZipFileDuplicateEntryTest.java line 176:
> 174: zos.closeEntry();
> 175: } catch (IOException e) {
> 176: e.printStackTrace();
Did you mean to catch IOException here? If the setup fails then the test will
still run.
-
PR: https://git.openjdk.java.net/jdk/pull/6342
Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File
On Thu, 11 Nov 2021 02:14:50 GMT, Jaikiran Pai wrote:
>> Hi all,
>>
>> This patch addresses a regression introduced in JDK 15 via JDK-8242959 where
>> you can no longer access a file entry contained within a Zip file when there
>> is also a directory entry with the same name via ZipFile:getEntry().
>>
>> Once fixed, the behavior will be consistent with earlier JDK releases.
>>
>> Mach5 tiers 1-3 have been run without failure
>>
>> Best
>> Lance
>
> src/java.base/share/classes/java/util/zip/ZipFile.java line 1642:
>
>> 1640: && entry.startsWith(name) &&
>> 1641: entry.charAt(entryLen - 1) == '/') {
>> 1642: // Now check for a match with a trailing
>> slash
>
> Hello Lance,
> Is this a typo in that comment? Should it instead say "... without a trailing
> slash"?
Hi Jaikiran,
The comment is correct
-
PR: https://git.openjdk.java.net/jdk/pull/6342
Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File
On Wed, 10 Nov 2021 20:51:20 GMT, Lance Andersen wrote:
> Hi all,
>
> This patch addresses a regression introduced in JDK 15 via JDK-8242959 where
> you can no longer access a file entry contained within a Zip file when there
> is also a directory entry with the same name via ZipFile:getEntry().
>
> Once fixed, the behavior will be consistent with earlier JDK releases.
>
> Mach5 tiers 1-3 have been run without failure
>
> Best
> Lance
src/java.base/share/classes/java/util/zip/ZipFile.java line 1642:
> 1640: && entry.startsWith(name) &&
> 1641: entry.charAt(entryLen - 1) == '/') {
> 1642: // Now check for a match with a trailing
> slash
Hello Lance,
Is this a typo in that comment? Should it instead say "... without a trailing
slash"?
-
PR: https://git.openjdk.java.net/jdk/pull/6342
Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File
On Wed, 10 Nov 2021 21:32:32 GMT, Claes Redestad wrote: >> Hi all, >> >> This patch addresses a regression introduced in JDK 15 via JDK-8242959 where >> you can no longer access a file entry contained within a Zip file when there >> is also a directory entry with the same name via ZipFile:getEntry(). >> >> Once fixed, the behavior will be consistent with earlier JDK releases. >> >> Mach5 tiers 1-3 have been run without failure >> >> Best >> Lance > > test/jdk/java/util/zip/ZipFile/ZipFileDuplicateEntryTest.java line 515: > >> 513: } >> 514: >> 515: /** > > Left-over? Thank you for the quick review Claes! I was going to remove this method but chose to leave it in to make it a bit more straight forward to port to JDK 8 (which I have run the test on as well). No strong preferences, just figured leaving it there might make it slightly easier for a back port. - PR: https://git.openjdk.java.net/jdk/pull/6342
Re: RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File
On Wed, 10 Nov 2021 20:51:20 GMT, Lance Andersen wrote: > Hi all, > > This patch addresses a regression introduced in JDK 15 via JDK-8242959 where > you can no longer access a file entry contained within a Zip file when there > is also a directory entry with the same name via ZipFile:getEntry(). > > Once fixed, the behavior will be consistent with earlier JDK releases. > > Mach5 tiers 1-3 have been run without failure > > Best > Lance LGTM! test/jdk/java/util/zip/ZipFile/ZipFileDuplicateEntryTest.java line 515: > 513: } > 514: > 515: /** Left-over? - Marked as reviewed by redestad (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6342
RFR: 8276123: ZipFile::getEntry will not return a file entry when there is a directory entry of the same name within a Zip File
Hi all, This patch addresses a regression introduced in JDK 15 via JDK-8242959 where you can no longer access a file entry contained within a Zip file when there is also a directory entry with the same name via ZipFile:getEntry(). Once fixed, the behavior will be consistent with earlier JDK releases. Mach5 tiers 1-3 have been run without failure Best Lance - Commit messages: - Address javadoc typo - Fix for JDK-8276123 Changes: https://git.openjdk.java.net/jdk/pull/6342/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6342=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8276123 Stats: 598 lines in 2 files changed: 593 ins; 3 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6342.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6342/head:pull/6342 PR: https://git.openjdk.java.net/jdk/pull/6342
