java.net.Socket should report the attempted address and port

Michael McMahon michael.x.mcmahon at oracle.com
Fri Jun 8 07:00:56 UTC 2018


Jaikiran

Thanks for the review and the two comments. You're right the close() was 
omitted
by mistake. On the initialization of the system property, I would think 
that caching
in a static final is very common for such properties and I'm not aware 
of that being
explicitly documented anywhere. But, I will check into that.

All the best,

Michael.

On 08/06/2018, 07:45, Jaikiran Pai wrote:
>
> Also one other minor detail - given that this system property value is 
> read onceand cached as a static final, would it make sense to include 
> a note (in the property description of the java.security file?) that 
> explicitly states that this property needs to be set while launching 
> Java and can't be overridden programatically (since that can 
> potentially behave differentlydepending on when the SocketExceptions 
> class gets initialized)?
>
> -Jaikiran
>
>
> On 08/06/18 12:05 PM, Jaikiran Pai wrote:
>>
>> Hi Michael,
>>
>> I'm not a reviewer. I just checked the webrev and saw this change:
>>
>>
>> --- old/src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java	2018-06-06 08:34:38.000000000 +0100
>> +++ new/src/java.base/share/classes/java/net/AbstractPlainSocketImpl.java	2018-06-06 08:34:38.000000000 +0100
>> @@ -37,6 +37,7 @@
>> ...
>>               }
>>           } catch (IOException e) {
>> -            close();
>> -            throw e;
>> +            throw SocketExceptions.of(e, new InetSocketAddress(address, port));
>>
>> Is it intentional to remove that call to close()?
>>
>> The rest of the changes look fine to me.
>>
>> -Jaikiran
>>
>> On 06/06/18 1:15 PM, Michael McMahon wrote:
>>> Hi all,
>>>
>>> Finally to return to this topic. We have looked at a few different 
>>> approaches
>>> and it seems the best way is to define a security property that can 
>>> be set
>>> in the java.security configuration file, but which can be overridden 
>>> as a
>>> system property. The current behavior will remain the default, but 
>>> setting
>>> the property will add addressing information to exception texts.
>>> The change applies to all TCP socket types in java.net and java.nio.
>>> Webrev at: 
>>> http://cr.openjdk.java.net/~michaelm/8204233/webrev.1/index.html
>>>
>>> Thanks,
>>>
>>> Michael.
>>>
>>> On 01/05/2018, 09:48, Michael McMahon wrote:
>>>> Peter,
>>>>
>>>> Just to followup on this. We are still investigating a few options 
>>>> for doing this
>>>> and it might be a few more weeks before we get a decision. I did 
>>>> take your patch
>>>> as a starting point, and modified it to also work with NIO, and 
>>>> also to preserve
>>>> the original exception (with original stack trace) which I think is 
>>>> desirable.
>>>> I don't think there is much point in reviewing the webrev until we 
>>>> get the decision
>>>> mentioned above. But, we should be able to push it soon after that.
>>>>
>>>> Thanks,
>>>> Michael
>>>>
>>>> On 23/04/2018, 10:05, Péter Gergely Horváth wrote:
>>>>> Hi Tobias,
>>>>>
>>>>> Thank you for pointing me to that thread: it's good to have that 
>>>>> context (it was sent before I joined the mailing list, so please 
>>>>> bear with me).
>>>>>
>>>>> I understand the JDK developers want to be safe than sorry around 
>>>>> reporting target addresses and I absolutely agree with that point.
>>>>>
>>>>> However considering how useful it would be to have this 
>>>>> _optionally_ for debugging, I am wondering if it would be possible 
>>>>> to have a dedicated Java system property defined for this (e.g. 
>>>>> 'java.net.socket.reportAddressInException' or something like 
>>>>> that), which would enable this behaviour (retaining the current 
>>>>> behaviour of *not reporting anything by default.*).
>>>>>
>>>>> What do you think about this, guys? With this in place both the 
>>>>> secure-by-default requirement would be met, and there would be a 
>>>>> powerful tool available to fight the horrors of debugging a 
>>>>> running complex distributed application from its logs.
>>>>>
>>>>> Thanks,
>>>>> Peter
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Sun, Apr 22, 2018 at 10:21 PM, James Roper <james at lightbend.com 
>>>>> <mailto:james at lightbend.com>>wrote:
>>>>>
>>>>>     This would be especially useful in asynchronous applications -
>>>>>     since in those cases the exception rarely maps back to a place
>>>>>     in user code that would indicate what is being connected to.
>>>>>     As someone who has spent a lot of time supporting developers
>>>>>     who use asynchronous libraries and post exceptions of this
>>>>>     nature (supporting both in open source, eg on stack overflow,
>>>>>     as well as providing commercial support), where I don't have
>>>>>     access to their code base so I can't do the necessary
>>>>>     investigations directly myself, having the attempted address
>>>>>     and port in the error message would save a lot of time, and
>>>>>     probably even prevent a lot of people from requiring support
>>>>>     in the first place.
>>>>>
>>>>>     On 22 April 2018 at 20:59, Péter Gergely Horváth
>>>>>     <peter.gergely.horvath at gmail.com
>>>>>     <mailto:peter.gergely.horvath at gmail.com>>wrote:
>>>>>
>>>>>         Hi All,
>>>>>
>>>>>         I am wondering if it would be possible to make a minor
>>>>>         improvement to the way *java.net.Socket* reports
>>>>>         connectivity errors and incorporate the attempted address,
>>>>>         port and the timeout used into the exception message.
>>>>>
>>>>>         The current implementation emits a generic error message,
>>>>>         which is not that helpful when one is operating a _large_
>>>>>         application. (Consider e.g. Big Data or complex legacy
>>>>>         systems written by someone else).
>>>>>
>>>>>         java.net.ConnectException: Connection refused (Connection
>>>>>         refused)
>>>>>         at java.net.PlainSocketImpl.socketConnect(Native Method)
>>>>>         at java.net
>>>>>         <http://java.net>.AbstractPlainSocketImpl.doConnect(AbstractPlainSocketImpl.java:350)
>>>>>         at java.net
>>>>>         <http://java.net>.AbstractPlainSocketImpl.connectToAddress(AbstractPlainSocketImpl.java:206)
>>>>>         at java.net
>>>>>         <http://java.net>.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:188)
>>>>>         at java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392)
>>>>>         at java.net.Socket.connect(Socket.java:589)
>>>>>         at java.net.Socket.connect(Socket.java:538)
>>>>>         at java.net.Socket.<init>(Socket.java:434)
>>>>>         at java.net.Socket.<init>(Socket.java:211)
>>>>>         at Sample.main(Sample.java:9)
>>>>>
>>>>>
>>>>>         I have looked into the JDK code base and implemented a
>>>>>         patch that reports the address, port and timeout used in
>>>>>         the error message without touching any native parts (see
>>>>>         attached patch file). I have tested this (created my own
>>>>>         build of the JDK and run a sample application against it)
>>>>>         and it seems to work properly.
>>>>>
>>>>>         Would it be possible to incorporate this change into the
>>>>>         official JDK code base? I do believe it would help a lot
>>>>>         of people out there.
>>>>>
>>>>>         Based on my understanding, once I have signed the OCA, I
>>>>>         should simply write an email to the group and request
>>>>>         a sponsor to pick up this issue. Could someone help me
>>>>>         with this?
>>>>>
>>>>>         Thank you,
>>>>>         Peter
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>     -- 
>>>>>     *James Roper*
>>>>>     /Senior Octonaut/
>>>>>
>>>>>     Lightbend <https://www.lightbend.com/> – Build reactive apps!
>>>>>     Twitter: @jroper <https://twitter.com/jroper>
>>>>>
>>>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20180608/d4a0d42f/attachment-0001.html>


More information about the net-dev mailing list