(tomcat) branch 10.1.x updated: Resolve missing back references or variables as empty string

[email protected]
Newsgroups gmane.comp.jakarta.tomcat.devel
Message-ID <[email protected]>
This is an automated email from the ASF dual-hosted git repository.

rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/10.1.x by this push:
     new b24cb50f94 Resolve missing back references or variables as empty string
b24cb50f94 is described below

commit b24cb50f94455fbf7da88dcb8b801cde695fc8e3
Author: remm <[email protected]>
AuthorDate: Thu Aug 27 08:46:28 2026 +0200

    Resolve missing back references or variables as empty string
    
    This seems to be the behavior of mod_rewrite.
    Tests coauthored with OpenCode.
---
 .../catalina/valves/rewrite/Substitution.java      |  13 ++-
 .../catalina/valves/rewrite/TestRewriteValve.java  | 104 +++++++++++++++++++++
 webapps/docs/changelog.xml                         |   4 +
 3 files changed, 118 insertions(+), 3 deletions(-)

diff --git a/java/org/apache/catalina/valves/rewrite/Substitution.java b/java/org/apache/catalina/valves/rewrite/Substitution.java
index 664df31c6f..dbd14634b7 100644
--- a/java/org/apache/catalina/valves/rewrite/Substitution.java
+++ b/java/org/apache/catalina/valves/rewrite/Substitution.java
@@ -101,7 +101,7 @@ public class Substitution {
 
         @Override
         public String evaluate(Matcher rule, Matcher cond, Resolver resolver) {
-            String result = rule.group(n);
+            String result = (n <= rule.groupCount()) ? rule.group(n) : null;
             if (result == null) {
                 result = "";
             }
@@ -134,7 +134,8 @@ public class Substitution {
 
         @Override
         public String evaluate(Matcher rule, Matcher cond, Resolver resolver) {
-            return (cond.group(n) == null ? "" : cond.group(n));
+            String result = (cond != null && n <= cond.groupCount()) ? cond.group(n) : null;
+            return (result == null) ? "" : result;
         }
     }
 
@@ -497,7 +498,13 @@ public class Substitution {
     private String evaluateSubstitution(SubstitutionElement[] elements, Matcher rule, Matcher cond, Resolver resolver) {
         StringBuilder buf = new StringBuilder();
         for (SubstitutionElement element : elements) {
-            buf.append(element.evaluate(rule, cond, resolver));
+            // Elements may return null, for example when a server variable,
+            // environment variable, SSL variable or map lookup is undefined.
+            // Like mod_rewrite, undefined values expand to an empty string.
+            String value = element.evaluate(rule, cond, resolver);
+            if (value != null) {
+                buf.append(value);
+            }
         }
         return buf.toString();
     }
diff --git a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
index e5ac7dd1c9..3634872943 100644
--- a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
+++ b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
@@ -20,11 +20,14 @@ import java.io.File;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.net.HttpURLConnection;
+import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import jakarta.servlet.ServletException;
 import jakarta.servlet.http.Cookie;
@@ -832,6 +835,107 @@ public class TestRewriteValve extends TomcatBaseTest {
     }
 
 
+    @Test
+    public void testSubstitutionUnknownServerVariable() throws Exception {
+        // Unknown server variables must expand to an empty string
+        doTestRewrite("RewriteRule ^/b$ /c/undef_%{UNKNOWN_VARIABLE}", "/b", "/c/undef_");
+    }
+
+
+    @Test
+    public void testSubstitutionFailedMapLookup() throws Exception {
+        // A failed map lookup without a default value must expand to an empty string
+        doTestRewrite("RewriteMap mapa org.apache.catalina.valves.rewrite.TesterRewriteMapA\n" +
+                "RewriteRule ^/b$ /c/map_${mapa:missing}", "/b", "/c/map_");
+    }
+
+
+    @Test
+    public void testRuleBackReferenceMissingGroup() throws Exception {
+        // $1 does not exist in the pattern so it must expand to an empty string
+        doTestRewrite("RewriteRule ^/b$ /c/backref_$1", "/b", "/c/backref_");
+    }
+
+
+    @Test
+    public void testCondBackReferenceNoMatchedCond() throws Exception {
+        // There are no RewriteCond directives so %1 must expand to an empty string
+        doTestRewrite("RewriteRule ^/b$ /c/backref_%1", "/b", "/c/backref_");
+    }
+
+
+    @Test
+    public void testCondBackReferenceLexicalCondOnly() throws Exception {
+        // The condition is a lexical comparison and has no capture groups so %1 must
+        // expand to an empty string
+        doTestRewrite("RewriteCond %{REQUEST_URI} =/b\n" +
+                "RewriteRule ^/b$ /c/backref_%1", "/b", "/c/backref_");
+    }
+
+
+    @Test
+    public void testCondMatcherReflectsCurrentRequest() throws Exception {
+        // The matcher returned for a condition must reflect the result of the most
+        // recent evaluation, including that it is cleared when the pattern does not
+        // match.
+        RewriteCond condition = new RewriteCond();
+        condition.setTestString("%{QUERY_STRING}");
+        condition.setCondPattern("!^a=([0-9]+)$");
+        condition.parse(new HashMap<String, RewriteMap>());
+
+        Matcher rule = Pattern.compile(".*").matcher("/b");
+
+        // The pattern matches so the negated condition is not satisfied
+        Assert.assertFalse(condition.evaluate(rule, null, new TestResolver("a=1")));
+        Matcher m = condition.getMatcher();
+        Assert.assertNotNull(m);
+        Assert.assertEquals("1", m.group(1));
+
+        // The pattern does not match so the negated condition is satisfied and the
+        // matcher must be cleared
+        Assert.assertTrue(condition.evaluate(rule, null, new TestResolver("b=2")));
+        Assert.assertNull(condition.getMatcher());
+    }
+
+
+    private static class TestResolver extends Resolver {
+
+        private final String queryString;
+
+        private TestResolver(String queryString) {
+            this.queryString = queryString;
+        }
+
+        @Override
+        public String resolve(String key) {
+            if (key.equals("QUERY_STRING")) {
+                return queryString;
+            }
+            return "";
+        }
+
+        @Override
+        public String resolveSsl(String key) {
+            return null;
+        }
+
+        @Override
+        public String resolveHttp(String key) {
+            return "";
+        }
+
+        @Override
+        public boolean resolveResource(int type, String name) {
+            return false;
+        }
+
+        @Override
+        public Charset getUriCharset() {
+            return StandardCharsets.UTF_8;
+        }
+    }
+
+
     @Test
     public void testNegativePattern01() throws Exception {
         doTestRewrite("RewriteRule !^/b/.* /c/", "/b", "/c/");
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 898bd14b86..f89a089adf 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -146,6 +146,10 @@
       <fix>
         Ensure namespace attributes are XML escaped in WebDAV responses. (markt)
       </fix>
+      <fix>
+        Resolve null or missing rewrite substitutions as an empty string, to
+        align with the mod_rewrite behavior. (remm)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Cluster">
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.