Merge pull request #4 from srivaths/master

Fixed content path matching edge case bug

pom.xml

@@ -3,7 +3,7 @@
   <modelVersion>4.0.0</modelVersion>
   <groupId>com.redhat.trie</groupId>
   <artifactId>PathPacker</artifactId>
-  <version>0.0.2</version>
+  <version>0.0.3</version>
   <packaging>bundle</packaging>
 
   <prerequisites>
@@ -22,6 +22,11 @@
       <artifactId>bcprov-jdk16</artifactId>
       <version>1.46</version>
     </dependency>
+    <dependency>
+      <groupId>org.apache.maven.surefire</groupId>
+      <artifactId>surefire-booter</artifactId>
+      <version>2.17</version>
+    </dependency>
 
     <dependency>
       <groupId>log4j</groupId>

src/main/java/com/redhat/trie/NodePair.java

@@ -50,5 +50,13 @@ public class NodePair implements Comparable {
     public int compareTo(Object other) {
         return this.name.compareTo(((NodePair) other).name);
     }
+    
+    /**
+     * Does this <tt>NodePair</tt> have any children?
+     * @return <tt>true</tt> if it has at least one connection and <tt>false</tt> otherwise.
+     */
+    public boolean hasNoChildren() {
+      return getConnection().getChildren().size() == 0;
+    }
 }
 

src/main/java/com/redhat/trie/PathNode.java

@@ -28,7 +28,7 @@ import org.apache.log4j.Logger;
  *
  * It holds the relationships to children, as well as all parents that regard it as a child.
  *
