From 05a7c5952a18006d8dc1c6dff20085e198f83f5b Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Mon, 6 Nov 2017 18:24:03 +1100 Subject: [PATCH 1/3] upgrade assembly plugin and use tarLongFileMode posix #1782 (#1943) Signed-off-by: olivier lamy --- jetty-distribution/pom.xml | 2 +- pom.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jetty-distribution/pom.xml b/jetty-distribution/pom.xml index ed33f454a71..3a0a05b4432 100644 --- a/jetty-distribution/pom.xml +++ b/jetty-distribution/pom.xml @@ -606,7 +606,7 @@ src/main/assembly/jetty-assembly.xml - gnu + posix false diff --git a/pom.xml b/pom.xml index e394b5af335..c0971e98978 100644 --- a/pom.xml +++ b/pom.xml @@ -313,7 +313,7 @@ org.apache.maven.plugins maven-assembly-plugin - 3.0.0 + 3.1.0 org.eclipse.jetty.toolchain From 7a6d7bd137ddde68eeee02f8dbaa6c96a9cef760 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 8 Nov 2017 19:03:22 +0100 Subject: [PATCH 2/3] Code reformatting (tabs -> spaces). --- .../jetty/annotations/AnnotationParser.java | 187 +++++++++--------- 1 file changed, 89 insertions(+), 98 deletions(-) diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java index 8409b1769b2..dc0e59e0696 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java +++ b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java @@ -69,14 +69,12 @@ import org.objectweb.asm.Opcodes; public class AnnotationParser { private static final Logger LOG = Log.getLogger(AnnotationParser.class); - protected static int ASM_OPCODE_VERSION = Opcodes.ASM6; //compatibility of api protected Map> _parsedClassNames = new ConcurrentHashMap<>(); private final int _javaPlatform; private int _asmVersion; - - + /** * Determine the runtime version of asm. * @return the org.objectweb.asm.Opcode matching the runtime version of asm. @@ -130,6 +128,7 @@ public class AnnotationParser } return asmVersion; } + /** * Convert internal name to simple name * @@ -357,37 +356,31 @@ public class AnnotationParser @Override public void handle(ClassInfo classInfo) { - //no-op } @Override public void handle(MethodInfo methodInfo) { - // no-op } @Override public void handle(FieldInfo fieldInfo) { - // no-op } @Override public void handle(ClassInfo info, String annotationName) { - // no-op } @Override public void handle(MethodInfo info, String annotationName) { - // no-op } @Override public void handle(FieldInfo info, String annotationName) { - // no-op } } @@ -621,21 +614,21 @@ public class AnnotationParser if (!resolver.isExcluded(className)) { - if (!isParsed(className) || resolver.shouldOverride(className)) - { - String tmp = className; - className = className.replace('.', '/')+".class"; - URL resource = Loader.getResource(this.getClass(),className); - if (resource!= null) - { - Resource r = Resource.newResource(resource); - addParsedClass(tmp, r); - try (InputStream is = r.getInputStream()) - { - scanClass(handlers, null, is); - } - } - } + if (!isParsed(className) || resolver.shouldOverride(className)) + { + String tmp = className; + className = className.replace('.', '/')+".class"; + URL resource = Loader.getResource(this.getClass(),className); + if (resource!= null) + { + Resource r = Resource.newResource(resource); + addParsedClass(tmp, r); + try (InputStream is = r.getInputStream()) + { + scanClass(handlers, null, is); + } + } + } } } @@ -654,23 +647,23 @@ public class AnnotationParser Class cz = clazz; while (cz != Object.class) { - if (!resolver.isExcluded(cz.getName())) - { - if (!isParsed(cz.getName()) || resolver.shouldOverride(cz.getName())) - { - String nameAsResource = cz.getName().replace('.', '/')+".class"; - URL resource = Loader.getResource(this.getClass(),nameAsResource); - if (resource!= null) - { - Resource r = Resource.newResource(resource); - addParsedClass(clazz.getName(), r); - try (InputStream is = r.getInputStream()) - { - scanClass(handlers, null, is); - } - } - } - } + if (!resolver.isExcluded(cz.getName())) + { + if (!isParsed(cz.getName()) || resolver.shouldOverride(cz.getName())) + { + String nameAsResource = cz.getName().replace('.', '/')+".class"; + URL resource = Loader.getResource(this.getClass(),nameAsResource); + if (resource!= null) + { + Resource r = Resource.newResource(resource); + addParsedClass(clazz.getName(), r); + try (InputStream is = r.getInputStream()) + { + scanClass(handlers, null, is); + } + } + } + } if (visitSuperClasses) cz = cz.getSuperclass(); @@ -713,21 +706,21 @@ public class AnnotationParser { try { - if ((resolver == null) || (!resolver.isExcluded(s) && (!isParsed(s) || resolver.shouldOverride(s)))) - { - String name = s; - s = s.replace('.', '/')+".class"; - URL resource = Loader.getResource(this.getClass(),s); - if (resource!= null) - { - Resource r = Resource.newResource(resource); - addParsedClass(name, r); - try (InputStream is = r.getInputStream()) - { - scanClass(handlers, null, is); - } - } - } + if ((resolver == null) || (!resolver.isExcluded(s) && (!isParsed(s) || resolver.shouldOverride(s)))) + { + String name = s; + s = s.replace('.', '/')+".class"; + URL resource = Loader.getResource(this.getClass(),s); + if (resource!= null) + { + Resource r = Resource.newResource(resource); + addParsedClass(name, r); + try (InputStream is = r.getInputStream()) + { + scanClass(handlers, null, is); + } + } + } } catch (Exception e) { @@ -767,33 +760,33 @@ public class AnnotationParser String name = r.getName(); if ((resolver == null)|| (!resolver.isExcluded(name) && (!isParsed(name) || resolver.shouldOverride(name)))) { - File file = r.getFile(); - if (isValidClassFileName((file==null?null:file.getName()))) - { - Path classpath = rootFile.toPath().relativize(file.toPath()); - String str = classpath.toString(); - str = str.substring(0, str.lastIndexOf(".class")).replace('/', '.').replace('\\', '.'); + File file = r.getFile(); + if (isValidClassFileName((file==null?null:file.getName()))) + { + Path classpath = rootFile.toPath().relativize(file.toPath()); + String str = classpath.toString(); + str = str.substring(0, str.lastIndexOf(".class")).replace('/', '.').replace('\\', '.'); - try - { - if (LOG.isDebugEnabled()) - LOG.debug("Scanning class {}", r); - addParsedClass(str, r); - try (InputStream is=r.getInputStream()) - { - scanClass(handlers, Resource.newResource(file.getParentFile()), is); - } - } - catch (Exception ex) - { - if (LOG.isDebugEnabled()) LOG.debug("Error scanning file "+file, ex); - me.add(new RuntimeException("Error scanning file "+file,ex)); - } - } - else - { - if (LOG.isDebugEnabled()) LOG.debug("Skipping scan on invalid file {}", file); - } + try + { + if (LOG.isDebugEnabled()) + LOG.debug("Scanning class {}", r); + addParsedClass(str, r); + try (InputStream is=r.getInputStream()) + { + scanClass(handlers, Resource.newResource(file.getParentFile()), is); + } + } + catch (Exception ex) + { + if (LOG.isDebugEnabled()) LOG.debug("Error scanning file "+file, ex); + me.add(new RuntimeException("Error scanning file "+file,ex)); + } + } + else + { + if (LOG.isDebugEnabled()) LOG.debug("Skipping scan on invalid file {}", file); + } } } } @@ -935,10 +928,8 @@ public class AnnotationParser me.add(new RuntimeException("Error scanning entry " + e.getName() + " from jar " + jarResource, ex)); } }); - - me.ifExceptionThrow(); - } + } } /** @@ -964,19 +955,19 @@ public class AnnotationParser //check file is a valid class file name if (isValidClassFileName(name) && isValidClassFilePath(name)) { - String shortName = name.replace('/', '.').substring(0,name.length()-6); - if ((resolver == null) - || - (!resolver.isExcluded(shortName) && (!isParsed(shortName) || resolver.shouldOverride(shortName)))) - { - addParsedClass(shortName, Resource.newResource("jar:"+jar.getURI()+"!/"+entry.getNameInJar())); - if (LOG.isDebugEnabled()) - LOG.debug("Scanning class from jar {}!/{}", jar, entry); - try (InputStream is = entry.getInputStream()) - { - scanClass(handlers, jar, is); - } - } + String shortName = name.replace('/', '.').substring(0,name.length()-6); + if ((resolver == null) + || + (!resolver.isExcluded(shortName) && (!isParsed(shortName) || resolver.shouldOverride(shortName)))) + { + addParsedClass(shortName, Resource.newResource("jar:"+jar.getURI()+"!/"+entry.getNameInJar())); + if (LOG.isDebugEnabled()) + LOG.debug("Scanning class from jar {}!/{}", jar, entry); + try (InputStream is = entry.getInputStream()) + { + scanClass(handlers, jar, is); + } + } } } From b1d5fea96cd9e8d40fb5446501e2b18cb9971e7b Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 8 Nov 2017 21:38:26 +0100 Subject: [PATCH 3/3] Issue #1797 - JEP 238 - Multi-Release JAR files break bytecode scanning. (#1951) Made MultiReleaseJarFile closeable and using try-with-resources in AnnotationParser to avoid leaking file descriptors. Made few code simplifications. Signed-off-by: Simone Bordet --- .../jetty/annotations/AnnotationParser.java | 22 ++++++++++--------- .../jetty/util/MultiReleaseJarFile.java | 19 +++++++++++----- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java index dc0e59e0696..145db9500bd 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java +++ b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java @@ -916,18 +916,20 @@ public class AnnotationParser LOG.debug("Scanning jar {}", jarResource); MultiException me = new MultiException(); - MultiReleaseJarFile jarFile = new MultiReleaseJarFile(jarResource.getFile(),_javaPlatform,false); - jarFile.stream().forEach(e-> + try (MultiReleaseJarFile jarFile = new MultiReleaseJarFile(jarResource.getFile(),_javaPlatform,false)) { - try + jarFile.stream().forEach(e-> { - parseJarEntry(handlers, jarResource, e, resolver); - } - catch (Exception ex) - { - me.add(new RuntimeException("Error scanning entry " + e.getName() + " from jar " + jarResource, ex)); - } - }); + try + { + parseJarEntry(handlers, jarResource, e, resolver); + } + catch (Exception ex) + { + me.add(new RuntimeException("Error scanning entry " + e.getName() + " from jar " + jarResource, ex)); + } + }); + } me.ifExceptionThrow(); } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiReleaseJarFile.java b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiReleaseJarFile.java index 506731c8f23..3a9c0759e50 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiReleaseJarFile.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiReleaseJarFile.java @@ -18,11 +18,13 @@ package org.eclipse.jetty.util; +import java.io.Closeable; import java.io.File; import java.io.IOException; import java.io.InputStream; import java.util.Collections; import java.util.Iterator; +import java.util.Locale; import java.util.Map; import java.util.TreeMap; import java.util.jar.JarEntry; @@ -33,7 +35,7 @@ import java.util.stream.Stream; /** *

