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-boun...@openjdk.java.net> On Behalf > Of Baesken, Matthias > Sent: Freitag, 12. Oktober 2018 15:19 > To: Alan Bateman <alan.bate...@oracle.com>; security- > d...@openjdk.java.net; core-libs-...@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.bate...@oracle.com> > Sent: Freitag, 12. Oktober 2018 11:18 > To: Baesken, Matthias <matthias.baes...@sap.com>; security- > d...@openjdk.java.net; core-libs-...@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
