Merge remote-tracking branch 'origin/jetty-8'

Conflicts:
	jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java

2 files changed, 231 insertions, 122 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 261badb7c4..c8048f462e 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
@@ -753,6 +753,7 @@ public class AnnotationParser
     public void parseDir (Resource dir, ClassNameResolver resolver)
     throws Exception
     {
+        //skip dirs whose name start with . (ie hidden)
         if (!dir.isDirectory() || !dir.exists() || dir.getName().startsWith("."))
             return;
 
@@ -766,16 +767,21 @@ public class AnnotationParser
                 Resource res = dir.addPath(files[f]);
                 if (res.isDirectory())
                     parseDir(res, resolver);
-                String name = res.getName();
-                if (isValidClassFileName(name))
+                else
                 {
-                    if ((resolver == null)|| (!resolver.isExcluded(name) && (!isParsed(name) || resolver.shouldOverride(name))))
+                    //we've already verified the directories, so just verify the class file name
+                    String filename = res.getFile().getName();
+                    if (isValidClassFileName(filename))
                     {
-                        Resource r = Resource.newResource(res.getURL());
-                        if (LOG.isDebugEnabled()) {LOG.debug("Scanning class {}", r);};
-                        scanClass(r.getInputStream());
-                    }
+                        String name = res.getName();
+                        if ((resolver == null)|| (!resolver.isExcluded(name) && (!isParsed(name) || resolver.shouldOverride(name))))
+                        {
+                            Resource r = Resource.newResource(res.getURL());
+                            if (LOG.isDebugEnabled()) {LOG.debug("Scanning class {}", r);};
+                            scanClass(r.getInputStream());
+                        }
 
+                    }
                 }
             }
             catch (Exception ex)
@@ -812,27 +818,11 @@ public class AnnotationParser
             {
                 try
                 {
-                    //skip directories
-                    if (entry.isDirectory())
-                        return;
-                    
-                    String name = entry.getName();
-                    if (isValidClassFileName(name))
-                    {
-                        String shortName =  name.replace('/', '.').substring(0,name.length()-6);
-                        if ((resolver == null)
-                             ||
-                            (!resolver.isExcluded(shortName) && (!isParsed(shortName) || resolver.shouldOverride(shortName))))
-                        {
-
-                            Resource clazz = Resource.newResource("jar:"+jarUri+"!/"+name);
-                            scanClass(clazz.getInputStream());
-                        }
-                    }
+                    parseJarEntry(jarUri, entry, resolver);
                 }
                 catch (Exception e)
                 {
-                    LOG.warn("Problem processing jar entry "+entry, e);
+                    LOG.warn("Problem parsing jar entry: {}", entry.getName());
                 }
             }
 
@@ -900,6 +890,8 @@ public class AnnotationParser
             scanClass(r.getInputStream());
             return;
         }
+        
+        if (LOG.isDebugEnabled()) LOG.warn("Resource not scannable for classes: {}", uri);
     }
 
 
@@ -933,34 +925,8 @@ public class AnnotationParser
             { 
                 JarEntry entry = jar_in.getNextJarEntry();
                 while (entry!=null)
-                {                   
-                    //skip directories
-                    if (!entry.isDirectory())
-                    {
-                        try
-                        {
-                            String name = entry.getName();
-
-                            //skip any class files that are in a hidden directory (ie dirname starts with .) 
-                            if (isValidClassFileName(name))
-                            {
-                                String shortName =  name.replace('/', '.').substring(0,name.length()-6);
-
-                                if ((resolver == null)
-                                        ||
-                                        (!resolver.isExcluded(shortName) && (!isParsed(shortName) || resolver.shouldOverride(shortName))))
-                                {
-                                    Resource clazz = Resource.newResource("jar:"+uri+"!/"+name);
-                                    if (LOG.isDebugEnabled()) {LOG.debug("Scanning class from jar {}", clazz);};
-                                    scanClass(clazz.getInputStream());
-                                }
-                            }
-                        }
-                        catch (Exception e)
-                        {
-                            LOG.warn("Problem processing jar entry "+entry, e);
-                        }
-                    }
+                {      
+                    parseJarEntry(uri, entry, resolver);
                     entry = jar_in.getNextJarEntry();
                 }
             }
@@ -970,8 +936,44 @@ public class AnnotationParser
             } 
         }   
     }
