Re: RFR: 8209137: Add ability to bind to specific local address to HTTP client [v7]

2022-01-25 Thread Jaikiran Pai
> This change proposes to implement the enhancement noted in 
> https://bugs.openjdk.java.net/browse/JDK-8209137.
> 
> The change introduces a new API to allow applications to build a 
> `java.net.http.HTTPClient` configured with a specific local address that will 
> be used while creating `Socket`(s) for connections.

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 21 additional commits since the 
last revision:

 - Merge latest from master branch
 - Merge latest from master branch
 - copyright year
 - By default add SocketPermission for java.net.http module to allow binding to 
non-ephemeral ports if local bind address is configured for HTTPClient
 - Merge latest master branch
 - Merge latest from master branch
 - add a security manager test to verify proper permission checks happen when 
local address is configured on HTTPClient
 - Merge latest from master branch
 - add a note to the javadoc of the new API to explain that calling 
localAddress() is only for advanced usages
 - move the security checks to the HttpClient itself instead of the builder
 - ... and 11 more: https://git.openjdk.java.net/jdk/compare/7773986d...5ecb4db4

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6690/files
  - new: https://git.openjdk.java.net/jdk/pull/6690/files/0217e1c1..5ecb4db4

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6690&range=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6690&range=05-06

  Stats: 19391 lines in 831 files changed: 12966 ins; 3096 del; 3329 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6690.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6690/head:pull/6690

PR: https://git.openjdk.java.net/jdk/pull/6690


Re: RFR: 8278263: Remove redundant synchronized from URLStreamHandler.openConnection methods

2022-01-25 Thread Florent Guillaume
On Fri, 12 Nov 2021 19:35:10 GMT, Andrey Turbanov  wrote:

> All this Handler's are stateless and there is nothing to protect via 
> synchronization.

In theory a subclass overloading using a non-synchronized `openConnection(URL 
u, Proxy p)` could have relied on `openConnection(URL u)` delegating to it and 
having been synchronized. So maybe this requires a small CSR?

-

PR: https://git.openjdk.java.net/jdk/pull/6373


Re: RFR: 8278263: Remove redundant synchronized from URLStreamHandler.openConnection methods

2022-01-25 Thread Daniel Fuchs
On Fri, 12 Nov 2021 19:35:10 GMT, Andrey Turbanov  wrote:

> All this Handler's are stateless and there is nothing to protect via 
> synchronization.

LGTM. Please integrate and drop me a note and I will sponsor it.

-

Marked as reviewed by dfuchs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6373


Integrated: 8280414: Memory leak in DefaultProxySelector

2022-01-25 Thread Daniel Jeliński
On Fri, 21 Jan 2022 08:25:30 GMT, Daniel Jeliński  wrote:

> We were leaking the list returned by `createProxyList`. Verified that the 
> leak is no longer present with this patch. Also removed a few unused 
> variables.
> 
> Reported by clang-tidy.

This pull request has now been integrated.

Changeset: fe77250f
Author:Daniel Jeliński 
Committer: Daniel Fuchs 
URL:   
https://git.openjdk.java.net/jdk/commit/fe77250fa450ec803d2818dc90c5bf156521d537
Stats: 11 lines in 1 file changed: 1 ins; 3 del; 7 mod

8280414: Memory leak in DefaultProxySelector

Reviewed-by: dfuchs

-

PR: https://git.openjdk.java.net/jdk/pull/7172


Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v8]

2022-01-25 Thread Michael Osipov
On Tue, 25 Jan 2022 12:47:26 GMT, Michael McMahon  wrote:

>> src/java.base/share/classes/sun/net/www/http/HttpClient.java line 150:
>> 
>>> 148:  * "domain:a,c.d,*.e.f" (sent to host a, or c.d or to the domain 
>>> e.f and any of its subdomains). This is
>>> 149:  * a comma separated list of arbitrary length with no white-space 
>>> allowed.
>>> 150:  * If enabled (for a particular destination) then SPNEGO 
>>> authentication requests will include
>> 
>> Previously `Negotiate` was used, now `SPNEGO`?
>
> "Negotiate" is the name of the HTTP authentication scheme which uses the 
> SPNEGO mechanism. Possibly makes more sense to refer to Negotiate here.

Yes, I know. That's the point. Clearly differentiate between GSS-API mech and 
HTTP auth scheme.

-

PR: https://git.openjdk.java.net/jdk/pull/7065


Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v8]

2022-01-25 Thread Michael McMahon
On Tue, 25 Jan 2022 11:34:57 GMT, Michael Osipov  wrote:

>> Michael McMahon has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   final review update (pre CSR)
>
> src/java.base/share/classes/sun/net/www/http/HttpClient.java line 150:
> 
>> 148:  * "domain:a,c.d,*.e.f" (sent to host a, or c.d or to the domain 
>> e.f and any of its subdomains). This is
>> 149:  * a comma separated list of arbitrary length with no white-space 
>> allowed.
>> 150:  * If enabled (for a particular destination) then SPNEGO 
>> authentication requests will include
> 
> Previously `Negotiate` was used, now `SPNEGO`?

