Repository: crawler-commons/crawler-commons
Pull request #428 (2023-06-16 10:42:01)

[Robots.txt] SimpleRobotRulesParser main() to follow five redirects
Follow "five consecutive redirects" when fetching robots.txt over HTTP as required by [RFC 9309](https://www.rfc-editor.org/rfc/rfc9309.html#name-redirects).


Patch:
diff --git a/CHANGES.txt b/CHANGES.txt
index cf3720f..93aaeef 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,6 +1,9 @@
 Crawler-Commons Change Log
 
 Current Development 1.4-SNAPSHOT (yyyy-mm-dd)
+- [Robots.txt] Document effect of rules merging in combination with multiple agent names (sebastian-nagel, Richard Zowalla) #423, #426
+- [Robots.txt] Pass empty collection of agent names to select rules for any robot (wildcard user-agent name) (sebastian-nagel, Richard Zowalla) #427
+- [Robots.txt] Rename default user-agent / robot name in unit tests (sebastian-nagel, Richard Zowalla) #429
 - [Robots.txt] Add units test based on examples in RFC 9309 (sebastian-nagel, Richard Zowalla) #420
 - [BasicNormalizer] Query parameters normalization in BasicURLNormalizer (aecio, sebastian-nagel, Richard Zowalla) #308, #421
 - [Robots.txt] Deduplicate robots rules before matching (sebastian-nagel, jnioche) #416
diff --git a/src/main/java/crawlercommons/robots/SimpleRobotRulesParser.java b/src/main/java/crawlercommons/robots/SimpleRobotRulesParser.java
index 2383d88..7b932b1 100644
--- a/src/main/java/crawlercommons/robots/SimpleRobotRulesParser.java
+++ b/src/main/java/crawlercommons/robots/SimpleRobotRulesParser.java
@@ -21,6 +21,7 @@ import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLConnection;
+import java.net.URLDecoder;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
@@ -139,18 +140,26 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
 
         ALLOW, CRAWL_DELAY, SITEMAP,
 
-        // Russian-specific directive for mirror site?
-        // Used by the zerkalschik robot?
-        // See http://wataro.ur/en/web/robot.html
+        /**
+         * The &quot;Host&quot; directive was used by Yandex to indicate the
+         * main or canonical host of a set of mirrored web sites, see <a href=
+         * "https://web.archive.org/web/20130509230548/http://help.yandex.com/webmaster/?id=1113851#1113856">Yandex'
+         * Using robots.txt (archived version)</a>
+         */
         HOST,
 
         // Google extension
         NO_INDEX,
 
-        // ACAP directives all start with ACAP-
+        /**
+         * <a href=
+         * "https://en.wikipedia.org/wiki/Automated_Content_Access_Protocol">Automated
+         * Content Access Protocol</a> directives all start with
+         * <code>ACAP-</code>.
+         */
         ACAP_(true, false),
 
-        // Extended standard
+        // Extensions to the standard
         REQUEST_RATE, VISIT_TIME, ROBOT_VERSION, COMMENT,
 
         // Line starts with http:, which we treat as sitemap directive.
@@ -315,13 +324,31 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
         DIRECTIVE_PREFIX.put("useragent", RobotDirective.USER_AGENT);
         DIRECTIVE_PREFIX.put("useg-agent", RobotDirective.USER_AGENT);
         DIRECTIVE_PREFIX.put("ser-agent", RobotDirective.USER_AGENT);
+        DIRECTIVE_PREFIX.put("users-agent", RobotDirective.USER_AGENT);
+        DIRECTIVE_PREFIX.put("user agent", RobotDirective.USER_AGENT);
+        DIRECTIVE_PREFIX.put("user-agnet", RobotDirective.USER_AGENT);
+        DIRECTIVE_PREFIX.put("user-agents", RobotDirective.USER_AGENT);
 
         DIRECTIVE_PREFIX.put("desallow", RobotDirective.DISALLOW);
+        DIRECTIVE_PREFIX.put("dissallow", RobotDirective.DISALLOW);
         DIRECTIVE_PREFIX.put("dissalow", RobotDirective.DISALLOW);
+        DIRECTIVE_PREFIX.put("disalow", RobotDirective.DISALLOW);
         DIRECTIVE_PREFIX.put("dssalow", RobotDirective.DISALLOW);
         DIRECTIVE_PREFIX.put("dsallow", RobotDirective.DISALLOW);
+        DIRECTIVE_PREFIX.put("diasllow", RobotDirective.DISALLOW);
+        DIRECTIVE_PREFIX.put("disallaw", RobotDirective.DISALLOW);
+        DIRECTIVE_PREFIX.put("diallow", RobotDirective.DISALLOW);
+        DIRECTIVE_PREFIX.put("disallows", RobotDirective.DISALLOW);
+        DIRECTIVE_PREFIX.put("disllow", RobotDirective.DISALLOW);
 
         DIRECTIVE_PREFIX.put("crawl delay", RobotDirective.CRAWL_DELAY);
+        DIRECTIVE_PREFIX.put("clawl-delay", RobotDirective.CRAWL_DELAY);
+        DIRECTIVE_PREFIX.put("craw-delay", RobotDirective.CRAWL_DELAY);
+        DIRECTIVE_PREFIX.put("crawl-deley", RobotDirective.CRAWL_DELAY);
+
+        DIRECTIVE_PREFIX.put("sitemaps", RobotDirective.SITEMAP);
+
+        DIRECTIVE_PREFIX.put("https", RobotDirective.HTTP);
     }
 
     // separator is either one or more spaces/tabs, or a colon
