Hi,
There is an updated webrev for this at:
http://cr.openjdk.java.net/~michaelm/8204233/webrev.2/
I'd like to get this into 11 and it needs a CSR to approve
the property name change. So, hopefully it can be reviewed
quickly.
Thanks,
Michael
On 18/06/2018, 13:15, Michael McMahon wrote:
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