+
+    /**
+     * Parse a single entry in a jar file
+     * @param jar
+     * @param entry
+     * @param resolver
+     * @throws Exception
+     */
+    protected void parseJarEntry (URI jar, JarEntry entry, final ClassNameResolver resolver)
+    throws Exception
+    {
+        if (jar == null || entry == null)
+            return;
+
+        //skip directories
+        if (entry.isDirectory())
+            return;
+
+        String name = entry.getName();
+
+        //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))))
+            {
+                Resource clazz = Resource.newResource("jar:"+jar+"!/"+name);
+                if (LOG.isDebugEnabled()) {LOG.debug("Scanning class from jar {}", clazz);};
+                scanClass(clazz.getInputStream());
+            }
+        }
+    }
     
     
+
     /**
      * Use ASM on a class
      * 
@@ -993,27 +995,56 @@ public class AnnotationParser
      * <li> it isn't a dot file or in a hidden directory </li>
      * <li> the name of the class at least begins with a valid identifier for a class name </li>
      * </ul>
-     * @param path
+     * @param name
      * @return
      */
-    private boolean isValidClassFileName (String path)
+    private boolean isValidClassFileName (String name)
     {
+        //no name cannot be valid
+        if (name == null || name.length()==0)
+            return false;
+
         //skip anything that is not a class file
-        if (!path.toLowerCase(Locale.ENGLISH).endsWith(".class"))
+        if (!name.toLowerCase(Locale.ENGLISH).endsWith(".class"))
+        {
+            if (LOG.isDebugEnabled()) LOG.debug("Not a class: {}",name);
             return false;
-        
-        //skip any classfiles that are not a valid name
+        }
+
+        //skip any classfiles that are not a valid java identifier
         int c0 = 0;      
-        int ldir = path.lastIndexOf('/', path.length()-6);
+        int ldir = name.lastIndexOf('/', name.length()-6);
         c0 = (ldir > -1 ? ldir+1 : c0);
-        
-        if (!Character.isJavaIdentifierStart(path.charAt(c0)))
+        if (!Character.isJavaIdentifierStart(name.charAt(c0)))
+        {
+            if (LOG.isDebugEnabled()) LOG.debug("Not a java identifier: {}"+name);
             return false;
-        
+        }
+   
+        return true;
+    }
+    
+    
+    /**
+     * Check that the given path does not contain hidden directories
+     *
+     * @param path
+     * @return
+     */
+    private boolean isValidClassFilePath (String path)
+    {
+        //no path is not valid
+        if (path == null || path.length()==0)
+            return false;
+
+
         //skip any classfiles that are in a hidden directory
         if (path.startsWith(".") || path.contains("/."))
+        {
+            if (LOG.isDebugEnabled()) LOG.debug("Contains hidden dirs: {}"+path);
             return false;
-        
+        }
+
         return true;
     }
 }
diff --git a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationParser.java b/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationParser.java
index 8deecacef4..5fb97a30de 100644
--- a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationParser.java
+++ b/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationParser.java
@@ -18,73 +18,109 @@
 
 package org.eclipse.jetty.annotations;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
 
 import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.URL;
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import org.eclipse.jetty.annotations.AnnotationParser.DiscoverableAnnotationHandler;
 import org.eclipse.jetty.annotations.AnnotationParser.Value;
+import org.eclipse.jetty.toolchain.test.FS;
+import org.eclipse.jetty.toolchain.test.IO;
 import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
