Re: Suggestion for the HTTP client: allow selection of a local address
Klaus I've created an RFE at https://bugs.openjdk.java.net/browse/JDK-8209137 to track this. I imagine the simplest solution would be to be able to specify the local address when constructing a HttpClient, and then all connections created using that client would be bound to the chosen address. Michael. On 08/08/2018, 15:43, Klaus Malorny wrote: Hi, I just discussed with a colleague that we dreadfully miss the ability in the old HttpURLConnection to set the address to which the client locally binds, which we require to get through the firewalls of our business partners when connecting from a multihomed system. This repeatedly caused us to use third-party libraries, which are powerful on the one hand, but cumbersome on the other. With the new HTTP2 client in mind, I wanted to tell him, wait, with Java 9+, it will be better. But I checked first and discovered that there is still no way to do so. So, when you eventually revise the API, it would be nice if you could consider the addition of such a feature. Regards, Klaus
Suggestion for the HTTP client: allow selection of a local address
Hi, I just discussed with a colleague that we dreadfully miss the ability in the old HttpURLConnection to set the address to which the client locally binds, which we require to get through the firewalls of our business partners when connecting from a multihomed system. This repeatedly caused us to use third-party libraries, which are powerful on the one hand, but cumbersome on the other. With the new HTTP2 client in mind, I wanted to tell him, wait, with Java 9+, it will be better. But I checked first and discovered that there is still no way to do so. So, when you eventually revise the API, it would be nice if you could consider the addition of such a feature. Regards, Klaus
Re: Priority can be set for HttpClient with HTTP/1.1
Hi Nezih, The client’s version is a default preferred version for requests that do not specify an explicit version. As such, it does not prevent requests, attempting to negotiate HTTP/2, from being sent, just that it is not the default. Given that, then the priority value is still be applicable to client’s build with a preferred HTTP/1.1 version. -Chris. > On 1 Aug 2018, at 10:56, nezih yigitbasi wrote: > > Hi, > Priority is only defined for version HTTP_2 (and that's clear from the > javadoc). However, it's possible to construct an instance of HttpClient with > a priority value and with version HTTP_1_1 (tested with Java "9.0.1"). I feel > that it will be less error prone for the users if conflicting combinations of > arguments weren't allowed. It's also confusing when I read the below code as > it doesn't make sense to set the priority for HTTP/1.1. What do you think? > > HttpClient client = HttpClient.newBuilder() > .executor(httpClientExecutor) > .priority(32) > .version(HTTP_1_1) > .build(); > > Thanks, > Nezih
Re: [httpclient] HTTP2: Memory Leak with Proxy
Chris, that's good to hear, I'll try it out. Best, Albert On 07.08.2018 18:53, Chris Hegarty wrote: Albert, I haven’t yet looked at what happens in JDK 10, but just to say, since things have moved on a lot in JDK 11 EA, that the same test runs with a reasonable amount of memory and CPU with JDK 11 EA [1]. I updated the test a little, since some the names have been changed with the standardisation ( see javadoc for more details [2] ). -Chris. [1] http://jdk.java.net/11/ [2] https://download.java.net/java/early_access/jdk11/docs/api/java.net.http/java/net/http/package-summary.html --- $ cat BadProxyLeak.java import java.net.InetSocketAddress; import java.net.ProxySelector; import java.net.URI; import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.util.concurrent.TimeUnit; public class BadProxyLeak { public static void main(String[] args) throws InterruptedException { // 1st heap dump //Thread.sleep(1); { HttpClient client = HttpClient.newBuilder() // malicious proxy? .proxy(ProxySelector.of(new InetSocketAddress("165.165.248.90", 8080))) .build(); HttpRequest req = HttpRequest // target is not relevant .newBuilder(URI.create("https://www.google.de";)) // leak occurs only with HTTP_2 .version(HttpClient.Version.HTTP_2) //.version(HttpClient.Version.HTTP_1_1) .GET() .build(); // use same client for every request // most likely happens on the first retry, if not try again or increase i for (int i = 0; i < 10; i++) { System.out.println("Request " + i); // body handler is not relevant HttpResponse.BodyHandler t = HttpResponse.BodyHandlers.ofString(); try { // happens with both async and sync send client.sendAsync(req, t).get(15, TimeUnit.SECONDS); //client.send(req, handler); } catch (Exception e) { e.printStackTrace(); } } } // 3rd heap dump System.out.println("Generating heap dump manually"); // space is not released Thread.sleep(6); } } On 7 Aug 2018, at 17:05, Chris Hegarty wrote: On 7 Aug 2018, at 16:55, Albert Schimpf wrote: Hi, by bad I mean that this proxy is the only one out of ~1000 proxies which causes this behavior. It's also the only one which causes SSLExceptions (General SSL engine problem) and EOFExceptions. I don't think that particular proxy is a valid proxy. Using curl indicates that this is a SSL handshake problem (https): * Rebuilt URL to: www.google.de/ * Trying 165.165.248.90… Oh, you mean the proxy at that actual IP address. Ok got it. * TCP_NODELAY set * Connected to 165.165.248.90 (165.165.248.90) port 8080 (#0) * successfully set certificate verify locations: * CAfile: /etc/ssl/certs/ca-certificates.crt CApath: /etc/ssl/certs * TLSv1.2 (OUT), TLS handshake, Client hello (1): * OpenSSL SSL_connect: SSL_ERROR_SYSCALL in connection to 165.165.248.90:8080 * Closing connection 0 curl: (35) OpenSSL SSL_connect: SSL_ERROR_SYSCALL in connection to 165.165.248.90:8080 I don't know that much about the protocol itself, so I don't think I can help very much. Thank you for looking into this! -Chris. Best, Albert On 07.08.2018 12:05, Chris Hegarty wrote: Hi Albert, Very strange indeed. Thanks for reporting it, I’ll investigate. What do your mean by “bad proxy”. What is bad about it, and how does it behave? -Chris. On 7 Aug 2018, at 10:25, Albert Schimpf wrote: Hi, I stumbled upon some strange behavior when using the new Java httpclient. The issue is very simple to reproduce. Send a GET request via a known bad proxy: HttpClient client = HttpClient.newBuilder() .proxy(ProxySelector.of(BAD_PROXY)) .build(); HttpRequest req = HttpRequest // target is not relevant .newBuilder(...) .GET() .build(); // body handler is not relevant HttpResponse.BodyHandler t = HttpResponse.BodyHandler.asString(); // happens with both async and sync send client.sendAsync(req, t).get(30, TimeUnit.SECONDS); The result is that the heap size increases dramatically (to about 1.5GB) and resources are not released. CPU consumption increases by a constant factor, too. I have tried many variations of the above code, and the only thing which seems to work (i.e. heap size does not explode) is to set the HTTP version to 1.1. In my main application this leads to both memory and CPU starvation (4GB memory limit, 100% CPU usage). It us
Re: RFR [11] 8208391: Need to differentiate response and connect timeouts in HTTP Client API
The updated webrev looks fine to me, Chris. Thanks, Michael On 07/08/2018, 10:52, Chris Hegarty wrote: On 4 Aug 2018, at 14:08, Jaikiran Pai wrote: ... Do you think this can be reworded a bit? Although I understand what's being said here, the wording doesn't seem right. Maybe something like: * In the case where a new connection needs to be established, if * the connection cannot be established within the given {@code * duration}, then {@link HttpClient#send(java.net.http.HttpRequest, * java.net.http.HttpResponse.BodyHandler) HttpClient::send} throws a * {@link HttpConnectTimeoutException}, or * {@link HttpClient#sendAsync(java.net.http.HttpRequest, * java.net.http.HttpResponse.BodyHandler) HttpClient::sendAsync} * completes exceptionally with an {@code HttpConnectTimeoutException}. Agreed. This wording avoids the previous awkwardness. src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java -this.timeout = null; +this.timeout = Duration.ofSeconds(30); Is this an intentional change of default value for the timeout? Is that something that needs to be documented? Accidental, left over from a previous hacking session. Removed. Updated webrev: http://cr.openjdk.java.net/~chegar/8208391/webrev.01/ One other thing - maybe not directly related to this single patch, but as you are aware, recently as part of [1], a new system property (and a security property) was introduced to optionally include host info in the exception messages thrown for socket exceptions I am aware of this. . Does the HttpClient honour that system property in the exceptions it throws? The HTPT Client does not have any special handing for this property. It may not be necessary, at least not for low-level NIO exceptions, once the exception, or its cause, is preserved. I don't see it being used in this patch for the timeout exceptions. I haven't checked the code, outside of this patch, to see if it's dealt with in other parts of this API. Separately, I will look into the possibility of where such an extension can be used. -Chris.