RFR: 8211752: JNU_ThrowIOExceptionWithLastErrorAndPath - enhance some IOExceptions with path causing the issue

[Langer, Christoph]

Hi Matthias,

I generally support this enhancement of IOExceptions to include path information.

I also think that we should protect this with the java.security property "jdk.includeInExceptions" and agree that "path" would be a good choice since it is generic enough to be used in other places where Exceptions are thrown that might include path names. As for some comment from security-folks: Sean, what is your take on this?

As for the implementation: We should definitely get rid of the upcall into Java from JNU_ThrowIOExceptionWithLastErrorAndPath and build the exception message in that function only, just using the " jstring path" argument.

The reason for having the current implementation of ExceptionUtils. createExceptionTextWithPath, as you suggested in your webrev was that we were chasing a very specific customer issue where cwd and user.dir diverged (e.g. because of some 3rd party native library which changed the cwd). And this code is somehow still around.
So, user.dir should definitely not be part of the exception message. And whether the current cwd should be contained should be reviewed in detail again. Maybe at some places it is interesting but on other places it isn't at all.

Best regards
Christoph

> -----Original Message-----
> From: core-libs-dev <core-libs-dev-bounces at openjdk.java.net> On Behalf
> Of Baesken, Matthias
> Sent: Freitag, 12. Oktober 2018 15:19
> To: Alan Bateman <Alan.Bateman at oracle.com>; security-
> dev at openjdk.java.net; core-libs-dev at openjdk.java.net
> Subject: [CAUTION] RE: RFR: 8211752:
> JNU_ThrowIOExceptionWithLastErrorAndPath - enhance some IOExceptions
> with path causing the issue
> 
> Hi Alan, Goetz,
> 
> >There are several issues with this proposal. The security concern is the main
> one and I think we should wait for comment from security-dev.
>   .....
> 
> >If this is required, I would choose a more generic tag
> 
> >so it can be reused in other places.
> 
> >I would just use "path".
> 
> >
> 
> >Best regards,
> 
> >  Goetz.
> 
> Thanks for  the comments .   Sure,  I can use path  for the category naming  as
> well, but would be good to hear the  opinions of  the security-dev guys on
> this .
> 
> > The second concern is that the patch is incomplete and inconsistent in that
> it changes the exception throw by two methods,
> >it ignores other file I/O methods that throw exceptions.
> 
> We  have these extended exceptions for quite some time in our JVM;   we
> decided to   enhance exceptions where we have a path already "at hand" ,
>   so it was easy to extend the exceptions .
> In case you think this can be done with some others , that's fine with me -  do
> you have some concrete examples ?
> 
> A lot of IO exceptions currently thrown that use
> JNU_ThrowIOExceptionWithLastError  do not have a path (at least not where
> the exception is thrown).
> 
> 
> Best regards ,
>                 Matthias
> 
> 
> 
> From: Alan Bateman <Alan.Bateman at oracle.com>
> Sent: Freitag, 12. Oktober 2018 11:18
> To: Baesken, Matthias <matthias.baesken at sap.com>; security-
> dev at openjdk.java.net; core-libs-dev at openjdk.java.net
> Subject: Re: RFR: 8211752: JNU_ThrowIOExceptionWithLastErrorAndPath -
> enhance some IOExceptions with path causing the issue
> 
> On 12/10/2018 09:12, Baesken, Matthias wrote:
> 
> Ping ...  any reviews / comments ?
> 
> Should I add  a category  , for example    "ioExceptionsWithPath"     to the
> java.security - file  to control  the enabling/disabling of the  enhanced
> exception ?
> 
>   java.security
> 
> #
> # Enhanced exception message information
> ...
> #jdk.includeInExceptions=hostInfo,jar,ioExceptionsWithPath
> 
> There are several issues with this proposal. The security concern is the main
> one and I think we should wait for comment from security-dev. The second
> concern is that the patch is incomplete and inconsistent in that it changes the
> exception throw by two methods, it ignores other file I/O methods that
> throw exceptions. Finally there is the concerns with the patch itself that both
> Roger and I commented on.
> 
> -Alan