- * The Name of a given PathNode, is inferred by the NodePair that regards this PathNode as it's "connection"
+ * The Name of a given PathNode, is inferred by the NodePair that regards this PathNode as its "connection"
  */
 public class PathNode {
     private static org.apache.log4j.Logger log = Logger.getLogger(PathTree.class);

src/main/java/com/redhat/trie/PathTree.java

@@ -311,36 +311,52 @@ public class PathTree {
      * @param  contentPath     A String, like "/foo/bar/baz"
      * @return true or false
      */
-    public boolean validate(String contentPath) {
+    public boolean validate(final String contentPath) {
-        StringTokenizer st = new StringTokenizer(contentPath, "/");
+      PathNode rootPathNode = null;
-        PathNode root;
-        PathNode pn;
-        String curTok;
-
       try {
-            root = this.getRootPathNode();
+        rootPathNode = getRootPathNode();
-        } catch (PayloadException ex) {
+      } catch(PayloadException pe) {
-            log.error(ex);
+        log.error(pe);
         return false;
       }
+      return test(contentPath, rootPathNode);
+    }
 
-        pn = root;
+    /** Character used to delimit client path request elements */
-        while (st.hasMoreTokens()) {
+    private static String PATH_DELIMITER = "/";
-            curTok = st.nextToken();
+    /** Character used as a variable name prefix in content path definitions */
-
+    private static String CONTENT_PATH_VARIABLE_PREFIX = "$";
-            for (NodePair np : pn.getChildren()) {
+    /**
-                if (curTok.equals(np.getName()) || np.getName().startsWith("$")) {
+     * Tests if the given path request is reachable via the current <tt>tree.</tt>.
-                    //System.out.println("[" + curTok + "] == [" + np.getName() + "]");
+     * @param request The request to test.
-                    if (np.getConnection().getChildren().size() == 0) {
+     * @param tree The content path tree.
+     * @return <tt>true</tt> if the path is reachable and <tt>false</tt> otherwise.
+     */
+    private boolean test(final String request, final PathNode tree) {
+      /* Request is of the form "/content/rc/rhel/7/..." 
+       * Grab the next element.
+       */
+      log.debug("test(" + request + ")");
+      StringTokenizer tokenizer = new StringTokenizer(request, PATH_DELIMITER);
+      if(tokenizer.countTokens() == 0) {
+        return false;
+      }
+      String currentToken = tokenizer.nextToken();
+      for(NodePair nodePair: tree.getChildren()) {
+        String nodePairName = nodePair.getName();
+        log.debug("Current token: [" + currentToken + "] =??= NodePair name: [" + nodePairName + "]");
+        if(currentToken.equals(nodePairName) || nodePairName.startsWith(CONTENT_PATH_VARIABLE_PREFIX)) {
+          if(nodePair.hasNoChildren()) {
+            return true;
+          } else {
+            String s = PATH_DELIMITER + currentToken;
+            boolean retval = test(request.substring(currentToken.length()+1), nodePair.getConnection());
+            if(retval) {
               return true;
             }
-
-                    pn = np.getConnection();
-                    break;
           }
         }
       }
-
       return false;
     }
 

src/test/java/com/redhat/trie/TestHelpers.java

@@ -16,11 +16,13 @@ import static org.junit.Assert.fail;
 import static org.junit.Assert.assertNotNull;
 
 import org.junit.Test;
+import org.apache.log4j.Logger;
 
 /**
  * This class is just to provide helpers for the other tests
  */
 public class TestHelpers {
+  private static org.apache.log4j.Logger log = Logger.getLogger(TestHelpers.class);
 
     /**
      * junit requires at least one runnable test
@@ -33,8 +35,8 @@ public class TestHelpers {
     // Helpers
     // 
     public static boolean cmpStrings(List<String> thisList, List<String> thatList) {
-        Collection<String> thisColl = new ArrayList(thisList);
+        Collection<String> thisColl = new ArrayList<String>(thisList);
-        Collection<String> thatColl = new ArrayList(thatList);
+        Collection<String> thatColl = new ArrayList<String>(thatList);
 
         Collection<String> similar = new HashSet<String>( thisColl );
         Collection<String> different = new HashSet<String>();
@@ -91,7 +93,14 @@ public class TestHelpers {
         String content;
         List<String> contentList = new ArrayList<String>();
         InputStream in = resStream(klass, filename);
-        BufferedReader br = new BufferedReader(new InputStreamReader(in));
+        BufferedReader br = null;
+        try {
+          br = new BufferedReader(new InputStreamReader(in));
+        } catch(NullPointerException npe) {
+          // can happen in the case of an empty content set list
+          log.warn(">>>>>>> Empty content set <<<<<");
+          return contentList;
+        }
 
         try {
             while ((content = br.readLine()) != null) {

src/test/java/com/redhat/trie/TestValidater.java

@@ -0,0 +1,106 @@
+package com.redhat.trie;
+
+import static org.junit.Assert.*;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+
+import org.apache.log4j.Logger;
+import org.apache.log4j.Level;
+
+import org.junit.Test;
+import org.junit.Before;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+/**
+ * Tests of whether a URL should be granted access based on a given content set
+ * (as would be specified in the requester's entitlement certificate).
+ *
+ */
+@RunWith(value = Parameterized.class)
+public class TestValidater {
+  private String contentSetFileName;
+  private String requestedPath;
+  private boolean expectedResult;
+  private PathTree pathTree;
+
+  @Parameters
+  public static Collection<Object[]> data() {
+    /*
+     * Data for each run of the test.
+     * [0] = The name of the file containing content set paths
+     * [1] = The requested resource
+     * [3] = The expected result.
+     */
+    Object[][] data = new Object[][] {
+      /*
+       * WARNING -- 
+       *   Do not uncomment empty.contents.list cases unless
+       *   PathTree has been updated to handle that case.
+       *
+      { "empty.contents.list", "/content/rc/rhel/7/vt", false},
+      { "empty.contents.list", "/content/rc/rhel/7", false},
+       */
+      { "contents.list", "/content/beta/rhel/server/5/5server/x86_64/sap/os/repomd.xml", true},
+      { "contents.list", "/fart/face/mcjones", false},
+      { "contents.list", "/content/dist/rhel/server/6/$releasever/$basearch/vt/os", false},
+
+      { "variables-last-contents.list", "/content/rc/rhel/server/7/x86_64/debug", true},
+      { "variables-last-contents.list", "/content/rc/rhel/server/7/x86_64/debug/tools/firefox", true},
+      { "variables-last-contents.list", "/content/rc/rhel/server/7/x86_64/Debug", false},
+      { "variables-last-contents.list", "/content/rc/rhel/server/7/Debug", false},
+      { "variables-last-contents.list", "/content/rc/rhel/server/7/x86_64/os", true},
+
+      { "variables-first-contents.list", "/content/rc/rhel/server/7/x86_64/debug", true},
+      { "variables-first-contents.list", "/content/rc/rhel/server/7/x86_64/debug/tools/firefox", true},
+      { "variables-first-contents.list", "/content/rc/rhel/server/7/x86_64/Debug", false},
+      { "variables-first-contents.list", "/content/rc/rhel/server/7/Debug", false},
+      { "variables-first-contents.list", "/content/rc/rhel/server/7/x86_64/os", true},
+
+      { "variables-mixed-contents.list", "/content/rc/rhel/server/7/x86_64/debug", true},
+      { "variables-mixed-contents.list", "/content/rc/rhel/server/7/x86_64/debug/tools/firefox", true},
+      { "variables-mixed-contents.list", "/content/rc/rhel/server/7/x86_64/Debug", false},
+      { "variables-mixed-contents.list", "/content/rc/rhel/server/7/Debug", false},
+      { "variables-mixed-contents.list", "/content/rc/rhel/server/7/x86_64/os", true},
+
+      { "another-contents.list", "/content/rc/rhel/server/7/x86_64/os", true},
+      { "another-contents.list", "/content/rc/rhel/server/7/x86_64/debug/os", false}
+    };
+    return Arrays.asList(data);
+  }
+
+  /**
+   * Instatiate a test class with the given parameters.
+   * @param contentSetFileName The name of the file containing the allowed content sets.
+   * @param requestedPath The resource being requested
+   * @param expectedResult The correct response.
+   */
+  public TestValidater(final String contentSetFileName, final String requestedPath, final boolean expectedResult) {
+    this.contentSetFileName = contentSetFileName;
+    this.requestedPath = requestedPath;
+    this.expectedResult = expectedResult;
+  }
+
+  /**
+   * Prepare for a test run.
+   */
+  @Before
+  public void setup() throws Exception {
+    List<String> validContentPaths = TestHelpers.loadContents(this, contentSetFileName);
+    pathTree = new PathTree();
+    pathTree.setContentSets(validContentPaths);
+    assertTrue(TestHelpers.cmpStrings(validContentPaths, pathTree.toList()));
+  }
+
+  /**
+   * Run the test.
+   */
+  @Test
+  public void justDoIt() throws Exception {
+    assertEquals("[" + requestedPath + "] " + (expectedResult?"allowed ":"not allowed ") + "using " + contentSetFileName, 
+        expectedResult, pathTree.validate(requestedPath));
+  }
+}

src/test/resources/another-contents.list

@@ -0,0 +1 @@
+/content/rc/rhel/server/$releasever/$basearch/os

src/test/resources/log4j.properties

@@ -0,0 +1,4 @@
+log4j.rootLogger=DEBUG, R
+log4j.appender.R=org.apache.log4j.ConsoleAppender
+log4j.appender.R.layout=org.apache.log4j.PatternLayout
+log4j.appender.R.layout.ConversionPattern=[%-5p] %c{1} - %m%n

src/test/resources/variables-first-contents.list

@@ -0,0 +1,16 @@
+/content/rc/rhel/server/7/$basearch/highavailability/debug
+/content/rc/rhel/server/7/$basearch/highavailability/os
+/content/rc/rhel/server/7/$basearch/highavailability/source/SRPMS
+/content/rc/rhel/server/7/$basearch/resilientstorage/debug
+/content/rc/rhel/server/7/$basearch/resilientstorage/os
+/content/rc/rhel/server/7/$basearch/resilientstorage/source/SRPMS
+/content/rc/rhel/server/7/x86_64/debug
+/content/rc/rhel/server/7/x86_64/iso
+/content/rc/rhel/server/7/x86_64/kickstart
+/content/rc/rhel/server/7/x86_64/os
+/content/rc/rhel/server/7/x86_64/source/iso
+/content/rc/rhel/server/7/x86_64/source/SRPMS
+/content/rc/rhel/server/7/x86_64/supplementary/debug
+/content/rc/rhel/server/7/x86_64/supplementary/iso
+/content/rc/rhel/server/7/x86_64/supplementary/os
+/content/rc/rhel/server/7/x86_64/supplementary/source/SRPMS

src/test/resources/variables-last-contents.list

@@ -0,0 +1,16 @@
+/content/rc/rhel/server/7/x86_64/debug
+/content/rc/rhel/server/7/x86_64/iso
+/content/rc/rhel/server/7/x86_64/kickstart
+/content/rc/rhel/server/7/x86_64/os
+/content/rc/rhel/server/7/x86_64/source/iso
+/content/rc/rhel/server/7/x86_64/source/SRPMS
+/content/rc/rhel/server/7/x86_64/supplementary/debug
+/content/rc/rhel/server/7/x86_64/supplementary/iso
+/content/rc/rhel/server/7/x86_64/supplementary/os
+/content/rc/rhel/server/7/x86_64/supplementary/source/SRPMS
+/content/rc/rhel/server/7/$basearch/highavailability/debug
+/content/rc/rhel/server/7/$basearch/highavailability/os
+/content/rc/rhel/server/7/$basearch/highavailability/source/SRPMS
+/content/rc/rhel/server/7/$basearch/resilientstorage/debug
+/content/rc/rhel/server/7/$basearch/resilientstorage/os
+/content/rc/rhel/server/7/$basearch/resilientstorage/source/SRPMS

src/test/resources/variables-mixed-contents.list

@@ -0,0 +1,16 @@
+/content/rc/rhel/server/7/$basearch/highavailability/debug
+/content/rc/rhel/server/7/$basearch/resilientstorage/debug
+/content/rc/rhel/server/7/$basearch/resilientstorage/source/SRPMS
+/content/rc/rhel/server/7/x86_64/debug
+/content/rc/rhel/server/7/$basearch/highavailability/os
+/content/rc/rhel/server/7/$basearch/highavailability/source/SRPMS
+/content/rc/rhel/server/7/x86_64/iso
+/content/rc/rhel/server/7/x86_64/kickstart
+/content/rc/rhel/server/7/x86_64/os
+/content/rc/rhel/server/7/x86_64/source/iso
+/content/rc/rhel/server/7/x86_64/source/SRPMS
+/content/rc/rhel/server/7/$basearch/resilientstorage/os
+/content/rc/rhel/server/7/x86_64/supplementary/debug
+/content/rc/rhel/server/7/x86_64/supplementary/iso
+/content/rc/rhel/server/7/x86_64/supplementary/os
+/content/rc/rhel/server/7/x86_64/supplementary/source/SRPMS