@@ -877,8 +904,20 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
         try {
             path = normalizePathDirective(path);
             if (path.length() == 0) {
-                // Disallow: <nothing> => allow all.
-                state.clearRules();
+                /*
+                 * "Disallow: <nothing>" meant "allow all" in the 1996 REP RFC
+                 * draft because there was no "Allow" directive.
+                 * 
+                 * RFC 9309 allows empty patterns in the formal syntax
+                 * (https://www.rfc-editor.org/rfc/rfc9309.html#section-2.2). No
+                 * specific handling of empty patterns is defined, as shortest
+                 * pattern they are matched with lowest priority.
+                 * 
+                 * Adding an extra rule with an empty pattern would have no
+                 * effect, because an "allow all" with lowest priority means
+                 * "allow everything else" which is the default anyway. So, we
+                 * ignore the empty disallow statement.
+                 */
             } else {
                 state.addRule(path, false);
             }
@@ -909,8 +948,11 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
         }
 
         if (path.length() == 0) {
-            // Allow: <nothing> => allow all.
-            state.clearRules();
+            /*
+             * Allow: <nothing> => allow all.
+             * 
+             * See handleDisallow(...): We ignore the empty allow statement.
+             */
         } else {
             state.addRule(path, true);
         }
@@ -1101,6 +1143,7 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
             System.err.println();
             System.err.println("Parse a robots.txt file");
             System.err.println("  <robots.txt>\tURL pointing to robots.txt file.");
+            System.err.println("              \tMax. five HTTP redirects are followed.");
             System.err.println("              \tTo read a local file use a file:// URL");
             System.err.println("              \t(parsed as http://example.com/robots.txt)");
             System.err.println("  <agentname> \tuser agent name to check for exclusion rules,");
@@ -1121,24 +1164,68 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
         }
 
         SimpleRobotRulesParser parser = new SimpleRobotRulesParser();
