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







Reply via email to