Re: RFR[8237890]: 'DatagramPacket::getSocketAddress doesn't specify what happens if address or port are not set'

[Alan Bateman]

On 15/04/2020 16:40, Patrick Concannon wrote:


Hi Alan,

Thanks for your feedback. I've updated the fix with your comments and 
you can find them in the new webrev below.


WRT the release note JDK-8237530 
, that's a good 
point. I will look into removing it.


webrev: http://cr.openjdk.java.net/~pconcannon/8237890/webrevs/webrev.01/


Thanks for the update. Looks good and I've reviewed the CSR too.

Chris is right that we should have tests to check the initial port (and 
address) when using the constructors that don't specify them.


-Alan

Re: RFR[8237890]: 'DatagramPacket::getSocketAddress doesn't specify what happens if address or port are not set'

[Patrick Concannon]

Hi Chris,

Thanks for that.

I've added the new testcases as requested, and you can find them in the 
new webrev below.


http://cr.openjdk.java.net/~pconcannon/8237890/webrevs/webrev.02/


Kind regards,

Patrick

On 15/04/2020 16:54, Chris Hegarty wrote:



On 15 Apr 2020, at 16:40, Patrick Concannon  
wrote:

...

webrev: http://cr.openjdk.java.net/~pconcannon/8237890/webrevs/webrev.01/


I added myself as reviewer on the CSR.

The changes mainly look good. Can you please add, or amend an existing, test 
for the newly specified default address and port values.

-Chris.

Re: RFR[8237890]: 'DatagramPacket::getSocketAddress doesn't specify what happens if address or port are not set'

[Daniel Fuchs]

Thanks Patrick.

Looks good me.

best regards,

-- daniel

On 16/04/2020 14:39, Patrick Concannon wrote:

Hi Chris,

Thanks for that.

I've added the new testcases as requested, and you can find them in the 
new webrev below.


http://cr.openjdk.java.net/~pconcannon/8237890/webrevs/webrev.02/


Kind regards,

Patrick

On 15/04/2020 16:54, Chris Hegarty wrote:

On 15 Apr 2020, at 16:40, Patrick Concannon  
wrote:

...

webrev:http://cr.openjdk.java.net/~pconcannon/8237890/webrevs/webrev.01/


I added myself as reviewer on the CSR.

The changes mainly look good. Can you please add, or amend an existing, test 
for the newly specified default address and port values.

-Chris.

Re: RFR[8237890]: 'DatagramPacket::getSocketAddress doesn't specify what happens if address or port are not set'

[Chris Hegarty]

> On 16 Apr 2020, at 14:39, Patrick Concannon  
> wrote:
> 
> ...
> 
> http://cr.openjdk.java.net/~pconcannon/8237890/webrevs/webrev.02/

LGTM.

-Chris.

Re: 8238270: java.net HTTP/2 client does not decrease stream count when receives 204 response

[Daniel Fuchs]

Hi Chris,


The fix ensures that any pending data frame will still be processed
after 204 has been received - thus triggering the logic that
eventually closes the stream.


I suppose that a 204 response MUST have an END_STREAM
in its final HEADERS / CONTINUATION frame, right?


I have rescanned the RFC. I could not find a definitive answer to
that question. AFAIU it's expected that the HEADERS frame will
carry the END_STREAM - but there is some sibylline text that let
me think that having an empty DATA frame carrying the END_STREAM
would be OK too:

<<< A response
   that is defined to have no payload, as described in [RFC7230],
   Section 3.3.2, can have a non-zero content-length header field, even
   though no content is included in DATA frames. >>>


It is allowable for a HEADERS frame to carry an END_STREAM,
but not an END_HEADERS. If this happens, then CONTINUATION
frames must follow, the last of which will carry END_HEADERS.
That probably explains why the END_STREAM handling is done
the way that it is.


Yes. And the CONTINUATION frame is not supposed to carry the
END_STREAM - so we can see END_STREAM before END_HEADERS.
Therefore I have removed my changes to HeaderFrame.java.
Code inspection shows that we might not deal with that situation
properly - so I have logged https://bugs.openjdk.java.net/browse/JDK-8242999
to follow up on this.


The new test is good, but has an unnecessary reference to
AbstractThrowingSubscribers.TestExecutor.

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


Done.

New webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8238270/webrev.01/

best regards, and thanks for the valuable feedback!

-- daniel



webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8238270/webrev.00/index.html

Re: RFR[8237890]: 'DatagramPacket::getSocketAddress doesn't specify what happens if address or port are not set'

[mark sheppard]

Hi Patrick et al.
   a trivial point and a javadoc question

trivial item in SendCheck test, there's a comment referring to pkt3 & 4 and 
invalid port -1.
This is no longer the case -- afaik it's now "can't send port 0 " in a 
SocketException ?

I note that there are some java doc changes and wonder is there an opportunity 
to make some further
amendments:

for getSocketAddress and setSocketAddress methods

the wording "of the remote host"  doesn't quite fit,
and could be replaced with   "of the application or service "

in general remote host could be replaced with host, as localhost provides  a 
valid IP address !!

references to machine could be replaced with host for consistency, also ?

regards
Mark


From: net-dev  on behalf of Patrick Concannon 

Sent: Thursday 16 April 2020 13:39
To: Chris Hegarty ; OpenJDK Network Dev list 

Subject: Re: RFR[8237890]: 'DatagramPacket::getSocketAddress doesn't specify 
what happens if address or port are not set'


Hi Chris,


Thanks for that.

I've added the new test cases as requested, and you can find them in the new 
webrev below.

http://cr.openjdk.java.net/~pconcannon/8237890/webrevs/webrev.02/


Kind regards,

Patrick

On 15/04/2020 16:54, Chris Hegarty wrote:

On 15 Apr 2020, at 16:40, Patrick Concannon 
 wrote:

...

webrev: http://cr.openjdk.java.net/~pconcannon/8237890/webrevs/webrev.01/



I added myself as reviewer on the CSR.

The changes mainly look good. Can you please add, or amend an existing, test 
for the newly specified default address and port values.

-Chris.