Repository: crawler-commons/crawler-commons
Pull request #351 (2021-12-14 20:14:17)

Improve robots check draft rfc compliance
This library suffers from multiple issues in processing robots.txt matches when compared to what's considered to be standard behavior. 

https://datatracker.ietf.org/doc/html/draft-rep-wg-topic-00 

- It doesn't merge multiple blocks for the same user agent. 
```
Crawlers MUST find the group that matches the product token exactly, 
   and then obey the rules of the group. If there is more than one 
   group matching the user-agent, the matching groups' rules MUST be 
   combined into one group. 
```
- It doesn't evaluate all matches to find the most specific. 
```
The most specific match found MUST be 
   used. The most specific match is the match that has the most octets. 
```
- When in conflict, it doesn't resolve conflicts correctly (because it doesn't eval them) 
```
If an allow and disallow rule is equivalent, the allow SHOULD be 
   used. 
```

This patch tries to address all of these issues.


Patch:
diff --git a/CHANGES.txt b/CHANGES.txt
index 50c59b0..2ef2135 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,10 +1,19 @@
 Crawler-Commons Change Log
 
-Current Development 1.3-SNAPSHOT (yyyy-mm-dd)
+Current Development 1.4-SNAPSHOT (yyyy-mm-dd)
+- Upgrade dependencies (dependabot) #379
+- Upgrade Maven plugins (dependabot) #377
+- Javadoc: ensure Javascript search is working (sebastian-nagel, Richard Zowalla, aecio) #378
+
+Release 1.3 (2022-07-19)
+- [Sitemaps] Disable support for DTDs in XML sitemaps and feeds by default (Kenneth Wong) #371
 - Migrate Continuous Integration from Travis to GitHub Actions (Valery Yatsynovich) #333
-- Upgrade dependencies (dependabot) #334, #339, #345, #346, #347
-- Upgrade Maven plugins (dependabot, Richard Zowalla, sebastian-nagel) #328, #329, #330, #331, #335, #336, #337, #338, #340, #341, #343
+- Upgrade dependencies (dependabot, Richard Zowalla) #334, #339, #345, #346, #347, #350, #354, #361, #369
+- Upgrade Maven plugins (dependabot, Richard Zowalla, sebastian-nagel) #328, #329, #330, #331, #335, #336, #337, #338, #340, #341, #343, #356, #363. #364, #366, #373, #374
+- Update pom.xml to address Maven warnings and deprecations (sebastian-nagel, Richard Zowalla, Avi Hayun) #342
 - Enable Dependabot (Valery Yatsynovich) #327
+- Removes test dependency towards mockito-core (Richard Zowalla) #367
+- Drops provided dependency towards servlet-api (Richard Zowalla) #368
 
 Release 1.2 (2021-10-06)
 - [Sitemaps] Avoid calling java.net.URL::equals in equals method of sitemaps and extensions (sebastian-nagel) #322
@@ -189,4 +198,4 @@ Release 0.1
 - parsing robots.txt
 - parsing sitemaps
 - URL analyzer which returns Top Level Domains
-- a simple HttpFetcher
\ No newline at end of file
+- a simple HttpFetcher
diff --git a/README.md b/README.md
index 9d0d9e6..43457e1 100644
--- a/README.md
+++ b/README.md
@@ -15,14 +15,15 @@ These components benefit from collaboration among various existing web crawler p
 # User Documentation
 
 ## Javadocs