+import org.eclipse.jetty.toolchain.test.TestingDir;
+import org.junit.Assert;
+import org.junit.Rule;
 import org.junit.Test;
 
 public class TestAnnotationParser
 {
+    public static class TrackingAnnotationHandler implements DiscoverableAnnotationHandler
+    {
+        private final String annotationName;
+        public final Set<String> foundClasses;
+
+        public TrackingAnnotationHandler(String annotationName)
+        {
+            this.annotationName = annotationName;
+            this.foundClasses = new HashSet<>();
+        }
+
+        @Override
+        public void handleClass(String className, int version, int access, String signature, String superName, String[] interfaces, String annotation,
+                List<Value> values)
+        {
+            foundClasses.add(className);
+        }
+
+        @Override
+        public void handleMethod(String className, String methodName, int access, String desc, String signature, String[] exceptions, String annotation,
+                List<Value> values)
+        {
+            /* ignore */
+        }
+
+        @Override
+        public void handleField(String className, String fieldName, int access, String fieldType, String signature, Object value, String annotation,
+                List<Value> values)
+        {
+            /* ignore */
+        }
+
+        @Override
+        public String getAnnotationName()
+        {
+            return this.annotationName;
+        }
+    }
+
+    @Rule
+    public TestingDir testdir = new TestingDir();
+
     @Test
     public void testSampleAnnotation() throws Exception
     {
-        String[] classNames = new String[]{"org.eclipse.jetty.annotations.ClassA"};
+        String[] classNames = new String[]
+        { "org.eclipse.jetty.annotations.ClassA" };
         AnnotationParser parser = new AnnotationParser();
 
         class SampleAnnotationHandler implements DiscoverableAnnotationHandler
         {
-            private List<String> methods = Arrays.asList("a", "b", "c", "d", "l");
+            private List<String> methods = Arrays.asList("a","b","c","d","l");
 
-           
-            
-            
             public void handleClass(String className, int version, int access, String signature, String superName, String[] interfaces, String annotation,
-                                    List<Value> values)
+                    List<Value> values)
             {
-                assertEquals ("org.eclipse.jetty.annotations.ClassA", className);
+                assertEquals("org.eclipse.jetty.annotations.ClassA",className);
             }
 
             public void handleField(String className, String fieldName, int access, String fieldType, String signature, Object value, String annotation,
-                                   List<Value> values)
+                    List<Value> values)
             {
-              assertEquals ("m", fieldName);
-              assertEquals (org.objectweb.asm.Type.OBJECT, org.objectweb.asm.Type.getType(fieldType).getSort());
-              assertEquals (1, values.size());
-              Value anv1 = values.get(0);
-              assertEquals ("value", anv1.getName());
-              assertEquals (7, anv1.getValue());
+                assertEquals("m",fieldName);
+                assertEquals(org.objectweb.asm.Type.OBJECT,org.objectweb.asm.Type.getType(fieldType).getSort());
+                assertEquals(1,values.size());
+                Value anv1 = values.get(0);
+                assertEquals("value",anv1.getName());
+                assertEquals(7,anv1.getValue());
 
             }
 
             public void handleMethod(String className, String methodName, int access, String desc, String signature, String[] exceptions, String annotation,
-                                     List<Value> values)
+                    List<Value> values)
             {
-                System.err.println("Sample annotated method : classname="+className+" methodName="+methodName+" access="+access+" desc="+desc+" signature="+signature);
-
-                org.objectweb.asm.Type retType = org.objectweb.asm.Type.getReturnType(desc);
-                System.err.println("REturn type = "+retType);
-                org.objectweb.asm.Type[] params = org.objectweb.asm.Type.getArgumentTypes(desc);
-                if (params == null)
-                    System.err.println("No params");
-                else
-                    System.err.println(params.length+" params");
-
-                if (exceptions == null)
-                    System.err.println("No exceptions");
-                else
-                    System.err.println(exceptions.length+" exceptions");
-
-                assertEquals("org.eclipse.jetty.annotations.ClassA", className);
+                assertEquals("org.eclipse.jetty.annotations.ClassA",className);
                 assertTrue(methods.contains(methodName));
-                assertEquals("org.eclipse.jetty.annotations.Sample", annotation);
+                assertEquals("org.eclipse.jetty.annotations.Sample",annotation);
             }
 
             @Override
@@ -97,7 +133,7 @@ public class TestAnnotationParser
         parser.registerHandler(new SampleAnnotationHandler());
 
         long start = System.currentTimeMillis();
