FSInfo#getJarClassPath throws an exception not declared in its throws clause

[David Lloyd]

The original bug - the one for which this email thread was begun - was
the URI issue, and the exception issue was merely a side-effect of
that change.  But very well: I'll start a new thread for it.

On Mon, Oct 14, 2019 at 11:14 AM Jonathan Gibbons
<jonathan.gibbons at oracle.com> wrote:
>
> David,
>
> That may be a valid concern, which can be investigated, but that is
> not the focus of this change, which is to avoid the regression of
> unexpected/undocumented exceptions being thrown from the method.
>
> -- Jon
>
> On 10/14/19 7:54 AM, David Lloyd wrote:
> > I'm concerned that this doesn't actually solve the underlying problem
> > of having `Class-Path` entries which are valid per the JAR file
> > specification (i.e. they are relative URLs) but which are invalid
> > paths (due to URL encoding for example, or due to a leading drive
> > letter on a Windows absolute path).
> >
> > On Mon, Oct 14, 2019 at 9:36 AM Jonathan Gibbons
> > <jonathan.gibbons at oracle.com> wrote:
> >> Jaikiran,
> >>
> >> I'll take a look and sponsor for you.
> >>
> >> -- Jon
> >>
> >> On 10/13/19 6:55 PM, Jaikiran Pai wrote:
> >>
> >> Hello Jon,
> >>
> >> Thank you for the review. I have taken your inputs and updated the patch to include this change. I have uploaded that patch as a webrev, in the RFR thread.
> >>
> >> -Jaikiran
> >>
> >> On 14/10/19 1:34 AM, Jonathan Gibbons wrote:
> >>
> >> Or, ...
> >>
> >>   111             for (StringTokenizer st = new StringTokenizer(path);
> >>   112                  st.hasMoreTokens(); ) {
> >>   113                 String elt = st.nextToken();
> >>   115                 try {
> >>   116                     Path f = FileSystems.getDefault().getPath(elt);
> >>   121                     if (!f.isAbsolute() && parent != null)
> >>   122                         f = parent.resolve(f).toAbsolutePath();
> >>   126                     list.add(f);
> >>   123                 } catch (InvalidPathException | IOError e) {
> >>   124                     throw new IOException(e);
> >>   125                 }
> >>   127             }
> >>
> >> -- Jon
> >>
> >>
> >> On 10/13/19 1:00 PM, Jonathan Gibbons wrote:
> >>
> >> Jaikiran,
> >>
> >> A slightly simpler patch with the same effective functionality would be to use a single try-catch block with a multi-catch
> >>
> >>   111             for (StringTokenizer st = new StringTokenizer(path);
> >>   112                  st.hasMoreTokens(); ) {
> >>   113                 String elt = st.nextToken();
> >>   114                 Path f = null;
> >>   115                 try {
> >>   116                     f = FileSystems.getDefault().getPath(elt);
> >>   121                     if (!f.isAbsolute() && parent != null)
> >>   122                         f = parent.resolve(f).toAbsolutePath();
> >>   123                 } catch (InvalidPathException | IOError e) {
> >>   124                     throw new IOException(e);
> >>   125                 }
> >>   126                 list.add(f);
> >>   127             }
> >>   128
> >>
> >> -- Jon
> >>
> >> On 10/12/19 4:33 AM, Jaikiran Pai wrote:
> >>
> >> Thank you Jon. I've created
> >> https://bugs.openjdk.java.net/browse/JDK-8232170 and submitted a RFR
> >> thread with the patch.
> >>
> >> -Jaikiran
> >>
> >> On 11/10/19 8:20 PM, Jonathan Gibbons wrote:
> >>
> >> Jaikiran,
> >>
> >> Not catching InvalidPathException is a bug and an unforeseen
> >> consequence of converting the file manager from using java.io.File
> >> to java.nio.file.Path.
> >>
> >> -- Jon
> >>
> >> On 10/11/19 7:26 AM, Jaikiran Pai wrote:
> >>
> >> In recent versions of JDK (I think after JDK 8), the
> >> com.sun.tools.javac.file.FSInfo#getJarClassPath(...) is throwing a
> >> java.nio.file.InvalidPathException in certain cases when the classpath
> >> entry it is parsing isn't a valid one. This is because of its usage of
> >> the java.nio.file.Path APIs, specifically
> >> https://github.com/openjdk/jdk/blob/4ad3d82c76936a8927ed8a505689a3683144ad98/src/jdk.compiler/share/classes/com/sun/tools/javac/file/FSInfo.java#L112.
> >>
> >>
> >> The throws clause of this FSInfo#getJarClassPath API lists IOException,
> >> so this method is now throwing an exception which isn't listed in its
> >> throws clause. As a result, callers, like the
> >> com.sun.tools.javac.file.Locations.SearchPath#addJarClassPath(...)
> >> https://github.com/openjdk/jdk/blob/4ad3d82c76936a8927ed8a505689a3683144ad98/src/jdk.compiler/share/classes/com/sun/tools/javac/file/Locations.java#L425
> >>
> >> are no longer able to catch this exception and log it and move on.
> >> Instead it gets propagated all the way back to the top level callers,
> >> thus breaking applications which are trying to compile java files
> >> programmatically.
> >>
> >> Would this be considered a bug? If so, I can create a JBS issue and
> >> provide a patch (and will try a jtreg test case too) for review.
> >>
> >> Although these classes are internal and not public API, the calling code
> >> is actually using public APIs (which internally end up calling these
> >> APIs). Like here
> >> https://github.com/quarkusio/quarkus/blob/master/core/devmode/src/main/java/io/quarkus/dev/JavaCompilationProvider.java#L48.
> >>
> >>
> >> For a lengthy discussion/context - please read the comments in this
> >> issue https://github.com/quarkusio/quarkus/issues/3592
> >>
> >> -Jaikiran
> >>
> >>
> >>
> >



-- 
- DML