"Negotiate" is the name of the HTTP authentication scheme which uses the SPNEGO 
mechanism. Possibly makes more sense to refer to Negotiate here.

-

PR: https://git.openjdk.java.net/jdk/pull/7065


Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v8]

2022-01-25 Thread Michael Osipov
On Tue, 25 Jan 2022 10:30:20 GMT, Michael McMahon  wrote:

>> Hi,
>> 
>> This change adds Channel Binding Token (CBT) support to HTTPS 
>> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, 
>> Kerberos) authentication scheme. When enabled, the implementation 
>> preemptively includes a CBT with authentication requests over Kerberos. The 
>> feature is enabled as follows:
>> 
>> A system property "jdk.spnego.cbt" is defined which can have the values 
>> "never" (default), which means the feature is disabled, "always", which 
>> means the CBT is included for all https Negotiate authentications, or it can 
>> take the form "domain:a,b.c,*.d.com" which is a comma separated list of 
>> domains/hosts where the feature is enabled, and disabled everywhere else. In 
>> the given example, the CBT would be included in authentication requests for 
>> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its 
>> sub-domains.
>> 
>> A test will be added separately to the implementation.
>> 
>> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
>> 
>> Thanks,
>> Michael
>
> Michael McMahon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   final review update (pre CSR)

src/java.base/share/classes/sun/net/www/http/HttpClient.java line 150:

> 148:  * "domain:a,c.d,*.e.f" (sent to host a, or c.d or to the domain e.f 
> and any of its subdomains). This is
> 149:  * a comma separated list of arbitrary length with no white-space 
> allowed.
> 150:  * If enabled (for a particular destination) then SPNEGO 
> authentication requests will include

Previously `Negotiate` was used, now `SPNEGO`?

-

PR: https://git.openjdk.java.net/jdk/pull/7065


Integrated: 8163921: HttpURLConnection default Accept header is malformed according to HTTP/1.1 RFC

2022-01-25 Thread Daniel Jeliński
On Fri, 21 Jan 2022 16:51:20 GMT, Daniel Jeliński  wrote:

> Fix RFC compliance.
> Tier1 and tier2 passed.

This pull request has now been integrated.

Changeset: 28796cbd
Author:Daniel Jeliński 
Committer: Michael McMahon 
URL:   
https://git.openjdk.java.net/jdk/commit/28796cbd1d15de678b80295418f5d1f9f59176a6
Stats: 11 lines in 2 files changed: 0 ins; 1 del; 10 mod

8163921: HttpURLConnection default Accept header is malformed according to 
HTTP/1.1 RFC

Reviewed-by: dfuchs, michaelm

-

PR: https://git.openjdk.java.net/jdk/pull/7181


Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v8]

2022-01-25 Thread Daniel Fuchs
On Tue, 25 Jan 2022 10:30:20 GMT, Michael McMahon  wrote:

>> Hi,
>> 
>> This change adds Channel Binding Token (CBT) support to HTTPS 
>> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, 
>> Kerberos) authentication scheme. When enabled, the implementation 
>> preemptively includes a CBT with authentication requests over Kerberos. The 
>> feature is enabled as follows:
>> 
>> A system property "jdk.spnego.cbt" is defined which can have the values 
>> "never" (default), which means the feature is disabled, "always", which 
>> means the CBT is included for all https Negotiate authentications, or it can 
>> take the form "domain:a,b.c,*.d.com" which is a comma separated list of 
>> domains/hosts where the feature is enabled, and disabled everywhere else. In 
>> the given example, the CBT would be included in authentication requests for 
>> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its 
>> sub-domains.
>> 
>> A test will be added separately to the implementation.
>> 
>> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
>> 
>> Thanks,
>> Michael
>
> Michael McMahon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   final review update (pre CSR)

The new version LGTM.

-

Marked as reviewed by dfuchs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7065


Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v8]

2022-01-25 Thread Michael McMahon
> Hi,
> 
> This change adds Channel Binding Token (CBT) support to HTTPS 
> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, Kerberos) 
> authentication scheme. When enabled, the implementation preemptively includes 
> a CBT with authentication requests over Kerberos. The feature is enabled as 
> follows:
> 
> A system property "jdk.spnego.cbt" is defined which can have the values 
> "never" (default), which means the feature is disabled, "always", which means 
> the CBT is included for all https Negotiate authentications, or it can take 
> the form "domain:a,b.c,*.d.com" which is a comma separated list of 
> domains/hosts where the feature is enabled, and disabled everywhere else. In 
> the given example, the CBT would be included in authentication requests for 
> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its 
> sub-domains.
> 
> A test will be added separately to the implementation.
> 
> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
> 
> Thanks,
> Michael

Michael McMahon has updated the pull request incrementally with one additional 
commit since the last revision:

  final review update (pre CSR)

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7065/files
  - new: https://git.openjdk.java.net/jdk/pull/7065/files/0d529f9d..004466ea

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7065&range=07
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7065&range=06-07

  Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7065.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7065/head:pull/7065

PR: https://git.openjdk.java.net/jdk/pull/7065