Hi Michael,
I am wondering if the property name could be maybe improved:
1.) The prefix "jdk" does not seem to match the naming convention that of
the existing networking properties [1], where for example prefix "java.net"
is used for "java.net.preferIPv4Stack"
2.) I agree with Alan that the current name ("enhanceExceptionText") is not
fully specific about what is going to happen when enabled. Since this
feature is made optional due to security considerations, I would rather be
completely clear regarding the meaning of this switch.
Once again, thank you for all your efforts here.
Thanks,
Peter
[1] https://docs.oracle.com/javase/7/docs/api/java/net/
doc-files/net-properties.html
On Thu, Jun 14, 2018 at 7:41 PM, Michael McMahon <
michael.x.mcma...@oracle.com> 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
>>
>>
>>
>>
>>
>>
>>
>>