Utility class to handle a Multi Release Jar file

*/ -public class MultiReleaseJarFile +public class MultiReleaseJarFile implements Closeable { private static final String META_INF_VERSIONS = "META-INF/versions/"; @@ -84,12 +86,10 @@ public class MultiReleaseJarFile { Map.Entry e = i.next(); VersionedJarEntry entry = e.getValue(); - if (entry.inner) { VersionedJarEntry outer = entry.outer==null?null:map.get(entry.outer); - - if (entry.outer==null || outer==null || outer.version!= entry.version) + if (outer==null || outer.version!=entry.version) i.remove(); } } @@ -130,6 +130,13 @@ public class MultiReleaseJarFile return entries.get(name); } + @Override + public void close() throws IOException + { + if (jarFile!=null) + jarFile.close(); + } + @Override public String toString() { @@ -172,8 +179,8 @@ public class MultiReleaseJarFile this.entry = entry; this.name = name; this.version = v; - this.inner = name.contains("$") && name.toLowerCase().endsWith(".class"); - this.outer = inner ? name.substring(0, name.indexOf('$')) + name.substring(name.length() - 6, name.length()) : null; + this.inner = name.contains("$") && name.toLowerCase(Locale.ENGLISH).endsWith(".class"); + this.outer = inner ? name.substring(0, name.indexOf('$')) + ".class" : null; } /**