Re: RFE: Add PATCH to the list of the supported HTTP methods

[James Roper]

I don't have any opinion on the matter, but a datapoint to the popularity
of PATCH is its use in the GitHub API:

https://developer.github.com/v3/

GitHub is often looked to as a gold standard for how to write a REST API,
so if they do it, it's likely that many others do too.

On 7 March 2018 at 05:40, Chris Hegarty  wrote:

> Hi Andrej,
>
> On 02/03/18 08:28, Andrej Golovnin wrote:
>
>> Hi all,
>>
>> it would be nice if could add PATCH
>> (https://tools.ietf.org/html/rfc5789) to the list of the supported
>> ...
>> - and one for the HTTP Client branch in the JDK sandbox repository
>> which adds PATCH to the HttpRequest builder:
>>java_net_http_HttpRequest_PATCH_method.diff
>>
>
> The JDK HTTP Client has:
>   `HttpRequest.Builder::method(String method, BodyPublisher publisher)`
>
>  ,so it is currently possible to use the `PATCH` method.
>
> Is `PATCH` sufficiently popular to warrant its own self-named
> method in the request builder?
>
> -Chris.
>



-- 
*James Roper*
*Senior Octonaut*

Lightbend  – Build reactive apps!
Twitter: @jroper 

Re: RFR [11] 8198358 : Align organization of DualStackPlainSocketImpl with TwoStacksPlainSocketImp [win]

[Ivan Gerasimov]

In order to make is easier to review the fix, I made the webrev.ksh to 
generate a series of incremental webrevs from the mq patch stack.


Here's the list of the incremental changes with a brief comments:

WEBREV-000: http://cr.openjdk.java.net/~igerasim/8198358/00/000/webrev/
Only changing the order of methods declaration

WEBREV-001: http://cr.openjdk.java.net/~igerasim/8198358/00/001/webrev/
Renaming initIDs to initProto

WEBREV-002: http://cr.openjdk.java.net/~igerasim/8198358/00/002/webrev/
Changing socketBind

WEBREV-003: http://cr.openjdk.java.net/~igerasim/8198358/00/003/webrev/
Changing socketCreate

WEBREV-004: http://cr.openjdk.java.net/~igerasim/8198358/00/004/webrev/
Changing socketListen

WEBREV-005: http://cr.openjdk.java.net/~igerasim/8198358/00/005/webrev/
Changin socketAvailable

WEBREV-006: http://cr.openjdk.java.net/~igerasim/8198358/00/006/webrev/
Changing socketClose0

WEBREV-007: http://cr.openjdk.java.net/~igerasim/8198358/00/007/webrev/
Changing socketShutdown

WEBREV-008: http://cr.openjdk.java.net/~igerasim/8198358/00/008/webrev/
Changing socketSendUrgentData

WEBREV-009: http://cr.openjdk.java.net/~igerasim/8198358/00/009/webrev/
Changing socketAccept

WEBREV-010: http://cr.openjdk.java.net/~igerasim/8198358/00/010/webrev/
Changing socketConnect

WEBREV-011: http://cr.openjdk.java.net/~igerasim/8198358/00/011/webrev/
   Minor editing, comments, moving code

WEBREV-012: http://cr.openjdk.java.net/~igerasim/8198358/00/012/webrev/
Changing socketSetOption

WEBREV-013: http://cr.openjdk.java.net/~igerasim/8198358/00/013/webrev/
Changing socketGetOption

WEBREV-014: http://cr.openjdk.java.net/~igerasim/8198358/00/014/webrev/
Moving a few methods one more time


Accumulative webrev with all the changes above is available here:
http://cr.openjdk.java.net/~igerasim/8198358/01/webrev/


Thanks in advance!

Ivan

On 3/1/18 8:43 PM, Ivan Gerasimov wrote:

Hello!

I'd like to do the next step toward removing the TwoStacks socket 
implementation on Windows.


It would be aligning the two implementations (DualStack and 
TwoStacks), so they can be easier merged together during the next step.


There are three PlainSocketImpl implementations in JDK:
java.base/windows/classes/java/net/DualStackPlainSocketImpl.java
java.base/windows/classes/java/net/TwoStacksPlainSocketImpl.java
java.base/unix/classes/java/net/PlainSocketImpl.java

While two later have very similar organization (in particular, set of 
native methods), the former is organized slightly differently.
In order to merge the two Windows implementation together, they first 
need to be organized in a similar way.
For consistency, DualStack implementation will be reorganized to be 
aligned with TwoStacks and unix/PlainSocketImpl.


Bug: https://bugs.openjdk.java.net/browse/JDK-8198358
Webrev: http://cr.openjdk.java.net/~igerasim/8198358/00/webrev/

The change looks somewhat messy, but in fact it was a series of 
incremental changes, which I still keep in the mercurial 'mq'.


(I wish the webrev could be made incremental based on the mq patches, 
to make it easier to review.)


The patched JDK builds fine and all the regression tests pass Okay.


Thanks in advance!


--
With kind regards,
Ivan Gerasimov

Re: RFE: Add PATCH to the list of the supported HTTP methods

[Chris Hegarty]

Hi Andrej,

On 02/03/18 08:28, Andrej Golovnin wrote:

Hi all,

it would be nice if could add PATCH
(https://tools.ietf.org/html/rfc5789) to the list of the supported
...
- and one for the HTTP Client branch in the JDK sandbox repository
which adds PATCH to the HttpRequest builder:
   java_net_http_HttpRequest_PATCH_method.diff


The JDK HTTP Client has:
  `HttpRequest.Builder::method(String method, BodyPublisher publisher)`

 ,so it is currently possible to use the `PATCH` method.

Is `PATCH` sufficiently popular to warrant its own self-named
method in the request builder?

-Chris.

Re: RFR: JDK-8144300 : Java does not honor http.nonProxyHosts value having wildcard * both at end and start

[Chris Hegarty]

Pallavi,

On 01/03/18 14:46, Pallavi Sonal wrote:

Hi,

Please review the following change to fix JDK-8144300.

  


Bug : https://bugs.openjdk.java.net/browse/JDK-8144300

Webrev : http://cr.openjdk.java.net/~rpatil/8144300/webrev.00/


Looks ok to me. Please add the bugId to the @bug tag in the test
before pushing.

Thanks,
-Chris.

Re: RFR 8198302: VS2017 (C4477) java.base/windows/native/libnet/NetworkInterface_winXP.c incorrect printf format strings

[Chris Hegarty]

Brian,

I think this is fine.

-Chris.

On 06/03/18 15:26, Brian Burkhalter wrote:

https://bugs.openjdk.java.net/browse/JDK-8198302

Changes are in the diff below.

Thanks,

Brian

--- a/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
+++ b/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights 
reserved.

   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
@@ -39,14 +39,15 @@
  #ifdef DEBUG
  void printnif (netif *nif) {
  #ifdef _WIN64
-printf ("nif:0x%I64x name:%s\n", nif,nif->name);
+printf ("nif:0x%I64x name:%s\n", (UINT_PTR)nif, nif->name);
  #else
-printf ("nif:0x%x name:%s\n", nif,nif->name);
+printf ("nif:0x%x name:%s\n", nif, nif->name);
  #endif
  if (nif->dNameIsUnicode) {
-printf ("dName:%S index:%d ", nif->displayName,nif->index);
+printf ("dName:%S index:%d ", (unsigned short 
*)nif->displayName,

+nif->index);
  } else {
-printf ("dName:%s index:%d ", nif->displayName,nif->index);
+printf ("dName:%s index:%d ", nif->displayName, nif->index);
  }
  printf ("naddrs:%d\n", nif->naddrs);
  }

Re: RFR [11] 8198358 : Align organization of DualStackPlainSocketImpl with TwoStacksPlainSocketImp [win]

[Ivan Gerasimov]

Thank you Christoph for reviewing this!


On 3/6/18 4:53 AM, Langer, Christoph wrote:

Hi Ivan,

I went through the changes a bit and I would think it is really a good cleanup 
and will make the future merge of the implementations easier.

One thing I saw was that TwoStacksPlainSocketImpl.c has an #include  
in line 25. I think that can be removed!?

Certainly!  Probably it's a leftover from already removed code.
Thanks for spotting this.

With kind regards,
Ivan


I had problems importing the patch(set) via mercurial queues - but maybe it is 
an issue of my local mercurial version.
Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
Ivan Gerasimov
Sent: Freitag, 2. März 2018 05:43
To: Chris Hegarty ; net-dev@openjdk.java.net
Subject: RFR [11] 8198358 : Align organization of DualStackPlainSocketImpl
with TwoStacksPlainSocketImp [win]

Hello!

I'd like to do the next step toward removing the TwoStacks socket
implementation on Windows.

It would be aligning the two implementations (DualStack and TwoStacks),
so they can be easier merged together during the next step.

There are three PlainSocketImpl implementations in JDK:
java.base/windows/classes/java/net/DualStackPlainSocketImpl.java
java.base/windows/classes/java/net/TwoStacksPlainSocketImpl.java
java.base/unix/classes/java/net/PlainSocketImpl.java

While two later have very similar organization (in particular, set of
native methods), the former is organized slightly differently.
In order to merge the two Windows implementation together, they first
need to be organized in a similar way.
For consistency, DualStack implementation will be reorganized to be
aligned with TwoStacks and unix/PlainSocketImpl.

Bug: https://bugs.openjdk.java.net/browse/JDK-8198358
Webrev: http://cr.openjdk.java.net/~igerasim/8198358/00/webrev/

The change looks somewhat messy, but in fact it was a series of
incremental changes, which I still keep in the mercurial 'mq'.

(I wish the webrev could be made incremental based on the mq patches, to
make it easier to review.)

The patched JDK builds fine and all the regression tests pass Okay.


Thanks in advance!
--

With kind regards,
Ivan Gerasimov




--
With kind regards,
Ivan Gerasimov

Re: RFR 8198302: VS2017 (C4477) java.base/windows/native/libnet/NetworkInterface_winXP.c incorrect printf format strings

[Roger Riggs]

+1

On 3/6/2018 10:57 AM, Langer, Christoph wrote:


Looks good, Brian.

*From:*net-dev [mailto:net-dev-boun...@openjdk.java.net] *On Behalf Of 
*Brian Burkhalter

*Sent:* Dienstag, 6. März 2018 16:27
*To:* OpenJDK Network Dev list 
*Subject:* RFR 8198302: VS2017 (C4477) 
java.base/windows/native/libnet/NetworkInterface_winXP.c incorrect 
printf format strings


https://bugs.openjdk.java.net/browse/JDK-8198302

Changes are in the diff below.

Thanks,

Brian

--- a/src/java.base/windows/native/libnet/NetworkInterface_winXP.c

+++ b/src/java.base/windows/native/libnet/NetworkInterface_winXP.c

@@ -1,5 +1,5 @@

 /*

- * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights 
reserved.


+ * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights 
reserved.


* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.

*

* This code is free software; you can redistribute it and/or modify it

@@ -39,14 +39,15 @@

 #ifdef DEBUG

 void printnif (netif *nif) {

 #ifdef _WIN64

-       printf ("nif:0x%I64x name:%s\n", nif,nif->name);

+       printf ("nif:0x%I64x name:%s\n", (UINT_PTR)nif, nif->name);

 #else

-       printf ("nif:0x%x name:%s\n", nif,nif->name);

+       printf ("nif:0x%x name:%s\n", nif, nif->name);

 #endif

      if (nif->dNameIsUnicode) {

-           printf ("dName:%S index:%d ", nif->displayName,nif->index);

+           printf ("dName:%S index:%d ", (unsigned short 
*)nif->displayName,


+               nif->index);

      } else {

-           printf ("dName:%s index:%d ", nif->displayName,nif->index);

+           printf ("dName:%s index:%d ", nif->displayName, nif->index);

      }

      printf ("naddrs:%d\n", nif->naddrs);

 }

RE: RFR 8198302: VS2017 (C4477) java.base/windows/native/libnet/NetworkInterface_winXP.c incorrect printf format strings

[Langer, Christoph]

Looks good, Brian.

From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Brian 
Burkhalter
Sent: Dienstag, 6. März 2018 16:27
To: OpenJDK Network Dev list 
Subject: RFR 8198302: VS2017 (C4477) 
java.base/windows/native/libnet/NetworkInterface_winXP.c incorrect printf 
format strings

https://bugs.openjdk.java.net/browse/JDK-8198302

Changes are in the diff below.

Thanks,

Brian

--- a/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
+++ b/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -39,14 +39,15 @@
 #ifdef DEBUG
 void printnif (netif *nif) {
 #ifdef _WIN64
-printf ("nif:0x%I64x name:%s\n", nif,nif->name);
+printf ("nif:0x%I64x name:%s\n", (UINT_PTR)nif, nif->name);
 #else
-printf ("nif:0x%x name:%s\n", nif,nif->name);
+printf ("nif:0x%x name:%s\n", nif, nif->name);
 #endif
 if (nif->dNameIsUnicode) {
-printf ("dName:%S index:%d ", nif->displayName,nif->index);
+printf ("dName:%S index:%d ", (unsigned short *)nif->displayName,
+nif->index);
 } else {
-printf ("dName:%s index:%d ", nif->displayName,nif->index);
+printf ("dName:%s index:%d ", nif->displayName, nif->index);
 }
 printf ("naddrs:%d\n", nif->naddrs);
 }

RFR 8198302: VS2017 (C4477) java.base/windows/native/libnet/NetworkInterface_winXP.c incorrect printf format strings

[Brian Burkhalter]

https://bugs.openjdk.java.net/browse/JDK-8198302

Changes are in the diff below.

Thanks,

Brian

--- a/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
+++ b/src/java.base/windows/native/libnet/NetworkInterface_winXP.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -39,14 +39,15 @@
 #ifdef DEBUG
 void printnif (netif *nif) {
 #ifdef _WIN64
-printf ("nif:0x%I64x name:%s\n", nif,nif->name);
+printf ("nif:0x%I64x name:%s\n", (UINT_PTR)nif, nif->name);
 #else
-printf ("nif:0x%x name:%s\n", nif,nif->name);
+printf ("nif:0x%x name:%s\n", nif, nif->name);
 #endif
 if (nif->dNameIsUnicode) {
-printf ("dName:%S index:%d ", nif->displayName,nif->index);
+printf ("dName:%S index:%d ", (unsigned short *)nif->displayName,
+nif->index);
 } else {
-printf ("dName:%s index:%d ", nif->displayName,nif->index);
+printf ("dName:%s index:%d ", nif->displayName, nif->index);
 }
 printf ("naddrs:%d\n", nif->naddrs);
 }

Re: RFR: JDK-8144300 : Java does not honor http.nonProxyHosts value having wildcard * both at end and start

[Daniel Fuchs]

Hi,

Looks reasonable to me.

best regards,

-- daniel

On 01/03/2018 14:46, Pallavi Sonal wrote:

Hi,

Please review the following change to fix JDK-8144300.

  


Bug : https://bugs.openjdk.java.net/browse/JDK-8144300

Webrev : http://cr.openjdk.java.net/~rpatil/8144300/webrev.00/

  


Whenever there is a wildcard(*) at either the beginning or the end of a 
hostname specified in the list of http.nonProxyHosts , the code works as 
expected and DIRECT connection is used for them. But in scenarios, where there 
is a wildcard both at the beginning and end of the hostname, proxy is used 
instead of DIRECT connection.

  


All the tier1 and tier2 Mach 5 tests have passed.

  


Thanks,

Pallavi Sonal

RE: RFR [11] 8198358 : Align organization of DualStackPlainSocketImpl with TwoStacksPlainSocketImp [win]

[Langer, Christoph]

Hi Ivan,

I went through the changes a bit and I would think it is really a good cleanup 
and will make the future merge of the implementations easier.

One thing I saw was that TwoStacksPlainSocketImpl.c has an #include  
in line 25. I think that can be removed!?

I had problems importing the patch(set) via mercurial queues - but maybe it is 
an issue of my local mercurial version.

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> Ivan Gerasimov
> Sent: Freitag, 2. März 2018 05:43
> To: Chris Hegarty ; net-dev@openjdk.java.net
> Subject: RFR [11] 8198358 : Align organization of DualStackPlainSocketImpl
> with TwoStacksPlainSocketImp [win]
> 
> Hello!
> 
> I'd like to do the next step toward removing the TwoStacks socket
> implementation on Windows.
> 
> It would be aligning the two implementations (DualStack and TwoStacks),
> so they can be easier merged together during the next step.
> 
> There are three PlainSocketImpl implementations in JDK:
> java.base/windows/classes/java/net/DualStackPlainSocketImpl.java
> java.base/windows/classes/java/net/TwoStacksPlainSocketImpl.java
> java.base/unix/classes/java/net/PlainSocketImpl.java
> 
> While two later have very similar organization (in particular, set of
> native methods), the former is organized slightly differently.
> In order to merge the two Windows implementation together, they first
> need to be organized in a similar way.
> For consistency, DualStack implementation will be reorganized to be
> aligned with TwoStacks and unix/PlainSocketImpl.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8198358
> Webrev: http://cr.openjdk.java.net/~igerasim/8198358/00/webrev/
> 
> The change looks somewhat messy, but in fact it was a series of
> incremental changes, which I still keep in the mercurial 'mq'.
> 
> (I wish the webrev could be made incremental based on the mq patches, to
> make it easier to review.)
> 
> The patched JDK builds fine and all the regression tests pass Okay.
> 
> 
> Thanks in advance!
> --
> 
> With kind regards,
> Ivan Gerasimov

RE: httpserver does not close connections when RejectedExecutionException occurs

[Langer, Christoph]

Hi Yuji,

I had a look and to me it seems safe to do like webrev.03 suggests. That is, 
remove the throws clause. As Daniel pointed out, it seems that all callers of 
the "handle" method would do merely the same. The only thing I'm not sure about 
is the CancelledKeyException caught in line 413. In that case no exception 
would be logged currently. With your change, it would now inside "handle". 
Maybe you want to handle a CancelledKeyException explicitly in "handle" now and 
suppress the log?

What tests did you run?

BTW: you could remove line 396: ' boolean closed;' while you are touching this 
file. It is not needed.

Best regards
Christoph

> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of
> KUBOTA Yuji
> Sent: Donnerstag, 1. März 2018 12:41
> To: OpenJDK Network Dev list 
> Cc: Yasumasa Suenaga 
> Subject: Re: httpserver does not close connections when
> RejectedExecutionException occurs
> 
> Hi all,
> 
> Could you please review my patch(s)?
> 
> Thanks,
> Yuji
> 
> 2018-02-20 14:21 GMT+09:00 KUBOTA Yuji :
> > Hi Daniel,
> >
> > Thank you for your comment.
> >
> > Dispatcher is package-private class and handle method is called at
> > only this file in the package (sun.net.httpserver), and all call sites
> > look like to handle exception suitably. So we can remove `throws
> > IOException` clause without modifying the call site logic as like
> > webrev.03.
> > http://cr.openjdk.java.net/~ykubota/8169358/webrev.03
> >
> > However, I could not find whether can I change a signature of public
> > method without specified steps. If we need to write CSR to remove
> > `throws IOException` clause, we should remove the clause by other
> > issue. And I want to commit webrev.02  at this issue.
> > http://cr.openjdk.java.net/~ykubota/8169358/webrev.02
> >
> > I'd like to get your thoughts on that.
> >
> > P.S. I'm an author, not a committer, so I cannot access Mach5.
> >
> > Thanks,
> > Yuji
> >
> > 2018-02-17 1:11 GMT+09:00 Daniel Fuchs :
> >> Hi,
> >>
> >> On the surface I'd say that this change looks reasonable.
> >> However, handle is declared to throw IOException, and
> >> with this change it is guaranteed to never throw even
> >> RuntimeException.
> >>
> >> It seems that the code that calls handle() - at least in
> >> this file, has some logic to handle IOException - or
> >> other exception possibly thrown by Dispatcher::handle,
> >> which as far as I can see is not doing much more than what
> >> closeConnection does. So it looks as if calling
> >> closeConnection in handle() instead of throwing could be OK.
> >>
> >> However it would be good to at least try to figure out
> >> whether there are any other call sites for Dispatcher::handle,
> >> validate that no longer throwing will not modify the call site
> >> logic, and possibly investigate if the 'throws IOException' clause
> >> can be removed from Dispatcher::handle (that is: look
> >> whether Dispatcher is exposed and/or can be subclassed and
> >> if there are any existing subclasses).
> >>
> >> best regards,
> >>
> >> -- daniel
> >>
> >>
> >> On 16/02/2018 15:29, KUBOTA Yuji wrote:
> >>>
> >>> Hi Chris and Yasumasa,
> >>>
> >>> Sorry to have remained this issue for a long time. I come back.
> >>>
> >>> I updated my patch for the following three reasons.
> >>> http://cr.openjdk.java.net/~ykubota/8169358/webrev.02/
> >>>
> >>> * applies to the latest jdk/jdk instead of jdk9/dev
> >>> * adds test by modifying reproducer as like tests on
> >>> test/jdk/com/sun/net/httpserver
> >>> * prevents potential connection leak as Yasumasa said. For example, a
> >>> user sets own implementation of the thread pool to HttpServer and
> then
> >>> throws unexpected exceptions and errors. I think that we should save
> >>> existing exception handler to keep compatibility and minimize the risk
> >>> of connection leak by catching Throwable.
> >>>
> >>> I've tested test/jdk/com/sun/net/httpserver and passed.
> >>> I'm not a committer, so I can not access March 5.
> >>>
> >>> Could you review and sponsor it?
> >>>
> >>> Thanks,
> >>> Yuji
> >>>
> >>> 2016-11-11 12:11 GMT+09:00 KUBOTA Yuji :
> 
>  Hi Chris and Yasumasa,
> 
>  Thank you for your comments!
> 
>  I had faced connection leak on production by this handler. It seems
>  like a corner-case but it's a real risk to me.
>  I had focused REE on this issue, but it is a subclass of
>  RuntimeException, so I think that we should investigate
>  RuntimeException, too.
> 
>  If Chris agrees with the covering RuntimeException by JDK-8169358, I
>  will investigate it and update my patch.
>  If we should investigate on another issue, I want to add a test case
>  to check fixing about REE like existing jtreg, and I will create a new
>  issue after an investigation about RuntimeException.
> 
>  Any