-* [1.2](http://crawler-commons.github.io/crawler-commons/1.2/)
-* [1.1](http://crawler-commons.github.io/crawler-commons/1.1/)
-* [1.0](http://crawler-commons.github.io/crawler-commons/1.0/)
-* [0.10](http://crawler-commons.github.io/crawler-commons/0.10/)
-* [0.9](http://crawler-commons.github.io/crawler-commons/0.9/)
-* [0.8](http://crawler-commons.github.io/crawler-commons/0.8/)
-* [0.7](http://crawler-commons.github.io/crawler-commons/0.7/)
-* [0.6](http://crawler-commons.github.io/crawler-commons/0.6/apidocs/)
+* [1.3](https://crawler-commons.github.io/crawler-commons/1.3/)
+* [1.2](https://crawler-commons.github.io/crawler-commons/1.2/)
+* [1.1](https://crawler-commons.github.io/crawler-commons/1.1/)
+* [1.0](https://crawler-commons.github.io/crawler-commons/1.0/)
+* [0.10](https://crawler-commons.github.io/crawler-commons/0.10/)
+* [0.9](https://crawler-commons.github.io/crawler-commons/0.9/)
+* [0.8](https://crawler-commons.github.io/crawler-commons/0.8/)
+* [0.7](https://crawler-commons.github.io/crawler-commons/0.7/)
+* [0.6](https://crawler-commons.github.io/crawler-commons/0.6/apidocs/)
 
 # Mailing List
 
@@ -48,6 +49,12 @@ dependencies {
 
 # News
 
+## 28th July 2022  - crawler-commons 1.3 released
+
+We are glad to announce the 1.3 release of Crawler-Commons. See the [CHANGES.txt](https://github.com/crawler-commons/crawler-commons/blob/crawler-commons-1.3/CHANGES.txt) file included with the release for a complete list of details.
+The new release includes multiple dependency upgrades, improvements to the automatic builds, and a tighter protections against XXE vulnerability issues in the Sitemap parser.
+
+
 ## 14th October 2021  - crawler-commons 1.2 released
 
 We are glad to announce the 1.2 release of Crawler-Commons. See the [CHANGES.txt](https://github.com/crawler-commons/crawler-commons/blob/crawler-commons-1.2/CHANGES.txt) file included with the release for a complete list of details.
diff --git a/pom.xml b/pom.xml
index 06ecb91..b6b2b70 100644
--- a/pom.xml
+++ b/pom.xml
@@ -6,7 +6,7 @@
 	<artifactId>crawler-commons</artifactId>
 	<name>Crawler-commons</name>
 	<packaging>jar</packaging>
-	<version>1.3-SNAPSHOT</version>
+	<version>1.4-SNAPSHOT</version>
 
 	<licenses>
 		<license>
@@ -95,6 +95,12 @@
 			<name>Sebastian Nagel</name>
 			<email>snagel@apache.org</email>
 		</developer>
+
+		<developer>
+			<id>rzo1</id>
+			<name>Richard Zowalla</name>
+			<email>rzo1@apache.org</email>
+		</developer>
 	</developers>
 
 	<build>
@@ -198,7 +204,7 @@
 			<plugin>
 				<groupId>de.thetaphi</groupId>
 				<artifactId>forbiddenapis</artifactId>
-				<version>3.2</version>
+				<version>3.3</version>
 				<configuration>
 					<!-- if the used Java version is too new, don't fail, just do nothing: -->
 					<failOnUnsupportedJava>false</failOnUnsupportedJava>
@@ -241,6 +247,28 @@
 					</execution>
 				</executions>
 			</plugin>
+			<plugin>
+				<groupId>org.apache.maven.plugins</groupId>
+				<artifactId>maven-javadoc-plugin</artifactId>
+				<version>${maven-javadoc-plugin.version}</version>
+				<configuration>
+					<links>
+						<link>https://docs.oracle.com/javase/8/docs/api/</link>
+					</links>
+					<source>${javac.src.version}</source>
+					<additionalJOptions>
+						<additionalJOption>${additionalJavadocOpts}</additionalJOption>
+					</additionalJOptions>
+				</configuration>
+				<executions>
+					<execution>
+						<id>attach-javadocs</id>
+						<goals>
+							<goal>jar</goal>
+						</goals>
+					</execution>
+				</executions>
+			</plugin>
 		</plugins>
 	</build>
 
@@ -291,6 +319,7 @@
 								</goals>
 								<configuration>
 									<quiet>true</quiet>
+									<additionalJOption>${additionalJavadocOpts}</additionalJOption>
 									<archive>
 										<manifest>
 											<addDefaultImplementationEntries>true</addDefaultImplementationEntries>
@@ -329,33 +358,39 @@
 				</plugins>
 			</build>
 		</profile>
+		<profile>
+			<id>javadoc-no-module-directories</id>
+			<activation>
+				<jdk>[11,12)</jdk>
+			</activation>
+			<properties>
+				<additionalJavadocOpts>--no-module-directories</additionalJavadocOpts>
+			</properties>
+		</profile>
 	</profiles>
 
 	<properties>
 		<!-- Dependencies -->
 
-		<commons-io.version>2.7</commons-io.version>
-		<slf4j-api.version>1.7.32</slf4j-api.version>
-		<junit.version>5.8.1</junit.version>
-		<slf4j-log4j12.version>1.7.32</slf4j-log4j12.version>
-		<mockito-core.version>4.0.0</mockito-core.version>
-		<jetty.version>5.1.10</jetty.version>
-		<servlet-api.version>2.5</servlet-api.version>
+		<commons-io.version>2.11.0</commons-io.version>
+		<slf4j-api.version>1.7.36</slf4j-api.version>
+		<junit.version>5.9.0</junit.version>
+		<slf4j-log4j12.version>1.7.33</slf4j-log4j12.version>
 
 		<!-- Maven Plugin Dependencies -->
-		<maven-compiler-plugin.version>3.8.1</maven-compiler-plugin.version>
+		<maven-compiler-plugin.version>3.10.1</maven-compiler-plugin.version>
 		<maven-resources-plugin.version>2.5</maven-resources-plugin.version>
-		<maven-jar-plugin.version>3.2.0</maven-jar-plugin.version>
+		<maven-jar-plugin.version>3.2.2</maven-jar-plugin.version>
 		<maven-surfire-plugin.version>2.22.2</maven-surfire-plugin.version>
 		<maven-release-plugin.version>2.5.3</maven-release-plugin.version>
 		<maven-source-plugin.version>3.2.1</maven-source-plugin.version>
-		<maven-javadoc-plugin.version>3.3.1</maven-javadoc-plugin.version>
+		<maven-javadoc-plugin.version>3.4.0</maven-javadoc-plugin.version>
 		<maven-gpg-plugin.version>3.0.1</maven-gpg-plugin.version>
 		<apache-rat-plugin.version>0.8</apache-rat-plugin.version>
 		<maven-assembly-plugin.version>2.2.2</maven-assembly-plugin.version>
-		<maven-deploy-plugin.version>2.8.2</maven-deploy-plugin.version>
+		<maven-deploy-plugin.version>3.0.0</maven-deploy-plugin.version>
 		<checksum-maven-plugin.version>1.4</checksum-maven-plugin.version>
-		<maven.download.plugin.version>1.6.7</maven.download.plugin.version>
+		<maven.download.plugin.version>1.6.8</maven.download.plugin.version>
 
 		<!-- General Properties -->
 		<implementation.build>${scmBranch}@r${buildNumber}</implementation.build>
@@ -408,20 +443,5 @@
 			<scope>test</scope>
 		</dependency>
 
-		<dependency>
-			<groupId>org.mockito</groupId>
-			<artifactId>mockito-core</artifactId>
-			<version>${mockito-core.version}</version>
-			<scope>test</scope>
-		</dependency>
-
-		<!-- Provided dependencies -->
-		<dependency>
-			<groupId>javax.servlet</groupId>
-			<artifactId>servlet-api</artifactId>
-			<version>${servlet-api.version}</version>
-			<scope>provided</scope>
-		</dependency>
-
 	</dependencies>
 </project>
diff --git a/src/main/java/crawlercommons/robots/SimpleRobotRules.java b/src/main/java/crawlercommons/robots/SimpleRobotRules.java
index e15e428..c359d77 100644
--- a/src/main/java/crawlercommons/robots/SimpleRobotRules.java
+++ b/src/main/java/crawlercommons/robots/SimpleRobotRules.java
@@ -163,13 +163,40 @@ public class SimpleRobotRules extends BaseRobotRules {
                 return true;
             }
 
+            boolean isAllowed = true;
+            int longestRuleMatch = Integer.MIN_VALUE;
             for (RobotRule rule : _rules) {
-                if (ruleMatches(pathWithQuery, rule._prefix)) {
-                    return rule._allow;
+                int matchLength = ruleMatches(pathWithQuery, rule._prefix);
+                if (matchLength == -1) {
+                    // See precedence-of-rules test case for an example
+                    // Some webmasters expect behavior close to google's, and
+                    // this block is equivalent to:
+                    // https://github.com/google/robotstxt/blob/02bc6cdfa32db50d42563180c42aeb47042b4f0c/robots.cc#L605-L618
+                    // There are example robots.txt in the wild that benefit
+                    // from this.
+                    // As of 2/7/2022, https://venmo.com/robots.txt for
+                    // instance.
+                    if (rule._prefix.endsWith("index.htm") || rule._prefix.endsWith("index.html")) {
+                        matchLength = ruleMatches(pathWithQuery, rule._prefix.substring(0, rule._prefix.indexOf("index.htm")) + "$");
+                        if (matchLength == -1) {
+                            continue;
+                        }
+                    } else {
+                        continue;
+                    }
+                }
+
+                if (longestRuleMatch < matchLength) {
+                    longestRuleMatch = matchLength;
+                    isAllowed = rule.isAllow();
+                } else if (longestRuleMatch == matchLength) {
+                    isAllowed |= rule.isAllow();
                 }
+                // else we've already got a more specific rule, and this match
+                // doesn't matter
             }
 
-            return true;
+            return isAllowed;
         }
     }
 
@@ -197,7 +224,7 @@ public class SimpleRobotRules extends BaseRobotRules {
         }
     }
 
-    private boolean ruleMatches(String text, String pattern) {
+    private int ruleMatches(String text, String pattern) {
         int patternPos = 0;
         int textPos = 0;
 
@@ -223,7 +250,7 @@ public class SimpleRobotRules extends BaseRobotRules {
                 patternPos += 1;
                 if (patternPos >= patternEnd) {
                     // Pattern ends with '*', we're all good.
-                    return true;
+                    return pattern.length();
                 }
 
                 // TODO - don't worry about having two '*' in a row?
@@ -255,14 +282,14 @@ public class SimpleRobotRules extends BaseRobotRules {
 
                 // If we matched, we're all set, otherwise we failed
                 if (!matched) {
-                    return false;
+                    return -1;
                 }
             } else {
                 // See if the pattern from patternPos to wildcardPos matches the
                 // text starting at textPos
                 while ((patternPos < wildcardPos) && (textPos < textEnd)) {
                     if (text.charAt(textPos++) != pattern.charAt(patternPos++)) {
-                        return false;
+                        return -1;
                     }
                 }
             }
@@ -277,7 +304,11 @@ public class SimpleRobotRules extends BaseRobotRules {
         // We're at the end, so we have a match if the pattern was completely
         // consumed, and either we consumed all the text or we didn't have to
         // match it all (no '$' at end of the pattern)
-        return (patternPos == patternEnd) && ((textPos == textEnd) || !containsEndChar);
+        if ((patternPos == patternEnd) && ((textPos == textEnd) || !containsEndChar)) {
+            return pattern.length();
+        } else {
+            return -1;
+        }
     }
 
     /**
diff --git a/src/main/java/crawlercommons/robots/SimpleRobotRulesParser.java b/src/main/java/crawlercommons/robots/SimpleRobotRulesParser.java
index 727efc1..207550d 100644
--- a/src/main/java/crawlercommons/robots/SimpleRobotRulesParser.java
+++ b/src/main/java/crawlercommons/robots/SimpleRobotRulesParser.java
@@ -167,11 +167,6 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
          */
         private boolean _finishedAgentFields;
 
-        // True if we're done adding rules for a matched (not wildcard) agent
-        // name. When this is true, we only consider sitemap directives, so we
-        // skip all remaining user agent blocks.
-        private boolean _skipAgents;
-
         /*
          * Counter of warnings reporting invalid rules/lines in the robots.txt
          * file. The counter is used to limit the number of logged warnings.
@@ -225,14 +220,6 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
             _finishedAgentFields = finishedAgentFields;
         }
 
-        public boolean isSkipAgents() {
-            return _skipAgents;
-        }
-
-        public void setSkipAgents(boolean skipAgents) {
-            _skipAgents = skipAgents;
-        }
-
         public void clearRules() {
             _curRules.clearRules();
         }
@@ -513,22 +500,27 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
                     break;
 
                 case DISALLOW:
+                parseState.setFinishedAgentFields(true);
                 handleDisallow(parseState, token);
                     break;
 
                 case ALLOW:
+                parseState.setFinishedAgentFields(true);
                 handleAllow(parseState, token);
                     break;
 
                 case CRAWL_DELAY:
+                parseState.setFinishedAgentFields(true);
                 handleCrawlDelay(parseState, token);
                     break;
 
                 case SITEMAP:
+                parseState.setFinishedAgentFields(true);
                 handleSitemap(parseState, token);
                     break;
 
                 case HTTP:
+                parseState.setFinishedAgentFields(true);
                 handleHttp(parseState, token);
                     break;
 
@@ -593,21 +585,16 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
      *            data for directive
      */
     private void handleUserAgent(ParseState state, RobotToken token) {
-        if (state.isMatchedRealName()) {
-            if (state.isFinishedAgentFields()) {
-                state.setSkipAgents(true);
-            }
-
-            return;
-        }
-
-        if (state.isFinishedAgentFields()) {
-            // We've got a user agent field, so we haven't yet seen anything
-            // that tells us we're done with this set of agent names.
-            state.setFinishedAgentFields(false);
+        // If we are adding rules, and had already finished reading user agent
+        // fields, then we are in a new section, hence stop adding rules
+        if (state.isAddingRules() && state.isFinishedAgentFields()) {
             state.setAddingRules(false);
         }
 
+        // Clearly we've encountered a new user agent directive, hence we need to start
+        // processing until we are finished.
+        state.setFinishedAgentFields(false);
+
         // Handle the case when there are multiple target names are passed
         // We assume we should do case-insensitive comparison of target name.
         String[] targetNames = state.getTargetName().toLowerCase(Locale.ROOT).split(",");
@@ -621,21 +608,24 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
             // TODO KKr - catch case of multiple names, log as non-standard.
             String[] agentNames = token.getData().split("[ \t,]");
             for (String agentName : agentNames) {
-                // TODO should we do case-insensitive matching? Probably yes.
                 agentName = agentName.trim().toLowerCase(Locale.ROOT);
                 if (agentName.isEmpty()) {
                     // Ignore empty names
-                } else if (agentName.equals("*") && !state.isMatchedWildcard()) {
+                } else if (agentName.equals("*") && !state.isMatchedRealName()) {
                     state.setMatchedWildcard(true);
                     state.setAddingRules(true);
                 } else {
-                    // TODO use longest match as winner
                     for (String targetName : targetNameSplits) {
                         if (targetName.startsWith(agentName)) {
+                            // In case we previously hit a wildcard rule match
+                            // but we could also have hit our own user agent before.
+                            // only clear if wildcard was matched earlier
+                            if (state.isMatchedWildcard()) {
+                                state.clearRules();
+                            }
                             state.setMatchedRealName(true);
                             state.setAddingRules(true);
-                            // In case we previously hit a wildcard rule match
-                            state.clearRules();
+                            state.setMatchedWildcard(false);
                             break;
                         }
                     }
@@ -644,6 +634,17 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
         }
     }
 
+    /**
+     * Add any uniform rules to clean up path directives
+     * @param path
+     * @return clean path
+     */
+    private String normalizePathDirective(String path) {
+        path = path.trim();
+
+        return path;
+    }
+
     /**
      * Handle the disallow: directive
      * 
@@ -653,12 +654,6 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
      *            data for directive
      */
     private void handleDisallow(ParseState state, RobotToken token) {
-        if (state.isSkipAgents()) {
-            return;
-        }
-
-        state.setFinishedAgentFields(true);
-
         if (!state.isAddingRules()) {
             return;
         }
@@ -667,7 +662,7 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
 
         try {
             path = URLDecoder.decode(path, "UTF-8");
-
+            path = normalizePathDirective(path);
             if (path.length() == 0) {
                 // Disallow: <nothing> => allow all.
                 state.clearRules();
@@ -688,12 +683,6 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
      *            data for directive
      */
     private void handleAllow(ParseState state, RobotToken token) {
-        if (state.isSkipAgents()) {
-            return;
-        }
-
-        state.setFinishedAgentFields(true);
-
         if (!state.isAddingRules()) {
             return;
         }
@@ -702,6 +691,7 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
 
         try {
             path = URLDecoder.decode(path, "UTF-8");
+            path = normalizePathDirective(path);
         } catch (Exception e) {
             reportWarning(state, "Error parsing robots rules - can't decode path: {}", path);
         }
@@ -723,12 +713,6 @@ public class SimpleRobotRulesParser extends BaseRobotsParser {
      *            data for directive
      */
     private void handleCrawlDelay(ParseState state, RobotToken token) {
-        if (state.isSkipAgents()) {
-            return;
-        }
-
-        state.setFinishedAgentFields(true);
-
         if (!state.isAddingRules()) {
             return;
         }
diff --git a/src/main/java/crawlercommons/sitemaps/SiteMapParser.java b/src/main/java/crawlercommons/sitemaps/SiteMapParser.java
index b89b66e..da2e7e8 100644
--- a/src/main/java/crawlercommons/sitemaps/SiteMapParser.java
+++ b/src/main/java/crawlercommons/sitemaps/SiteMapParser.java
@@ -102,6 +102,11 @@ public class SiteMapParser {
 
     private MimeTypeDetector mimeTypeDetector;
 
+    /**
+     * Option to allow DTDs in sitemaps.
+     */
+    private boolean allowDocTypeDefinitions = false;
+
     /* Function to normalize or filter URLs. Does nothing by default. */
     private Function<String, String> urlFilter = (String url) -> url;
 
@@ -140,6 +145,16 @@ public class SiteMapParser {
         this.mimeTypeDetector = new MimeTypeDetector();
     }
 
+    /**
+     * Sets if the parser allows a DTD in sitemaps or feeds.
+     *
+     * @param allowDocTypeDefinitions
+     *            true if allowed. Default is false.
+     */
+    public void setAllowDocTypeDefinitions(boolean allowDocTypeDefinitions) {
+        this.allowDocTypeDefinitions = allowDocTypeDefinitions;
+    }
+
     /**
      * @return whether invalid URLs will be rejected (where invalid means that
      *         the URL is not under the base URL, see <a href=
@@ -164,7 +179,7 @@ public class SiteMapParser {
      * specification, or any accepted namespace (see
      * {@link #addAcceptedNamespace(String)}). Note enabling strict namespace
      * checking always adds the namespace defined by the current sitemap
-     * specificiation ({@link Namespace#SITEMAP}) to the list of accepted
+     * specification ({@link Namespace#SITEMAP}) to the list of accepted
      * namespaces.
      * 
      * @param s
@@ -242,6 +257,7 @@ public class SiteMapParser {
     public void setURLFilter(URLFilter filter) {
         urlFilter = filter::filter;
     }
+
     /**
      * Returns a SiteMap or SiteMapIndex given an online sitemap URL
      *
@@ -595,6 +611,9 @@ public class SiteMapParser {
             factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
             factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
             factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+            if (!this.allowDocTypeDefinitions) {
+                factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+            }
         } catch (Exception e) {
             throw new RuntimeException("Failed to configure XML parser: " + e.toString());
         }


Test patch:
diff --git a/src/main/java/crawlercommons/sitemaps/SiteMapTester.java b/src/main/java/crawlercommons/sitemaps/SiteMapTester.java
index fa475da..c73720e 100644
--- a/src/main/java/crawlercommons/sitemaps/SiteMapTester.java
+++ b/src/main/java/crawlercommons/sitemaps/SiteMapTester.java
@@ -46,6 +46,8 @@ public class SiteMapTester {
             LOG.error("Java properties:");
             LOG.error("  sitemap.strictNamespace");
             LOG.error("                  if true sitemaps are required to use the standard namespace URI");
+            LOG.error("  sitemap.allow.dtd");
+            LOG.error("                  if true sitemaps are allowed to include a DTD");
             LOG.error("  sitemap.extensions");
             LOG.error("                  if true enable sitemap extension parsing");
             LOG.error("  sitemap.filter.urls");
@@ -71,6 +73,9 @@ public class SiteMapTester {
         boolean strictNamespace = Boolean.getBoolean("sitemap.strictNamespace");
         saxParser.setStrictNamespace(strictNamespace);
 
+        boolean allowDTD = Boolean.getBoolean("sitemap.allow.dtd");
+        saxParser.setAllowDocTypeDefinitions(allowDTD);
+
         boolean enableExtensions = Boolean.getBoolean("sitemap.extensions");
         if (enableExtensions) {
             saxParser.enableExtensions();
diff --git a/src/test/java/crawlercommons/robots/SimpleRobotRulesParserTest.java b/src/test/java/crawlercommons/robots/SimpleRobotRulesParserTest.java
index 22afa54..53732da 100644
--- a/src/test/java/crawlercommons/robots/SimpleRobotRulesParserTest.java
+++ b/src/test/java/crawlercommons/robots/SimpleRobotRulesParserTest.java
@@ -20,8 +20,8 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
 
-import javax.servlet.http.HttpServletResponse;
 import java.io.InputStream;
+import java.net.HttpURLConnection;
 import java.util.Arrays;
 import java.util.Locale;
 
@@ -287,27 +287,27 @@ public class SimpleRobotRulesParserTest {
     }
 
     @ParameterizedTest
-    @CsvSource({ "False, http://www.domain.com/a",//
-                    "False, http://www.domain.com/a/",//
-                    "False, http://www.domain.com/a/bloh/foo.html",//
-                    "True, http://www.domain.com/b",//
-                    "False, http://www.domain.com/b/a",//
-                    "False, http://www.domain.com/b/a/index.html",//
-                    "True, http://www.domain.com/b/b/foo.html",//
-                    "True, http://www.domain.com/c",//
-                    "True, http://www.domain.com/c/a",//
-                    "True, http://www.domain.com/c/a/index.html",//
-                    "True, http://www.domain.com/c/b/foo.html",//
-                    "True, http://www.domain.com/d",//
-                    "True, http://www.domain.com/d/a",//
-                    "True, http://www.domain.com/e/a/index.html",//
-                    "True, http://www.domain.com/e/d",//
-                    "True, http://www.domain.com/e/d/foo.html",//
-                    "True, http://www.domain.com/e/doh.html",//
-                    "True, http://www.domain.com/f/index.html",//
-                    "True, http://www.domain.com/foo/bar/baz.html",//
-                    "True, http://www.domain.com/f/" })
-    void testNutchCases(boolean isAllowed, String urlStr) {
+    @CsvSource({ "False, False, http://www.domain.com/a",//
+                    "False, False, http://www.domain.com/a/",//
+                    "False, False, http://www.domain.com/a/bloh/foo.html",//
+                    "True, True, http://www.domain.com/b",//
+                    "False, False, http://www.domain.com/b/a",//
+                    "False, False, http://www.domain.com/b/a/index.html",//
+                    "True, True, http://www.domain.com/b/b/foo.html",//
+                    "True, True, http://www.domain.com/c",//
+                    "True, True, http://www.domain.com/c/a",//
+                    "True, True, http://www.domain.com/c/a/index.html",//
+                    "True, True, http://www.domain.com/c/b/foo.html",//
+                    "True, False, http://www.domain.com/d",//
+                    "True, False, http://www.domain.com/d/a",//
+                    "True, True, http://www.domain.com/e/a/index.html",//
+                    "True, True, http://www.domain.com/e/d",//
+                    "True, False, http://www.domain.com/e/d/foo.html",//
+                    "True, True, http://www.domain.com/e/doh.html",//
+                    "True, True, http://www.domain.com/f/index.html",//
+                    "True, False, http://www.domain.com/foo/bar/baz.html",//
+                    "True, True, http://www.domain.com/f/" })
+    void testNutchCases(boolean isAllowed, boolean isMergeAllowed, String urlStr) {
         // Run through the Nutch test cases.
         final String nutchRobotsTxt = "User-Agent: Agent1 #foo" + CR + "Disallow: /a" + CR + "Disallow: /b/a" + CR + "#Disallow: /c" + CR + "" + CR + "" + CR + "User-Agent: Agent2 Agent3#foo" + CR
                         + "User-Agent: Agent4" + CR + "Disallow: /d" + CR + "Disallow: /e/d/" + CR + "" + CR + "User-Agent: *" + CR + "Disallow: /foo/bar/" + CR;
@@ -317,11 +317,10 @@ public class SimpleRobotRulesParserTest {
         rules = createRobotRules("Agent1", nutchRobotsTxt);
         assertEquals(isAllowed, rules.isAllowed(urlStr));
 
-        // Note that the SimpleRobotRulesParser only parses the rule set of the
-        // first matching agent name. For the following example, the parser
-        // returns only the rules matching 'Agent1'.
+        // Note that SimpleRobotRulesParser now merges all matching user agent
+        // rules.
         rules = createRobotRules("Agent5,Agent2,Agent1,Agent3,*", nutchRobotsTxt);
-        assertEquals(isAllowed, rules.isAllowed(urlStr));
+        assertEquals(isMergeAllowed, rules.isAllowed(urlStr));
     }
 
     @ParameterizedTest
@@ -537,11 +536,11 @@ public class SimpleRobotRulesParserTest {
         BaseRobotRules rules;
 
         SimpleRobotRulesParser robotParser = new SimpleRobotRulesParser();
-        rules = robotParser.failedFetch(HttpServletResponse.SC_SERVICE_UNAVAILABLE);
+        rules = robotParser.failedFetch(HttpURLConnection.HTTP_UNAVAILABLE);
         assertTrue(rules.isDeferVisits());
         assertFalse(rules.isAllowed("http://www.domain.com/index.html"));
 
-        rules = robotParser.failedFetch(HttpServletResponse.SC_MOVED_PERMANENTLY);
+        rules = robotParser.failedFetch(HttpURLConnection.HTTP_MOVED_PERM);
         assertTrue(rules.isDeferVisits());
         assertFalse(rules.isAllowed("http://www.domain.com/index.html"));
 
@@ -558,7 +557,7 @@ public class SimpleRobotRulesParserTest {
         // Calling failedFetch with a good status code should trigger an
         // exception.
         try {
-            robotParser.failedFetch(HttpServletResponse.SC_OK);
+            robotParser.failedFetch(HttpURLConnection.HTTP_OK);
             fail("Should have thrown an exception");
         } catch (Exception e) {
             // valid
@@ -713,6 +712,31 @@ public class SimpleRobotRulesParserTest {
         assertFalse(rules.isAllowed("http://domain.com/bot-trap/"), "many-user-agents");
     }
 
+    @Test
+    void testManyUserAgentsMergeRules() throws Exception {
+        BaseRobotRules rules = createRobotRules("wget", readFile("/robots/merge-rules.txt"));
+        assertTrue(rules.isAllowed("http://domain.com/foo/bar/bar.txt"), "merge-rules");
+        assertFalse(rules.isAllowed("http://domain.com/tmp/bar/bar.txt"), "merge-rules");
+        assertTrue(rules.isAllowed("http://domain.com/chk/bar/bar.txt"), "merge-rules");
+    }
+
+    @Test
+    void testPrecedenceOfRules() throws Exception {
+        BaseRobotRules rules = createRobotRules("wget", readFile("/robots/precedence-of-rules.txt"));
+
+        assertTrue(rules.isAllowed("http://domain.com/"), "precedence-of-rules");
+        assertTrue(rules.isAllowed("http://domain.com/foo/bar"), "precedence-of-rules");
+        assertFalse(rules.isAllowed("http://domain.com/bar/foo"), "precedence-of-rules");
+
+        rules = createRobotRules("testy", readFile("/robots/precedence-of-rules.txt"));
+
+        assertTrue(rules.isAllowed("http://domain.com/"), "precedence-of-rules");
+        assertTrue(rules.isAllowed("http://domain.com/foo/bar"), "precedence-of-rules");
+        assertFalse(rules.isAllowed("http://domain.com/foo/blue"), "precedence-of-rules");
+        assertFalse(rules.isAllowed("http://domain.com/foo/"), "precedence-of-rules");
+        assertFalse(rules.isAllowed("http://domain.com/foo/tar"), "precedence-of-rules");
+    }
+
     @Test
     void testMalformedPathInRobotsFile() throws Exception {
         BaseRobotRules rules = createRobotRules("bot1", readFile("/robots/malformed-path.txt"));
diff --git a/src/test/java/crawlercommons/sitemaps/SiteMapParserTest.java b/src/test/java/crawlercommons/sitemaps/SiteMapParserTest.java
index 3856e17..64ceaaa 100644
--- a/src/test/java/crawlercommons/sitemaps/SiteMapParserTest.java
+++ b/src/test/java/crawlercommons/sitemaps/SiteMapParserTest.java
@@ -113,7 +113,7 @@ public class SiteMapParserTest {
     }
 
     @Test
-    public void testSitemapXXE() throws UnknownFormatException, IOException {
+    public void testSitemapXXE() throws IOException {
         // A file on disk that would be read if we were vulnerable to XXE
         File doNotVisit = new File("src/test/resources/sitemaps/do-not-visit.txt");
 
@@ -137,6 +137,37 @@ public class SiteMapParserTest {
                         .append("</urlset>");
         byte[] content = scontent.toString().getBytes(UTF_8);
 
+        URL url = new URL("http://www.example.com/sitemap.xxe.xml");
+        Assertions.assertThrows(UnknownFormatException.class,
+            () -> parser.parseSiteMap(contentType, content, url));
+    }
+
+    @Test
+    public void testSitemapXXEWithDocTypeAllowed() throws UnknownFormatException, IOException {
+        // A file on disk that would be read if we were vulnerable to XXE
+        File doNotVisit = new File("src/test/resources/sitemaps/do-not-visit.txt");
+
+        // Create a sitemap with an external entity referring to the local file
+        SiteMapParser parser = new SiteMapParser();
+        parser.setAllowDocTypeDefinitions(true);
+        String contentType = "text/xml";
+        StringBuilder scontent = new StringBuilder(1024);
+        scontent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n") //
+                        .append("<!DOCTYPE urlset [\n") //
+                        .append("  <!ENTITY test SYSTEM \"file://" + doNotVisit.getAbsolutePath() + "\">\n") //
+                        .append("]>\n") //
+                        .append("<urlset xmlns=\"http://www.sitemaps.org/schemas/sitemap/0.9\">\n") //
+                        .append("  <url>\n") //
+                        .append("    <loc>http://www.example.com/visit-here</loc>\n") //
+                        .append("    <lastmod>2019-06-19</lastmod>\n") //
+                        .append("   </url>\n") //
+                        .append("  <url>\n") //
+                        .append("    <loc>&test;</loc>\n") //
+                        .append("    <lastmod>2019-06-19</lastmod>\n") //
+                        .append("  </url>\n") //
+                        .append("</urlset>");
+        byte[] content = scontent.toString().getBytes(UTF_8);
+
         URL url = new URL("http://www.example.com/sitemap.xxe.xml");
         AbstractSiteMap asm = parser.parseSiteMap(contentType, content, url);
         assertEquals(SitemapType.XML, asm.getType());
diff --git a/src/test/resources/robots/merge-rules.txt b/src/test/resources/robots/merge-rules.txt
new file mode 100644
index 0000000..3751796
--- /dev/null
+++ b/src/test/resources/robots/merge-rules.txt
@@ -0,0 +1,16 @@
+User-Agent: testy
+Disallow: /
+
+User-Agent: curl
+Disallow: /
+
+User-Agent: wget
+Allow: /foo/bar
+
+User-Agent: *
+User-Agent: wget
+Disallow: /foo/bar
+Disallow: /tmp/bar
+Allow: /ckh/bar
+
+
diff --git a/src/test/resources/robots/precedence-of-rules.txt b/src/test/resources/robots/precedence-of-rules.txt
new file mode 100644
index 0000000..866c620
--- /dev/null
+++ b/src/test/resources/robots/precedence-of-rules.txt
@@ -0,0 +1,15 @@
+User-Agent: testy
+Disallow: /foo
+
+User-Agent: wget
+Allow: /
+Allow: /foo
+Allow: /index.html
+Disallow: /*
+
+User-Agent: testy
+Allow: /foo/bar
+Disallow: /foo/b*
+
+
+