-        BaseRobotRules rules;
-        URLConnection connection = new URL(url).openConnection();
+        BaseRobotRules rules = null;
+        URL u = new URL(url);
+        URLConnection connection = u.openConnection();
+        if (!agentNames.isEmpty()) {
+            connection.setRequestProperty("User-Agent", agentName);
+        }
+
         try {
-            byte[] content = IOUtils.toByteArray(connection);
-            if (!url.matches("^https?://")) {
+            if (connection instanceof HttpURLConnection) {
+                HttpURLConnection httpConnection = (HttpURLConnection) connection;
+                int redirects = 0;
+                int maxRedirects = 5; // "five consecutive redirects" (RFC 9309)
+                while (redirects <= maxRedirects) {
+                    httpConnection.setInstanceFollowRedirects(false);
+                    int code = httpConnection.getResponseCode();
+                    switch (code) {
+                        case HttpURLConnection.HTTP_OK:
+                        System.out.println("Successfully fetched robots.txt");
+                        byte[] content = IOUtils.toByteArray(httpConnection);
+                        rules = parser.parseContent(url, content, httpConnection.getContentType(), agentNames);
+                            break;
+                        case HttpURLConnection.HTTP_MOVED_PERM:
+                        case HttpURLConnection.HTTP_MOVED_TEMP:
+                        case HttpURLConnection.HTTP_SEE_OTHER:
+                        redirects++;
+                        String location = httpConnection.getHeaderField("Location");
+                        if (location == null) {
+                            System.out.println("Redirect without Location header");
+                            rules = parser.failedFetch(code);
+                            break;
+                        }
+                        location = URLDecoder.decode(location, "UTF-8");
+                        u = new URL(u, location);
+                        if (redirects == maxRedirects) {
+                            System.out.println("Reached maximum of " + maxRedirects + " redirects, not following redirect to " + u.toString());
+                            rules = parser.failedFetch(code);
+                            break;
+                        }
+                        System.out.println("Following redirect to " + u.toString());
+                        httpConnection = (HttpURLConnection) u.openConnection();
+                        if (!agentNames.isEmpty()) {
+                            httpConnection.setRequestProperty("User-Agent", agentName);
+                        }
+                        continue; // continue redirecting
+                        default:
+                        System.out.println("Fetch of " + url + " failed with HTTP status code " + code);
+                        rules = parser.failedFetch(code);
+                            break;
+                    }
+                    break;
+                }
+            } else {
+                // not a HTTP URL, maybe file://
+                byte[] content = IOUtils.toByteArray(connection);
+                rules = parser.parseContent(url, content, "text/plain", agentNames);
                 // use artificial URL to avoid problems resolving relative
                 // sitemap paths for file:/ URLs
                 url = "http://example.com/robots.txt";
             }
-            rules = parser.parseContent(url, content, "text/plain", agentNames);
         } catch (IOException e) {
-            if (connection instanceof HttpURLConnection) {
-                int code = ((HttpURLConnection) connection).getResponseCode();
-                rules = parser.failedFetch(code);
-                System.out.println("Fetch of " + url + " failed with HTTP status code " + code);
-            } else {
-                throw e;
-            }
+            System.out.println("Fetch of " + url + " failed with: " + e.getMessage());
+            throw e;
         }
 
         if (args.length < 3) {


Test patch:
diff --git a/src/test/java/crawlercommons/robots/SimpleRobotRulesParserTest.java b/src/test/java/crawlercommons/robots/SimpleRobotRulesParserTest.java
index 6752d1c..18ef40e 100644
--- a/src/test/java/crawlercommons/robots/SimpleRobotRulesParserTest.java
+++ b/src/test/java/crawlercommons/robots/SimpleRobotRulesParserTest.java
@@ -77,7 +77,7 @@ public class SimpleRobotRulesParserTest {
 
     @Test
     void testEmptyRules() {
-        BaseRobotRules rules = createRobotRules("Any-darn-crawler", "");
+        BaseRobotRules rules = createRobotRules("anybot", "");
         assertTrue(rules.isAllowed("http://www.domain.com/anypage.html"));
     }
 
@@ -86,7 +86,7 @@ public class SimpleRobotRulesParserTest {
         final String simpleRobotsTxt = "User-agent: *" + CRLF //
                         + "Disallow: /index.cfm?fuseaction=sitesearch.results*";
 
-        BaseRobotRules rules = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rules = createRobotRules("anybot", simpleRobotsTxt);
         assertFalse(rules.isAllowed("http://searchservice.domain.com/index.cfm?fuseaction=sitesearch.results&type=People&qry=california&pg=2"));
     }
 
@@ -105,13 +105,13 @@ public class SimpleRobotRulesParserTest {
         // Test for /fish
         final String simpleRobotsTxt1 = "User-agent: *" + CRLF //
                         + "Disallow: /fish" + CRLF;
-        BaseRobotRules rule1 = createRobotRules("Any-darn-crawler", simpleRobotsTxt1);
+        BaseRobotRules rule1 = createRobotRules("anybot", simpleRobotsTxt1);
         assertEquals(isAllowed, rule1.isAllowed(urlStr));
 
         // Test for /fish*
         final String simpleRobotsTxt2 = "User-agent: *" + CRLF //
                         + "Disallow: /fish*" + CRLF;
-        BaseRobotRules rule2 = createRobotRules("Any-darn-crawler", simpleRobotsTxt2);
+        BaseRobotRules rule2 = createRobotRules("anybot", simpleRobotsTxt2);
         assertEquals(isAllowed, rule2.isAllowed(urlStr));
     }
 
@@ -126,7 +126,7 @@ public class SimpleRobotRulesParserTest {
         // Test for /fish
         final String simpleRobotsTxt = "User-agent: *" + CRLF //
                         + "Disallow: /fish/" + CRLF;
-        BaseRobotRules rule = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rule = createRobotRules("anybot", simpleRobotsTxt);
         assertEquals(isAllowed, rule.isAllowed(urlStr));
     }
 
@@ -142,7 +142,7 @@ public class SimpleRobotRulesParserTest {
         // Test for /*.php
         final String simpleRobotsTxt = "User-agent: *" + CRLF //
                         + "Disallow: /*.php" + CRLF;
-        BaseRobotRules rule = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rule = createRobotRules("anybot", simpleRobotsTxt);
         assertEquals(isAllowed, rule.isAllowed(urlStr));
     }
 
@@ -157,7 +157,7 @@ public class SimpleRobotRulesParserTest {
         // Test for /*.php$
         final String simpleRobotsTxt = "User-agent: *" + CRLF //
                         + "Disallow: /*.php$" + CRLF;
-        BaseRobotRules rule = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rule = createRobotRules("anybot", simpleRobotsTxt);
         assertEquals(isAllowed, rule.isAllowed(urlStr));
     }
 
@@ -169,7 +169,7 @@ public class SimpleRobotRulesParserTest {
         // Test for /fish*.php
         final String simpleRobotsTxt = "User-agent: *" + CRLF //
                         + "Disallow: /fish*.php" + CRLF;
-        BaseRobotRules rule = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rule = createRobotRules("anybot", simpleRobotsTxt);
         assertEquals(isAllowed, rule.isAllowed(urlStr));
     }
 
@@ -181,7 +181,7 @@ public class SimpleRobotRulesParserTest {
         // Test rule with multiple '*' characters
         final String simpleRobotsTxt = "User-agent: *" + CRLF //
                         + "Disallow: /*fish*.php" + CRLF;
-        BaseRobotRules rule = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rule = createRobotRules("anybot", simpleRobotsTxt);
         assertEquals(isAllowed, rule.isAllowed(urlStr));
     }
 
@@ -199,7 +199,7 @@ public class SimpleRobotRulesParserTest {
                         + "#disallow: /index.html" + LF //
                         + "#disallow: /test" + LF + LF;
 
-        BaseRobotRules rules = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rules = createRobotRules("anybot", simpleRobotsTxt);
         assertTrue(rules.isAllowed("http://www.domain.com/anypage.html"));
     }
 
@@ -232,7 +232,7 @@ public class SimpleRobotRulesParserTest {
         final String simpleRobotsTxt = "User-agent: *" + " # \u00A2 \u20B5" + CRLF //
                         + "Disallow:";
 
-        BaseRobotRules rules = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rules = createRobotRules("anybot", simpleRobotsTxt);
         assertTrue(rules.isAllowed("http://www.domain.com/anypage.html"));
     }
 
@@ -336,7 +336,7 @@ public class SimpleRobotRulesParserTest {
         final String simpleRobotsTxt = "User-agent: *" + CRLF //
                         + "Disallow:";
 
-        BaseRobotRules rules = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rules = createRobotRules("anybot", simpleRobotsTxt);
         assertTrue(rules.isAllowed("http://www.domain.com/anypage.html"));
     }
 
@@ -346,7 +346,7 @@ public class SimpleRobotRulesParserTest {
         final String simpleRobotsTxt = "User-agent: *" + CRLF //
                         + "Disallow: /";
 
-        BaseRobotRules rules = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rules = createRobotRules("anybot", simpleRobotsTxt);
         assertFalse(rules.isAllowed("http://www.example.com"));
         assertFalse(rules.isAllowed("http://www.example.com?q=a"));
     }
@@ -624,13 +624,24 @@ public class SimpleRobotRulesParserTest {
 
     @Test
     void testHtmlMarkupInRobotsTxt() {
-        final String htmlRobotsTxt = "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 3.2 Final//EN\"><HTML>\n" + "<HEAD>\n" + "<TITLE>/robots.txt</TITLE>\n" + "</HEAD>\n" + "<BODY>\n"
-                        + "User-agent: anybot<BR>\n" + "Disallow: <BR>\n" + "Crawl-Delay: 10<BR>\n" + "\n" + "User-agent: *<BR>\n" + "Disallow: /<BR>\n" + "Crawl-Delay: 30<BR>\n" + "\n" + "</BODY>\n"
+        final String htmlRobotsTxt = "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 3.2 Final//EN\"><HTML>\n" //
+                        + "<HEAD>\n" //
+                        + "<TITLE>/robots.txt</TITLE>\n" //
+                        + "</HEAD>\n" //
+                        + "<BODY>\n" //
+                        + "User-agent: mybot<BR>\n" //
+                        + "Disallow: <BR>\n" //
+                        + "Crawl-Delay: 10<BR>\n" //
+                        + "\n" + "User-agent: *<BR>\n" //
+                        + "Disallow: /<BR>\n" //
+                        + "Crawl-Delay: 30<BR>\n" //
+                        + "\n" //
+                        + "</BODY>\n" //
                         + "</HTML>\n";
 
         BaseRobotRules rules;
 
-        rules = createRobotRules("anybot", htmlRobotsTxt);
+        rules = createRobotRules("mybot", htmlRobotsTxt);
         assertTrue(rules.isAllowed("http://www.domain.com/index.html"));
         assertEquals(10000, rules.getCrawlDelay());
 
@@ -699,7 +710,7 @@ public class SimpleRobotRulesParserTest {
                         + "Allow: /somepage.html" + CRLF //
                         + "Disallow: /";
 
-        BaseRobotRules rules = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rules = createRobotRules("anybot", simpleRobotsTxt);
         assertTrue(rules.isAllowed("http://www.domain.com/AnyPage.html"));
         assertFalse(rules.isAllowed("http://www.domain.com/anypage.html"));
         assertTrue(rules.isAllowed("http://www.domain.com/somepage.html"));
@@ -711,7 +722,7 @@ public class SimpleRobotRulesParserTest {
         final String simpleRobotsTxt = "User-agent: *" + CRLF //
                         + "Disallow:";
 
-        BaseRobotRules rules = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rules = createRobotRules("anybot", simpleRobotsTxt);
         assertTrue(rules.isAllowed("http://www.domain.com/anypage.html"));
     }
 
@@ -720,10 +731,37 @@ public class SimpleRobotRulesParserTest {
         final String simpleRobotsTxt = "User-agent: *" + CRLF //
                         + "Allow:";
 
-        BaseRobotRules rules = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rules = createRobotRules("anybot", simpleRobotsTxt);
         assertTrue(rules.isAllowed("http://www.domain.com/anypage.html"));
     }
 
+    @Test
+    void testEmptyDisallowLowestPrecedence() {
+        String robotsTxt = "User-agent: *" + CRLF //
+                        + "Disallow: /disallowed/" + CRLF //
+                        + "Allow: /allowed/" + CRLF //
+                        + "Disallow: ";
+
+        BaseRobotRules rules = createRobotRules("anybot", robotsTxt);
+        assertTrue(rules.isAllowed("http://www.example.com/"));
+        assertTrue(rules.isAllowed("http://www.example.com/anypage.html"));
+        assertFalse(rules.isAllowed("http://www.example.com/disallowed/index.html"));
+        assertTrue(rules.isAllowed("http://www.example.com/allowed/index.html"));
+
+        // with merged groups
+        robotsTxt = "User-agent: *" + CRLF //
+                        + "Disallow: /disallowed/" + CRLF //
+                        + "Allow: /allowed/" + CRLF //
+                        + "User-agent: *" + CRLF //
+                        + "Disallow: ";
+
+        rules = createRobotRules("anybot", robotsTxt);
+        assertTrue(rules.isAllowed("http://www.example.com/"));
+        assertTrue(rules.isAllowed("http://www.example.com/anypage.html"));
+        assertFalse(rules.isAllowed("http://www.example.com/disallowed/index.html"));
+        assertTrue(rules.isAllowed("http://www.example.com/allowed/index.html"));
+    }
+
     @Test
     void testMultiWildcard() {
         // Make sure we only take the first wildcard entry.
@@ -734,7 +772,7 @@ public class SimpleRobotRulesParserTest {
                         + "User-agent: *" + CRLF //
                         + "Disallow: /";
 
-        BaseRobotRules rules = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rules = createRobotRules("anybot", simpleRobotsTxt);
         assertFalse(rules.isAllowed("http://www.domain.com/index.html"));
         assertTrue(rules.isAllowed("http://www.domain.com/anypage.html"));
     }
@@ -1069,7 +1107,7 @@ public class SimpleRobotRulesParserTest {
 
     @Test
     void testAmazonRobotsWithWildcards() throws Exception {
-        BaseRobotRules rules = createRobotRules("Any-darn-crawler", readFile("/robots/wildcards.txt"));
+        BaseRobotRules rules = createRobotRules("anybot", readFile("/robots/wildcards.txt"));
         assertFalse(rules.isAllowed("http://www.fict.com/wishlist/bogus"));
         assertTrue(rules.isAllowed("http://www.fict.com/wishlist/universal/page"));
         assertFalse(rules.isAllowed("http://www.fict.com/anydirectoryhere/gcrnsts"));
@@ -1081,7 +1119,7 @@ public class SimpleRobotRulesParserTest {
                         + "Disallow: /fish" + CRLF //
                         + "Allow: /fish" + CRLF;
 
-        BaseRobotRules rules = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        BaseRobotRules rules = createRobotRules("anybot", simpleRobotsTxt);
 
         assertTrue(rules.isAllowed("http://www.fict.com/fish"));
     }
@@ -1103,7 +1141,7 @@ public class SimpleRobotRulesParserTest {
         rules = createRobotRules("Three", simpleRobotsTxt, false);
         assertFalse(rules.isAllowed("http://www.fict.com/fish"));
 
-        rules = createRobotRules("Any-darn-crawler", simpleRobotsTxt);
+        rules = createRobotRules("anybot", simpleRobotsTxt);
         assertTrue(rules.isAllowed("http://www.fict.com/fish"));
     }