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]

2021-11-12 Thread Alan Bateman
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]

2021-11-11 Thread Claes Redestad
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]

2021-11-11 Thread Lance Andersen
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]

2021-11-11 Thread Lance Andersen
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]

2021-11-11 Thread Lance Andersen
> 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