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

[Michael McMahon]

Hi all,

I agree with Sean's suggestion below that a multi-valued property
captures the generality in the name and the specific case implemented
here with the value "hostInfo". So, how about exactly as suggested:
property name - "jdk.net.includeInExceptions" with possible value limited to
"hostinfo" for now?

Michael

On 15/06/2018, 15:28, Sean Mullan wrote:
> Hi Michael,
>
> I agree with Alan and Peter that the name should more clearly identify 
> the security implications of setting it.
>
> Alternatively, if you think you may build on this you might want to 
> add support for a multi-valued property, like 
> jdk.net.includeInExceptions=hostInfo,...
>
> --Sean
>
> On 6/14/18 1:41 PM, Michael McMahon wrote:
>> Hi Alan,
>>
>> Thanks for looking at it.
>>
>> On 14/06/2018, 18:10, Alan Bateman wrote:
>>> On 06/06/2018 08:45, 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
>>> This looks quite good and the ability to use a system property to 
>>> override the java.security file is useful for ad hoc enabling. The 
>>> property name can probably be improved The jdk.net prefix looks 
>>> right but jdk.net.enhanceExceptionText isn't very clear, esp. when 
>>> used on the command line. It feels it should something like 
>>> jdk.net.includeHostInfoInExceptions or something that makes it clear 
>>> that it adds host information to socket exceptions.
>>>
>> That seems too specific to me. My feeling was that other exceptions 
>> might be enhanced in the same way and would
>> hang off the same property name. If we use a name that is specific to 
>> hostinfo, then we would need a new property
>> for other information types.
>>
>>> I see Jaikiran Pai spotted the close was accidentally removed from 
>>> AbstractPlainSocketImpl so I assume you'll fix that.
>>>
>> Yes, that was fixed and the webrev updated in place.
>>
>>> Aside from AsynchronousCloseException, are there are other 
>>> IOException classes that don't have the 1-arg String constructor. 
>>> Just wondering if it would be better to special case that to not use 
>>> SocketExceptions or alternative not rely on catching 
>>> NoSuchMethodException.
>>>
>> The problem was I wrote it first checking types statically, and there 
>> were a lot of different exception types,
>> which is ugly enough to begin with but I also overlooked those NIO 
>> types completely.
>> It was just difficult to write a test that generated all the possible 
>> exceptions. So, my concern was overlooking
>> any future change in that area. Or are you suggesting we just not 
>> implement this for the async socket channels?
>>
>> Thanks,
>>
>> Michael
>>> -Alan
>>>
>>>
>>>
>>>
>>>
>>>
>>>