-        parser.parse(classNames, new ClassNameResolver ()
+        parser.parse(classNames,new ClassNameResolver()
         {
             public boolean isExcluded(String name)
             {
@@ -112,44 +148,36 @@ public class TestAnnotationParser
         });
         long end = System.currentTimeMillis();
 
-        System.err.println("Time to parse class: "+((end-start)));
+        //System.err.println("Time to parse class: " + ((end - start)));
     }
 
     @Test
     public void testMultiAnnotation() throws Exception
     {
-        String[] classNames = new String[]{"org.eclipse.jetty.annotations.ClassB"};
+        String[] classNames = new String[]
+        { "org.eclipse.jetty.annotations.ClassB" };
         AnnotationParser parser = new AnnotationParser();
 
         class MultiAnnotationHandler implements DiscoverableAnnotationHandler
         {
             public void handleClass(String className, int version, int access, String signature, String superName, String[] interfaces, String annotation,
-                                    List<Value> values)
+                    List<Value> values)
             {
                 assertTrue("org.eclipse.jetty.annotations.ClassB".equals(className));
-
-                for (Value anv: values)
-                {
-                   System.err.println(anv.toString());
-                }
             }
 
             public void handleField(String className, String fieldName, int access, String fieldType, String signature, Object value, String annotation,
-                                    List<Value> values)
+                    List<Value> values)
             {
-                //there should not be any
+                // there should not be any
                 fail();
             }
 
             public void handleMethod(String className, String methodName, int access, String params, String signature, String[] exceptions, String annotation,
-                                     List<Value> values)
+                    List<Value> values)
             {
                 assertTrue("org.eclipse.jetty.annotations.ClassB".equals(className));
                 assertTrue("a".equals(methodName));
-                for (Value anv: values)
-                {
-                    System.err.println(anv.toString());
-                }
             }
 
             @Override
@@ -157,21 +185,71 @@ public class TestAnnotationParser
             {
                 return "org.eclipse.jetty.annotations.Multi";
             }
-            
-            
+
         }
 
         parser.registerHandler(new MultiAnnotationHandler());
-        parser.parse(classNames, null);
+        parser.parse(classNames,null);
     }
-    
-    
+
     @Test
-    public void testHiddenFilesInJar () throws Exception
+    public void testHiddenFilesInJar() throws Exception
     {
         File badClassesJar = MavenTestingUtils.getTestResourceFile("bad-classes.jar");
         AnnotationParser parser = new AnnotationParser();
-        parser.parse(badClassesJar.toURI(), null);
-        //only the valid classes inside bad-classes.jar should be parsed. If any invalid classes are parsed and exception would be thrown here
+        parser.parse(badClassesJar.toURI(),null);
+        // only the valid classes inside bad-classes.jar should be parsed. If any invalid classes are parsed and exception would be thrown here
+    }
+
+    @Test
+    public void testBasedirExclusion() throws Exception
+    {
+        // Build up basedir, which itself has a path segment that violates java package and classnaming.
+        // The basedir should have no effect on annotation scanning.
+        // Intentionally using a base director name that starts with a "."
+        // This mimics what you see in jenkins, hudson, hadoop, solr, camel, and selenium for their 
+        // installed and/or managed webapps
+        File basedir = testdir.getFile(".base/workspace/classes");
+        FS.ensureEmpty(basedir);
+
+        // Copy in class that is known to have annotations.
+        copyClass(ClassA.class,basedir);
+
+        // Setup Tracker
+        TrackingAnnotationHandler tracker = new TrackingAnnotationHandler(Sample.class.getName());
+
+        // Setup annotation scanning
+        AnnotationParser parser = new AnnotationParser();
+        parser.registerHandler(tracker);
+        
+        // Parse
+        parser.parse(basedir.toURI(),null);
+        
+        // Validate
+        Assert.assertThat("Found Class", tracker.foundClasses, contains(ClassA.class.getName()));
+    }
+
+    private void copyClass(Class<?> clazz, File basedir) throws IOException
+    {
+        String classname = clazz.getName().replace('.',File.separatorChar) + ".class";
+        URL url = this.getClass().getResource('/'+classname);
+        Assert.assertThat("URL for: " + classname,url,notNullValue());
+
+        String classpath = classname.substring(0,classname.lastIndexOf(File.separatorChar));
+        FS.ensureDirExists(new File(basedir,classpath));
+
+        InputStream in = null;
+        OutputStream out = null;
+        try
+        {
+            in = url.openStream();
+            out = new FileOutputStream(new File(basedir,classname));
+            IO.copy(in,out);
+        }
+        finally
+        {
+            IO.close(out);
+            IO.close(in);
+        }
     }
 }