RE: RFR (S) JDK-8160174: java.net.NetworkInterface - fixes and improvements for network interface listing

[Langer, Christoph]

Chris,

thanks for your comments. I'm kind of thinking the same - I actually also don't 
really like my result of merging the strings of code so much.

I'll go on and prepare a smaller change set only addressing the real bugs.

I also agree to switching the Linux implementation to getifaddrs at the time 
the Java 10 branch opens to gain experience with that without shipment pressure.

Stay tuned :)
Christoph


> -Original Message-
> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> Sent: Dienstag, 12. Juli 2016 16:10
> To: Langer, Christoph 
> Cc: net-dev@openjdk.java.net
> Subject: Re: RFR (S) JDK-8160174: java.net.NetworkInterface - fixes and
> improvements for network interface listing
> 
> Christoph,
> 
> > On 11 Jul 2016, at 14:36, Langer, Christoph 
> wrote:
> >
> > Hi Chris (or anyone who is holding a stake in the NetworkInterface native
> implementation),
> >
> > I’ve spent some time on cleaning up NetworkInterface.c and came up with a
> bigger change:http://cr.openjdk.java.net/~clanger/webrevs/8160174.2/
> >
> > With this I attempted to consolidate the interface listing functionality of 
> > the 2
> main categories – one is ioctl (used on Linux IPv4, Solaris and AIX) and the
> other is getifaddrs (used on MacOS). I introduced some defines to switch
> between the implementations. I also consolidated the functionality for the 
> ioctl
> based network interface listings by using an #ifdef section to distinguish
> between the Linux/AIX versus Solaris field and constant names.
> >
> > I’ve spent some time testing on the platforms and in principal it works. 
> > But as
> it is a matter of taste, I’d like to ask you if you support this type of 
> change or
> have any hints or recommendations before going forward to finalize this.
> 
> I’m personally don’t like this approach much. I think it adds further
> complexity and difficulty to this code, which already has its fair share
> of both.
> 
> > For Linux I’m also suggesting to use the getifaddrs approach – I tested and
> found it working everywhere and with this we could get rid of the
> implementation to read /proc/net for IPv6.
> 
> You should probably break this type of change out, so that it can be
> evaluated independently, on its own merit. If it add nothing more than
> clean up, may it makes sense for this to happen early in 10, where it
> can have a longer time to bake.
> 
> > Furthermore I’m generally setting Null for the IPv6 broadcast address – 
> > which
> I think is common sense for IPv6.
> 
> Same as above.  This “smaller” changes can sometimes get lost in
> the noise of larger refactoring.
> 
> -Chris.
> 
> 
> > Thanks in advance and best regards
> > Christoph
> >
> >
> > From: Langer, Christoph
> > Sent: Donnerstag, 23. Juni 2016 16:37
> > To: 'net-dev@openjdk.java.net' 
> > Subject: RFR (S) JDK-8160174: java.net.NetworkInterface - fixes and
> improvements for network interface listing
> >
> > Hi,
> >
> > can I please get a review the following change:
> > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8160174.1/
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8160174
> >
> > I made further fixes and cleanups for listing Unix type network interfaces,
> especially on AIX and Linux. I ran builds and verified also on Solaris and 
> Mac.
> >
> > Thanks
> > Christoph

Re: RFR 8151788: NullPointerException from ntlm.Client.type3

[Weijun Wang]

On 7/12/2016 22:34, Pavel Rappo wrote:

What's the difference between no security buffer and an empty one (from the
com.sun.security.ntlm.Client#type3's perspective)?


I quickly browse through the NTLM protocol and yes they look like the 
same in each case. (Except for one which I am not sure, is there any 
difference between no domain and empty domain?) In all cases where a 
security buffer is optional, there is a flag we can rely on, and no need 
to look at whether the offset of the security buffer is zero.


So it does look safer to return a new byte[0] right inside 
readSecurityBuffer(int offset) when the offset is zero.


Thanks
Max




On 12 Jul 2016, at 15:25, Wang Weijun  wrote:

When there is no offset, there is no security buffer at all. When the length is 
zero, the security buffer is an empty byte array.

Re: RFR 8151788: NullPointerException from ntlm.Client.type3

[Pavel Rappo]

What's the difference between no security buffer and an empty one (from the
com.sun.security.ntlm.Client#type3's perspective)?

> On 12 Jul 2016, at 15:25, Wang Weijun  wrote:
> 
> When there is no offset, there is no security buffer at all. When the length 
> is zero, the security buffer is an empty byte array.

Re: RFR 8151788: NullPointerException from ntlm.Client.type3

[Wang Weijun]

The fix looks fine to me. 

The readSecurityBuffer() is correct. When there is no offset, there is no 
security buffer at all. When the length is zero, the security buffer is an 
empty byte array. 

Here I think the customer encountered a server-side error. The fix is for 
interop with this abnormal behavior only. 

Thanks
Max

> 在 2016年7月12日，22:11，Pavel Rappo  写道：
> 
> Hi Vyom,
> 
> I wonder if Max (the author of the original NTLM.java) has seen the fix? The
> reason I'm asking is that when I see this
> 
>byte[] readSecurityBuffer(int offset) throws NTLMException {
>int pos = readInt(offset+4);
> *   if (pos == 0) return null;
>try {
>return Arrays.copyOfRange(
>internal, pos, pos + readShort(offset));
>} catch (ArrayIndexOutOfBoundsException ex) {
>throw new NTLMException(NTLMException.PACKET_READ_ERROR,
>"Input message incorrect size");
>}
>}
> 
> and this
> 
> *   // Some client create a alist even if server does not send
> *   // one: (i16)2 (i16)len target_in_unicode (i16)0 (i16) 0
>byte[] alist = null;
>if ((inputFlags & 0x80) != 0) {
>alist = r.readSecurityBuffer(40);
>}
> 
> It seems to me like we return null to work around it later with byte[0]. If 
> so,
> wouldn't it be any better to return byte[0] straight from readSecurityBuffer?
> 
> In other words, can we dig a bit deeper in order to make this awkward null
> handling more natural?
> 
> P.S. I'm not an expert in this area, I may be easily saying something that
> doesn't make much sense. Feel free to ignore it.
> 
> Thanks,
> -Pavel
> 
>> On 12 Jul 2016, at 14:27, Vyom Tewari  wrote:
>> 
>> gentile reminder.
>> Vyom
>> 
>>> On Thursday 30 June 2016 02:16 PM, Vyom Tewari wrote:
>>> Hi All,
>>> 
>>> Please review the below simple fix.
>>> 
>>> Bug: JDK-8151788  NullPointerException from ntlm.Client.type3
>>> Webrev  : http://cr.openjdk.java.net/~vtewari/8151788/webrev0.0/index.html 
>>> 
>>> 
>>> Thanks,
>>> Vyom
>> 
> 

Re: RFR (S) JDK-8160174: java.net.NetworkInterface - fixes and improvements for network interface listing

[Chris Hegarty]

Christoph,

> On 11 Jul 2016, at 14:36, Langer, Christoph  wrote:
> 
> Hi Chris (or anyone who is holding a stake in the NetworkInterface native 
> implementation),
>  
> I’ve spent some time on cleaning up NetworkInterface.c and came up with a 
> bigger change:http://cr.openjdk.java.net/~clanger/webrevs/8160174.2/
>  
> With this I attempted to consolidate the interface listing functionality of 
> the 2 main categories – one is ioctl (used on Linux IPv4, Solaris and AIX) 
> and the other is getifaddrs (used on MacOS). I introduced some defines to 
> switch between the implementations. I also consolidated the functionality for 
> the ioctl based network interface listings by using an #ifdef section to 
> distinguish between the Linux/AIX versus Solaris field and constant names.
>  
> I’ve spent some time testing on the platforms and in principal it works. But 
> as it is a matter of taste, I’d like to ask you if you support this type of 
> change or have any hints or recommendations before going forward to finalize 
> this.

