From 38931f152a79ac8c2ba3b3a3e52b3b02ee56b81d Mon Sep 17 00:00:00 2001
From: Noah Levitt <nlevitt@archive.org>
Date: Fri, 11 Dec 2015 15:42:16 -0800
Subject: [PATCH] handle multiple clauses for same user agent in robots.txt

---
 .../archive/modules/net/RobotsDirectives.java |  8 +++++--
 .../org/archive/modules/net/Robotstxt.java    | 16 ++++++++------
 .../archive/modules/net/RobotstxtTest.java    | 22 +++++++++++++++++++
 3 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/modules/src/main/java/org/archive/modules/net/RobotsDirectives.java b/modules/src/main/java/org/archive/modules/net/RobotsDirectives.java
index 97752a0e..d0ccf9df 100644
--- a/modules/src/main/java/org/archive/modules/net/RobotsDirectives.java
+++ b/modules/src/main/java/org/archive/modules/net/RobotsDirectives.java
@@ -34,7 +34,8 @@ public class RobotsDirectives implements Serializable {
     
     protected ConcurrentSkipListSet<String> disallows = new ConcurrentSkipListSet<String>();
     protected ConcurrentSkipListSet<String> allows = new ConcurrentSkipListSet<String>();
-    protected float crawlDelay = -1; 
+    protected float crawlDelay = -1; 
+    public transient boolean hasDirectives = false;
 
     public boolean allows(String path) {
         return !(longestPrefixLength(disallows, path) > longestPrefixLength(allows, path));
@@ -57,19 +58,22 @@ public class RobotsDirectives implements Serializable {
     }
 
     public void addDisallow(String path) {
+        hasDirectives = true;
         if(path.length()==0) {
             // ignore empty-string disallows 
             // (they really mean allow, when alone)
-            return;
+            return;
         }
         disallows.add(path);
     }
 
     public void addAllow(String path) {
+        hasDirectives = true;
         allows.add(path);
     }
 
     public void setCrawlDelay(float i) {
+        hasDirectives = true;
         crawlDelay=i;
     }
 
diff --git a/modules/src/main/java/org/archive/modules/net/Robotstxt.java b/modules/src/main/java/org/archive/modules/net/Robotstxt.java
index 6ec60571..5cf12187 100644
--- a/modules/src/main/java/org/archive/modules/net/Robotstxt.java
+++ b/modules/src/main/java/org/archive/modules/net/Robotstxt.java
@@ -84,8 +84,6 @@ public class Robotstxt implements Serializable {
         long charCount = 0;
         // current is the disallowed paths for the preceding User-Agent(s)
         RobotsDirectives current = null;
-        // whether a non-'User-Agent' directive has been encountered
-        boolean hasDirectivesYet = false; 
         while (reader != null) {
             // we count characters instead of bytes because the byte count isn't easily available 
             if (charCount >= MAX_SIZE) {
@@ -117,11 +115,18 @@ public class Robotstxt implements Serializable {
                 read = read.trim();
                 if (read.matches("(?i)^User-agent:.*")) {
                     String ua = read.substring(11).trim().toLowerCase();
-                    if (current == null || hasDirectivesYet ) {
+                    RobotsDirectives preexisting;
+                    if (ua.equals("*")) {
+                        preexisting = wildcardDirectives;
+                    } else {
+                        preexisting = agentsToDirectives.get(ua);
+                    }
+                    if (preexisting != null && preexisting.hasDirectives) {
+                        current = preexisting;
+                    } else if (current == null || current.hasDirectives) {
                         // only create new rules-list if necessary
                         // otherwise share with previous user-agent
                         current = new RobotsDirectives();
-                        hasDirectivesYet = false; 
                     }
                     if (ua.equals("*")) {
                         wildcardDirectives = current;
@@ -146,7 +151,6 @@ public class Robotstxt implements Serializable {
                         path = path.substring(0,path.length()-1); 
                     }
                     current.addDisallow(path);
-                    hasDirectivesYet = true; 
                     continue;
                 }
                 if (read.matches("(?i)Crawl-delay:.*")) {
@@ -157,7 +161,6 @@ public class Robotstxt implements Serializable {
                     }
                     // consider a crawl-delay as sufficient to end a grouping of
                     // User-Agent lines
-                    hasDirectivesYet = true;
                     String val = read.substring(12).trim();
                     try {
                         val = val.split("[^\\d\\.]+")[0];
@@ -184,7 +187,6 @@ public class Robotstxt implements Serializable {
                         path = path.substring(0,path.length()-1); 
                     }
                     current.addAllow(path);
-                    hasDirectivesYet = true;
                     continue;
                 }
                 // unknown line; do nothing for now
diff --git a/modules/src/test/java/org/archive/modules/net/RobotstxtTest.java b/modules/src/test/java/org/archive/modules/net/RobotstxtTest.java
index c8083e96..1f529077 100644
--- a/modules/src/test/java/org/archive/modules/net/RobotstxtTest.java
+++ b/modules/src/test/java/org/archive/modules/net/RobotstxtTest.java
@@ -205,4 +205,26 @@ public class RobotstxtTest extends TestCase {
             assertTrue("user-agent a and b shares the same RobotsDirectives after deserialization", da == db);
         }
     }
+
+    public void testSeparatedSections() throws IOException {
+        final String TEST_ROBOTS_TXT = "User-agent: *\n"
+                + "Crawl-delay: 5\n"
+                + "User-agent: a\n"
+                + "Disallow: /\n"
+                + "User-agent: *\n"
+                + "Disallow: /disallowed\n"
+                + "User-agent: a\n"
+                + "Crawl-delay: 99\n";
+        StringReader sr = new StringReader(TEST_ROBOTS_TXT);
+        Robotstxt rt = new Robotstxt(new BufferedReader(sr));
+
+        assertFalse(rt.getDirectivesFor("a").allows("/foo"));
+
+        assertTrue(rt.getDirectivesFor("c").allows("/foo"));
+        assertFalse(rt.getDirectivesFor("c").allows("/disallowed"));
+
+        assertEquals(5f, rt.getDirectivesFor("c").getCrawlDelay());
+
+        assertEquals(99f, rt.getDirectivesFor("a").getCrawlDelay());
+    }
 }