RFR: 8209137: Add ability to bind to specific local address to HTTP client
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. - Commit messages: - 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 - first round of security manager checks - use IPSupport in test and don't rely on ipv4/ipv6 being available - use correct IPv6 "any address" - Refactor the test to use the HttpServerAdapters test infrastructure - increase the jtreg maxOutputSize for java/net/httpclient tests - Enhance HttpServerAdapters test infrastructure to allow returning client remote address from the (test) exchanges - print the stacktrace, to aid debugging, from Http2TestServer when server fails to start - ... and 4 more: https://git.openjdk.java.net/jdk/compare/aed3ea20...5ab5a61a Changes: https://git.openjdk.java.net/jdk/pull/6690/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6690&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8209137 Stats: 427 lines in 8 files changed: 425 ins; 0 del; 2 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: 8209137: Add ability to bind to specific local address to HTTP client
On Fri, 3 Dec 2021 06:15:31 GMT, Jaikiran Pai wrote:
> 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.
Generally it would be good if the test did not make any assumption on the
presence - or absence - of either IPv4 or IPv6 on the tested machine.
The IPSupport test library class has methods that allows a test to inquire
whether IPv6 or IPv4 are available - I would recommend using them to figure out
which test cases can be tested on the machine the test runs on.
test/jdk/java/net/httpclient/HttpClientLocalAddrTest.java line 74:
> 72:
> 73: @BeforeClass
> 74: public static void beforeClass() throws Exception {
For HttpClient tests - unless they are specific to HTTP/1.1 I'd recommend
making the test class implement HttpServerAdapters and test will all versions
of the protocol (http, https, http/2, and https/2).
An example can be found in e.g.
`test/jdk/java/net/httpclient/CancelRequestTest.java`
test/jdk/java/net/httpclient/HttpClientLocalAddrTest.java line 205:
> 203: // anyAddress
> 204: if (Boolean.getBoolean("java.net.preferIPv6Addresses")) {
> 205: // ipv6 wildcard
::1 is the ipv6 loopback - not the ipv6 wildcard
-
PR: https://git.openjdk.java.net/jdk/pull/6690
Re: RFR: 8209137: Add ability to bind to specific local address to HTTP client
On Fri, 3 Dec 2021 17:29:44 GMT, Daniel Fuchs wrote:
>> test/jdk/java/net/httpclient/HttpClientLocalAddrTest.java line 74:
>>
>>> 72:
>>> 73: @BeforeClass
>>> 74: public static void beforeClass() throws Exception {
>>
>> For HttpClient tests - unless they are specific to HTTP/1.1 I'd recommend
>> making the test class implement HttpServerAdapters and test will all
>> versions of the protocol (http, https, http/2, and https/2).
>> An example can be found in e.g.
>> `test/jdk/java/net/httpclient/CancelRequestTest.java`
>
> Also it's better if the test is IP version agnostic.
Hello Daniel, I will update the test accordingly in the next round of updates.
-
PR: https://git.openjdk.java.net/jdk/pull/6690
Re: RFR: 8209137: Add ability to bind to specific local address to HTTP client
On Fri, 3 Dec 2021 17:28:25 GMT, Daniel Fuchs wrote:
>> 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.
>
> test/jdk/java/net/httpclient/HttpClientLocalAddrTest.java line 74:
>
>> 72:
>> 73: @BeforeClass
>> 74: public static void beforeClass() throws Exception {
>
> For HttpClient tests - unless they are specific to HTTP/1.1 I'd recommend
> making the test class implement HttpServerAdapters and test will all versions
> of the protocol (http, https, http/2, and https/2).
> An example can be found in e.g.
> `test/jdk/java/net/httpclient/CancelRequestTest.java`
Also it's better if the test is IP version agnostic.
> test/jdk/java/net/httpclient/HttpClientLocalAddrTest.java line 205:
>
>> 203: // anyAddress
>> 204: if (Boolean.getBoolean("java.net.preferIPv6Addresses")) {
>> 205: // ipv6 wildcard
>
> ::1 is the ipv6 loopback - not the ipv6 wildcard
You may encounter some issues when testing with https and IPv6 - Michael is
working on a fix to add the loopback addresses as SNI aliases to the
SimpleSSLContext certificate:
https://bugs.openjdk.java.net/browse/JDK-8278312
-
PR: https://git.openjdk.java.net/jdk/pull/6690
Re: RFR: 8209137: Add ability to bind to specific local address to HTTP client
On Tue, 7 Dec 2021 14:33:40 GMT, Daniel Fuchs wrote:
>> test/jdk/java/net/httpclient/HttpClientLocalAddrTest.java line 205:
>>
>>> 203: // anyAddress
>>> 204: if (Boolean.getBoolean("java.net.preferIPv6Addresses")) {
>>> 205: // ipv6 wildcard
>>
>> ::1 is the ipv6 loopback - not the ipv6 wildcard
>
> You may encounter some issues when testing with https and IPv6 - Michael is
> working on a fix to add the loopback addresses as SNI aliases to the
> SimpleSSLContext certificate:
> https://bugs.openjdk.java.net/browse/JDK-8278312
You could also replace `Boolean.getBoolean("java.net.preferIPv6Addresses")`
with a call to `IPSupport::preferIPv6Addresses()`
-
PR: https://git.openjdk.java.net/jdk/pull/6690
Re: RFR: 8209137: Add ability to bind to specific local address to HTTP client
On Wed, 8 Dec 2021 01:22:50 GMT, Jaikiran Pai wrote:
>> You could also replace `Boolean.getBoolean("java.net.preferIPv6Addresses")`
>> with a call to `IPSupport::preferIPv6Addresses()`
>
>> ::1 is the ipv6 loopback - not the ipv6 wildcard
>
> Oops, that's embarrassing and it isn't even a copy/paste mistake. I have been
> in this part of the test so many times while cleaning up/debugging and yet
> didn't notice this. Fixed it in the latest PR update now.
>
> I'll update the rest of this test with your suggestions shortly. Thank you
> Daniel for the reviews.
>
>
>
>>
> You may encounter some issues when testing with https and IPv6 - Michael is
> working on a fix to add the loopback addresses as SNI aliases to the
> SimpleSSLContext certificate: https://bugs.openjdk.java.net/browse/JDK-8278312
I had a look at this one. Turns out my test wasn't impacted by it because the
test ends up using a hostname based URLs (`localhost`) for requests and the
issue only impacts requests that use IP addresses for the requests (where the
server then presents a hostname based cert causing the cert validation
failure.).
-
PR: https://git.openjdk.java.net/jdk/pull/6690
Re: RFR: 8209137: Add ability to bind to specific local address to HTTP client
On Tue, 7 Dec 2021 14:35:27 GMT, Daniel Fuchs wrote:
>> You may encounter some issues when testing with https and IPv6 - Michael is
>> working on a fix to add the loopback addresses as SNI aliases to the
>> SimpleSSLContext certificate:
>> https://bugs.openjdk.java.net/browse/JDK-8278312
>
> You could also replace `Boolean.getBoolean("java.net.preferIPv6Addresses")`
> with a call to `IPSupport::preferIPv6Addresses()`
> ::1 is the ipv6 loopback - not the ipv6 wildcard
Oops, that's embarrassing and it isn't even a copy/paste mistake. I have been
in this part of the test so many times while cleaning up/debugging and yet
didn't notice this. Fixed it in the latest PR update now.
I'll update the rest of this test with your suggestions shortly. Thank you
Daniel for the reviews.
>
-
PR: https://git.openjdk.java.net/jdk/pull/6690
RFR: Merge jdk18
Forwardport JDK 18 -> JDK 19 - Commit messages: - Merge - 8277621: ARM32: multiple fastdebug failures with "bad AD file" after JDK-8276162 - 8278538: Test langtools/jdk/javadoc/tool/CheckManPageOptions.java fails after the manpage was updated - 8273179: Update nroff pages in JDK 18 before RC The merge commit only contains trivial merges, so no merge-specific webrevs have been generated. Changes: https://git.openjdk.java.net/jdk/pull/6802/files Stats: 1142 lines in 30 files changed: 688 ins; 155 del; 299 mod Patch: https://git.openjdk.java.net/jdk/pull/6802.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6802/head:pull/6802 PR: https://git.openjdk.java.net/jdk/pull/6802
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v5]
> 8274809: Update java.base classes to use try-with-resources
Andrey Turbanov has updated the pull request incrementally with one additional
commit since the last revision:
8274809: Update java.base classes to use try-with-resources
move { to previous line
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/5818/files
- new: https://git.openjdk.java.net/jdk/pull/5818/files/3b05ec49..033ee3c2
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5818&range=04
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5818&range=03-04
Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod
Patch: https://git.openjdk.java.net/jdk/pull/5818.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5818/head:pull/5818
PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v4]
On Tue, 7 Dec 2021 07:59:54 GMT, Alan Bateman wrote:
>> Andrey Turbanov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8274809: Update java.base classes to use try-with-resources
>> fix review comments
>
> src/java.base/share/classes/sun/net/NetProperties.java line 73:
>
>> 71: try (FileInputStream in = new FileInputStream(fname);
>> 72: BufferedInputStream bin = new BufferedInputStream(in))
>> 73: {
>
> Style-wisee I'd probably put the "{" at the end of L72.
As you wish.
But I personally like to have `{` on separate line, in case of multi-line
`try`. It makes clear when `try` header is actually finished and body starts.
-
PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v4]
On Tue, 7 Dec 2021 14:48:17 GMT, Daniel Fuchs wrote: >Remind me to sponsor this after the fork. fork is done :) - PR: https://git.openjdk.java.net/jdk/pull/5818
Re: RFR: 8274809: Update java.base classes to use try-with-resources [v4]
On Thu, 9 Dec 2021 01:25:20 GMT, Bradford Wetmore wrote: >> That's one of the places where I personally would always use the var keyword >> too: it makes the line shorter and the type is already clearly visible in >> the RHS of the assignment, so repeating it on the left hand side does not >> bring much value... > > Just interesting that the style wasn't consistent. Both ways are fine by me. I used `var` only in places, where it allows to reduce line length to be less than 80. - PR: https://git.openjdk.java.net/jdk/pull/5818
Integrated: Merge jdk18
On Fri, 10 Dec 2021 17:51:31 GMT, Jesper Wilhelmsson wrote: > Forwardport JDK 18 -> JDK 19 This pull request has now been integrated. Changeset: 61736f81 Author:Jesper Wilhelmsson URL: https://git.openjdk.java.net/jdk/commit/61736f81fb4a20375c83d59e2b37a00aafb11107 Stats: 1142 lines in 30 files changed: 688 ins; 155 del; 299 mod Merge - PR: https://git.openjdk.java.net/jdk/pull/6802
