Re: RFR: 8231632: HttpURLConnection::usingProxy could specify that it may lazily evaluate the fact
Hi, Chris and Daniel provided some off-list feedback regarding the ProxySelector::select call. It returns a list of proxies, which could be dynamic. Each call can potentially return a different list of proxies so it's not desirable to make that call twice. An alternative is to maintain a class field that is set to true in the connect method, as done in the updated webrev. Webrev: http://cr.openjdk.java.net/~jboes/webrevs/8231632/webrev.01/index.html Regards, Julia On 17/10/2019 10:21, Julia Boes wrote: Hi, This fix involves HttpURLConnection::usingProxy. Previously, the method would return true only once the connection was established (once the underlying socket was connected) and would switch back to false when as the input stream was fully read. The new implementation checks whether an explicit proxy is set and if not, it invokes the ProxySelector to determine if a proxy is used. This widens the window in which the method returns a meaningful value. This is a best-effort as it might not always be able to determine if a proxy is actually used before the connection is established, as the ProxySelector might return different results when connect is called. An apinote is added to clarify the behaviour. Bug: https://bugs.openjdk.java.net/browse/JDK-8231632 Webrev: http://cr.openjdk.java.net/~jboes/webrevs/8231632/webrev.00/index.html I'll create a CSR for this once I gathered some feedback on this solution. The fix was tested with JCK java_net and tier 1 - 3. Regards, Julia
Re: RFR: 8231632: HttpURLConnection::usingProxy could specify that it may lazily evaluate the fact
> On 18 Oct 2019, at 10:02, Julia Boes wrote: > > Hi, > > Chris and Daniel provided some off-list feedback regarding the > ProxySelector::select call. It returns a list of proxies, which could be > dynamic. Each call can potentially return a different list of proxies so it's > not desirable to make that call twice. An alternative is to maintain a class > field that is set to true in the connect method, as done in the updated > webrev. > > Webrev: http://cr.openjdk.java.net/~jboes/webrevs/8231632/webrev.01/index.html Thanks Julia, this looks good. Just a few minor comments. 1) Maybe add @Override to `public boolean usingProxy()`. ( pre-existing issue ) 2) Maybe move the return on 3040 to a new line. Also use the curly braces consistently, or not at all ;-) 3039 public boolean usingProxy() { 3040 if (usingProxy || usingProxyInternal()) return true; 3041 3042 if (instProxy != null) { 3043 return instProxy.type().equals(Proxy.Type.HTTP); 3044 } 3045 return false; 3046 } -Chris.
Re: RFR: 8231632: HttpURLConnection::usingProxy could specify that it may lazily evaluate the fact
1) Maybe add @Override to `public boolean usingProxy()`. ( pre-existing issue ) 2) Maybe move the return on 3040 to a new line. Also use the curly braces consistently, or not at all ;-) That's right! Thanks for reviewing, Chris. I added those two changes. Updated webrev: http://cr.openjdk.java.net/~jboes/webrevs/8231632/webrev.02/index.html
Re: RFR: 8231632: HttpURLConnection::usingProxy could specify that it may lazily evaluate the fact
> On 18 Oct 2019, at 15:30, Julia Boes wrote: > > > 1) Maybe add @Override to `public boolean usingProxy()`. ( pre-existing issue > ) > > 2) Maybe move the return on 3040 to a new line. Also use the curly braces > consistently, or not at all ;-) > > That's right! Thanks for reviewing, Chris. I added those two changes. > > Updated webrev: > http://cr.openjdk.java.net/~jboes/webrevs/8231632/webrev.02/index.html LGTM. -Chris.
Re: RFR: 8231632: HttpURLConnection::usingProxy could specify that it may lazily evaluate the fact
Thanks all! Here's CSR for the fix: https://bugs.openjdk.java.net/browse/JDK-8232600
java.net.http.HttpClient Redirect Policy
Net-dev, I'm attempting to use java.net.http.HttpClient to make a login form submission and follow a sequence of redirects. To my surprise, the HttpClient redirect internals (jdk.internal.net.http.HttpRequestImpl) seem to carry the original request body into subsequent requests. In my case, that means sending user credentials (gasp!) to the target of a redirect. Additionally, GET requests with bodies are rejected outright by the target system. Why is HttpClient behaving this way? Browsers certainly doin't do this. Am I missing a config option? - HttpClient client = HttpClient.newBuilder() .version(HttpClient.Version.HTTP_1_1) .connectTimeout(Duration.ofSeconds(5)) .cookieHandler(new CookieManager()) .followRedirects(HttpClient.Redirect.ALWAYS) .build(); String url = "..."; Map body = Map.of( "emailAddress", "...", "password", "..."); String encoded = body.entrySet().stream() .map(e -> e.getKey() + "=" + UrlEncoded.encodeString(e.getValue(), StandardCharsets.UTF_8)) .collect(Collectors.joining("&")); HttpRequest request = HttpRequest.newBuilder() .timeout(Duration.ofSeconds(5)) .uri(URI.create(url)) .header("User-Agent", "...") .header("Content-Type", "application/x-www-form-urlencoded") .POST(HttpRequest.BodyPublishers.ofString(encoded)) .build(); HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); System.out.println(response.statusCode()); System.out.println(response.headers()); System.out.println(response.body());
Re: java.net.http.HttpClient Redirect Policy
Hi Eliot, On 04/10/2019 22:13, Elliot Barlas wrote: Why is HttpClient behaving this way? Browsers certainly doin't do this. Am I missing a config option? Thanks for reporting this. I believe this is a bug - I have logged https://bugs.openjdk.java.net/browse/JDK-8232625 In the mean time - I'd suggest simply not setting any redirect policy in the HttpClient, and handling the redirect directly from your application code instead. e.g. something like: HttpResponse resp = client.send(...); if (resp.statusCode() == 303) { String location = resp.headers().firstValue("Location") .orElseThrows() resp = client.send(...) } best regards, -- daniel