Re: RFR: 8231632: HttpURLConnection::usingProxy could specify that it may lazily evaluate the fact

2019-10-18 Thread Julia Boes

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

2019-10-18 Thread Chris Hegarty


> 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

2019-10-18 Thread Julia Boes



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

2019-10-18 Thread Chris Hegarty



> 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

2019-10-18 Thread Julia Boes

Thanks all!

Here's CSR for the fix: https://bugs.openjdk.java.net/browse/JDK-8232600



java.net.http.HttpClient Redirect Policy

2019-10-18 Thread Elliot Barlas
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

2019-10-18 Thread Daniel Fuchs

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