From c973151a36ece09f4118836bb05041f4bf0dcc01 Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Tue, 20 Jun 2017 12:42:42 +0200 Subject: [PATCH 1/2] [JENKINS-37566] - Annotate URLish methods and handle NPE in RemoteClassLoader. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NPE seems to be non-realistic (why would RemoteClassloader construct URL from File?), but it’s better to fix it just in case. --- src/main/java/hudson/remoting/RemoteClassLoader.java | 12 +++++++++++- src/main/java/hudson/remoting/URLish.java | 12 ++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/main/java/hudson/remoting/RemoteClassLoader.java b/src/main/java/hudson/remoting/RemoteClassLoader.java index 9b36f2256..e12a892d1 100644 --- a/src/main/java/hudson/remoting/RemoteClassLoader.java +++ b/src/main/java/hudson/remoting/RemoteClassLoader.java @@ -441,6 +441,11 @@ public URL findResource(String name) { } } + /** + * @return {@code null} if one of the URLs cannot be converted. + * E.g. when the referenced file does not exist. + */ + @CheckForNull private static Vector toURLs(Vector src) throws MalformedURLException { Vector r = new Vector(src.size()); for (URLish s : src) { @@ -485,7 +490,12 @@ public Enumeration findResources(String name) throws IOException { } resourcesMap.put(name,v); - return toURLs(v).elements(); + Vector resURLs = toURLs(v); + if (resURLs == null) { + // TODO: Better than NPE, but ideally needs correct error propagation from URLish + throw new IOException("One of the URLish objects cannot be converted to URL"); + } + return resURLs.elements(); } /** diff --git a/src/main/java/hudson/remoting/URLish.java b/src/main/java/hudson/remoting/URLish.java index 569d18a99..7645d7532 100644 --- a/src/main/java/hudson/remoting/URLish.java +++ b/src/main/java/hudson/remoting/URLish.java @@ -3,6 +3,9 @@ import java.io.File; import java.net.MalformedURLException; import java.net.URL; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; /** * Something that's effectively URL. @@ -16,19 +19,24 @@ abstract class URLish { private URLish() {} + @CheckForNull abstract URL toURL() throws MalformedURLException; - static URLish from(final URL url) { + @Nullable + static URLish from(@CheckForNull final URL url) { if (url==null) return null; + return new URLish() { @Override + @Nonnull URL toURL() { return url; } }; } - static URLish from(final File f) { + @Nullable + static URLish from(@CheckForNull final File f) { if (f==null) return null; return new URLish() { @Override From c72260b6126b3e29a1dbc84f7238255967a8eff1 Mon Sep 17 00:00:00 2001 From: Oleg Nenashev Date: Thu, 22 Jun 2017 08:44:33 +0200 Subject: [PATCH 2/2] Make URLish#from() non-null as suggested by @jglick + some Javadoc --- .../java/hudson/remoting/ResourceImageInJar.java | 2 ++ src/main/java/hudson/remoting/URLish.java | 16 +++++++++------- src/main/java/hudson/remoting/Util.java | 1 + 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/main/java/hudson/remoting/ResourceImageInJar.java b/src/main/java/hudson/remoting/ResourceImageInJar.java index 0669ec504..b6d3a372c 100644 --- a/src/main/java/hudson/remoting/ResourceImageInJar.java +++ b/src/main/java/hudson/remoting/ResourceImageInJar.java @@ -4,6 +4,7 @@ import java.net.URL; import java.net.URLClassLoader; import java.util.concurrent.ExecutionException; +import javax.annotation.Nonnull; /** * {@link ResourceImageRef} that points to a resource inside a jar file. @@ -65,6 +66,7 @@ protected URLish adapt(URL jar) throws ExecutionException { }; } + @Nonnull private URL toResourceURL(URL jar, String resourcePath) throws IOException { if (path!=null) resourcePath = path; diff --git a/src/main/java/hudson/remoting/URLish.java b/src/main/java/hudson/remoting/URLish.java index 7645d7532..b076fc7e8 100644 --- a/src/main/java/hudson/remoting/URLish.java +++ b/src/main/java/hudson/remoting/URLish.java @@ -5,7 +5,6 @@ import java.net.URL; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; -import javax.annotation.Nullable; /** * Something that's effectively URL. @@ -19,12 +18,16 @@ abstract class URLish { private URLish() {} + /** + * Converts URLish to the standard {@link URL} type. + * @return URL or {@code null} if the target destination is known to be non-existent. + * @throws MalformedURLException URL cannot be constructed + */ @CheckForNull abstract URL toURL() throws MalformedURLException; - @Nullable - static URLish from(@CheckForNull final URL url) { - if (url==null) return null; + @Nonnull + static URLish from(@Nonnull final URL url) { return new URLish() { @Override @@ -35,9 +38,8 @@ URL toURL() { }; } - @Nullable - static URLish from(@CheckForNull final File f) { - if (f==null) return null; + @Nonnull + static URLish from(@Nonnull final File f) { return new URLish() { @Override URL toURL() throws MalformedURLException { diff --git a/src/main/java/hudson/remoting/Util.java b/src/main/java/hudson/remoting/Util.java index aa292cb1e..62f562a79 100644 --- a/src/main/java/hudson/remoting/Util.java +++ b/src/main/java/hudson/remoting/Util.java @@ -54,6 +54,7 @@ public static void copy(InputStream in, OutputStream out) throws IOException { } } + @Nonnull static File makeResource(String name, byte[] image) throws IOException { File tmpFile = createTempDir(); File resource = new File(tmpFile, name);