Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
On Fri, 27 May 2022 18:40:32 GMT, XenoAmess wrote: >> as title. > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > do it as naotoj said `java.io` and `java.nio` look all right. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
On Fri, 27 May 2022 18:40:32 GMT, XenoAmess wrote:
>> as title.
>
> XenoAmess has updated the pull request incrementally with one additional
> commit since the last revision:
>
> do it as naotoj said
Reviewed i18n-related changes and they look good. One minor suggestion in
`Calendar`, but that can be applied later.
src/java.base/share/classes/java/util/Calendar.java line 2648:
> 2646: set.add("gregory");
> 2647: set.add("buddhist");
> 2648: set.add("japanese");
This can be replaced with `SET = Set.of("gregory", "buddhist", "japanese");`.
-
Marked as reviewed by naoto (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8302
Integrated: 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication
On Sat, 30 Apr 2022 10:17:43 GMT, Andrey Turbanov wrote: > `AuthenticationInfo.requestAuthentication` uses separate `HashMap`'s `get` > +`put` calls. > > https://github.com/openjdk/jdk/blob/176bb23de18d9ab448e77e85a9c965a7c02f2c50/src/java.base/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java#L155-L165 > > Instead we can use the `HashMap.putIfAbsent` to make code a bit easier to > follow. We know that `requests` can contain only non-null values. This pull request has now been integrated. Changeset: 4caf1ef3 Author:Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/4caf1ef389fd02bf53a9b7ed33d3b57fdaa79bd2 Stats: 12 lines in 1 file changed: 0 ins; 6 del; 6 mod 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication Reviewed-by: dfuchs, jpai - PR: https://git.openjdk.java.net/jdk/pull/8484
Re: RFR: 8284780: Need methods to create pre-sized HashSet and LinkedHashSet [v17]
On Fri, 27 May 2022 18:40:32 GMT, XenoAmess wrote: >> as title. > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > do it as naotoj said Reviewers for i18n, net, nio, and security, please review call site changes in your areas. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/8302
Re: RFR: 8287237: (fs) Files.probeContentType returns null if filename contains hash mark on Linux [v4]
On Wed, 1 Jun 2022 07:28:11 GMT, Vyom Tewari wrote: > Looks ok, i tested on centos 7 and it is working as expected. To really verify it you would have to suppress `~/.mime.types` and `/etc/mime.types` before running the test. - PR: https://git.openjdk.java.net/jdk/pull/8909
Integrated: 8287237: (fs) Files.probeContentType returns null if filename contains hash mark on Linux
On Thu, 26 May 2022 23:03:05 GMT, Brian Burkhalter wrote: > Modify `sun.net.www.MimeTable.findByFileName(String)` to attempt to find the > file extension in the entire file name if it is not found in the portion of > the name preceding the optional fragment beginning with a hash (`#`). This pull request has now been integrated. Changeset: 8071b231 Author:Brian Burkhalter URL: https://git.openjdk.java.net/jdk/commit/8071b2311caaacd714d74f12aee6cb7c2fe700fa Stats: 79 lines in 2 files changed: 53 ins; 15 del; 11 mod 8287237: (fs) Files.probeContentType returns null if filename contains hash mark on Linux Reviewed-by: rriggs, jpai, vtewari - PR: https://git.openjdk.java.net/jdk/pull/8909
Re: RFR: 8272702: Resolving URI relative path with no / may lead to incorrect toString
On Thu, 26 May 2022 09:18:56 GMT, KIRIYAMA Takuya wrote:
> Consider an authority component without trailing "/" as a base URI. When
> resolving a relative path against this base URI, the resulting URI is a
> concatenated URI without "/".
> This behaviour should be fixed, which is rationalized by
> rfc3986#section-5.2.3.
> Could you review this fix?
Changes requested by dfuchs (Reviewer).
src/java.base/share/classes/java/net/URI.java line 2140:
> 2138: } else {
> 2139: sb.append("/");
> 2140: }
This is wrong as it will cause `URI.create("foo").resolve(URI.create("test"))`
to return `"/test"` instead of `"test"`
-
PR: https://git.openjdk.java.net/jdk/pull/8899
Re: RFR: 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication [v2]
> `AuthenticationInfo.requestAuthentication` uses separate `HashMap`'s `get` > +`put` calls. > > https://github.com/openjdk/jdk/blob/176bb23de18d9ab448e77e85a9c965a7c02f2c50/src/java.base/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java#L155-L165 > > Instead we can use the `HashMap.putIfAbsent` to make code a bit easier to > follow. We know that `requests` can contain only non-null values. Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication remove obvious assert - Changes: - all: https://git.openjdk.java.net/jdk/pull/8484/files - new: https://git.openjdk.java.net/jdk/pull/8484/files/b493aab1..f850d61f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8484=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8484=00-01 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8484.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8484/head:pull/8484 PR: https://git.openjdk.java.net/jdk/pull/8484
Re: RFR: 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication [v2]
On Wed, 1 Jun 2022 13:27:27 GMT, Andrey Turbanov wrote:
>> src/java.base/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java
>> line 159:
>>
>>> 157: if (t == null || t == c) {
>>> 158: assert cached == null;
>>> 159: return cached;
>>
>> Hello Andrey, while you are in this code, I think changing these 2 lines:
>>
>>
>> assert cached == null;
>> return cached;
>>
>> to just:
>>
>>
>> return null;
>>
>> would be better. There's already a `if (cached != null) return cached;`
>> code, a few lines above and after that line there's no other modifications
>> to this `cached` local variable, so changing this line to just return null
>> would remove any confusion while reading this code.
>
> Good idea. Updated.
Thank you for that change. The changes in this PR looks fine to me.
-
PR: https://git.openjdk.java.net/jdk/pull/8484
Re: RFR: 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication [v2]
On Wed, 1 Jun 2022 13:32:28 GMT, Andrey Turbanov wrote: >> `AuthenticationInfo.requestAuthentication` uses separate `HashMap`'s `get` >> +`put` calls. >> >> https://github.com/openjdk/jdk/blob/176bb23de18d9ab448e77e85a9c965a7c02f2c50/src/java.base/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java#L155-L165 >> >> Instead we can use the `HashMap.putIfAbsent` to make code a bit easier to >> follow. We know that `requests` can contain only non-null values. > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication > remove obvious assert Marked as reviewed by jpai (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8484
Re: RFR: 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication [v2]
On Wed, 1 Jun 2022 04:08:53 GMT, Jaikiran Pai wrote:
>> Andrey Turbanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication
>> remove obvious assert
>
> src/java.base/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java
> line 159:
>
>> 157: if (t == null || t == c) {
>> 158: assert cached == null;
>> 159: return cached;
>
> Hello Andrey, while you are in this code, I think changing these 2 lines:
>
>
> assert cached == null;
> return cached;
>
> to just:
>
>
> return null;
>
> would be better. There's already a `if (cached != null) return cached;` code,
> a few lines above and after that line there's no other modifications to this
> `cached` local variable, so changing this line to just return null would
> remove any confusion while reading this code.
Good idea. Updated.
-
PR: https://git.openjdk.java.net/jdk/pull/8484
Re: RFR: 8287390: Cleanup Map usage in AuthenticationInfo.requestAuthentication
On Sat, 30 Apr 2022 10:17:43 GMT, Andrey Turbanov wrote: > `AuthenticationInfo.requestAuthentication` uses separate `HashMap`'s `get` > +`put` calls. > > https://github.com/openjdk/jdk/blob/176bb23de18d9ab448e77e85a9c965a7c02f2c50/src/java.base/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java#L155-L165 > > Instead we can use the `HashMap.putIfAbsent` to make code a bit easier to > follow. We know that `requests` can contain only non-null values. LGTM - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8484
Re: RFR: 8287237: (fs) Files.probeContentType returns null if filename contains hash mark on Linux [v4]
On Tue, 31 May 2022 18:29:30 GMT, Brian Burkhalter wrote: >> Modify `sun.net.www.MimeTable.findByFileName(String)` to attempt to find the >> file extension in the entire file name if it is not found in the portion of >> the name preceding the optional fragment beginning with a hash (`#`). > > Brian Burkhalter has updated the pull request incrementally with one > additional commit since the last revision: > > 8287237: Simplify code a bit Looks ok, i tested on centos 7 and it is working as expected. - Marked as reviewed by vtewari (Committer). PR: https://git.openjdk.java.net/jdk/pull/8909