I’m personally don’t like this approach much. I think it adds further
complexity and difficulty to this code, which already has its fair share
of both.

> For Linux I’m also suggesting to use the getifaddrs approach – I tested and 
> found it working everywhere and with this we could get rid of the 
> implementation to read /proc/net for IPv6.

You should probably break this type of change out, so that it can be
evaluated independently, on its own merit. If it add nothing more than
clean up, may it makes sense for this to happen early in 10, where it
can have a longer time to bake.

> Furthermore I’m generally setting Null for the IPv6 broadcast address – which 
> I think is common sense for IPv6.

Same as above.  This “smaller” changes can sometimes get lost in 
the noise of larger refactoring.

-Chris.


> Thanks in advance and best regards
> Christoph
>  
>  
> From: Langer, Christoph 
> Sent: Donnerstag, 23. Juni 2016 16:37
> To: 'net-dev@openjdk.java.net' 
> Subject: RFR (S) JDK-8160174: java.net.NetworkInterface - fixes and 
> improvements for network interface listing
>  
> Hi,
>  
> can I please get a review the following change:
> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8160174.1/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8160174
>  
> I made further fixes and cleanups for listing Unix type network interfaces, 
> especially on AIX and Linux. I ran builds and verified also on Solaris and 
> Mac.
>  
> Thanks
> Christoph

Re: RFR 8151788: NullPointerException from ntlm.Client.type3

[Vyom Tewari]

gentile reminder.
Vyom

On Thursday 30 June 2016 02:16 PM, Vyom Tewari wrote:

Hi All,

Please review the below simple fix.

Bug: JDK-8151788  NullPointerException from ntlm.Client.type3
Webrev  : 
http://cr.openjdk.java.net/~vtewari/8151788/webrev0.0/index.html 



Thanks,
Vyom

Re: RFR 8144692: HttpServer API: use of non-existant method in example in package Javadoc

[Vyom Tewari]

Hi Pavel,

Thanks for  review, i updated the 
webrev(http://cr.openjdk.java.net/~vtewari/8144692/webrev0.0/index.html 
) in 
place.


Thanks,
Vyom


On Tuesday 12 July 2016 06:10 PM, Pavel Rappo wrote:

Hi Vyom,

A minor comment. Do you think it would make sense to use space after

 new InetSocketAddress(8000),
 
as in every other line in this javadoc where multiple arguments passed, the

space is used. Just to be consistent in formatting.

Thanks.


On 12 Jul 2016, at 12:36, Vyom Tewari  wrote:

Hi All,

Please review below small doc fix.

Bug: JDK-8144692 HttpServer API: use of non-existant method in 
example in package Javadoc
Webrev  : http://cr.openjdk.java.net/~vtewari/8144692/webrev0.0/index.html 


Thanks,
Vyom

Re: RFR 8144692: HttpServer API: use of non-existant method in example in package Javadoc

[Pavel Rappo]

Hi Vyom,

A minor comment. Do you think it would make sense to use space after 

new InetSocketAddress(8000),

as in every other line in this javadoc where multiple arguments passed, the
space is used. Just to be consistent in formatting.

Thanks.

> On 12 Jul 2016, at 12:36, Vyom Tewari  wrote:
> 
> Hi All,
> 
> Please review below small doc fix.
> 
> Bug: JDK-8144692 HttpServer API: use of non-existant method in 
> example in package Javadoc
> Webrev  : 
> http://cr.openjdk.java.net/~vtewari/8144692/webrev0.0/index.html 
> 
> 
> Thanks,
> Vyom

RFR 8144692: HttpServer API: use of non-existant method in example in package Javadoc

[Vyom Tewari]

Hi All,

Please review below small doc fix.

Bug: JDK-8144692 HttpServer API: use of non-existant method 
in example in package Javadoc
Webrev  : 
http://cr.openjdk.java.net/~vtewari/8144692/webrev0.0/index.html 



Thanks,
Vyom