This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 5156d2194a Fix SSI issues
5156d2194a is described below
commit 5156d2194aacde4b6c90bed45a00d7c04bbaf9bc
Author: remm <[email protected]>
AuthorDate: Sun Jun 28 15:56:33 2026 +0200
Fix SSI issues
Quoting with SSI exec.
Escaping.
Recursive substitution.
Avoid tokenizer to handle = inside quotes for parameters.
Co authored with OpenCode, based on the code review.
---
java/org/apache/catalina/ssi/SSIExec.java | 46 +++++++++---
.../apache/catalina/ssi/SSIExternalResolver.java | 2 +-
java/org/apache/catalina/ssi/SSIFilter.java | 7 +-
java/org/apache/catalina/ssi/SSIMediator.java | 67 ++++++++++++++---
java/org/apache/catalina/ssi/SSIProcessor.java | 86 +++++++++++++---------
java/org/apache/catalina/ssi/SSIServlet.java | 4 +-
.../catalina/ssi/TestExpressionParseTree.java | 75 ++++++++++++++++++-
webapps/docs/changelog.xml | 3 +
8 files changed, 225 insertions(+), 65 deletions(-)
diff --git a/java/org/apache/catalina/ssi/SSIExec.java
b/java/org/apache/catalina/ssi/SSIExec.java
index e0e3a38410..feec1c7fe7 100644
--- a/java/org/apache/catalina/ssi/SSIExec.java
+++ b/java/org/apache/catalina/ssi/SSIExec.java
@@ -20,7 +20,7 @@ import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintWriter;
-import java.util.StringTokenizer;
+import java.util.ArrayList;
import org.apache.catalina.util.IOTools;
import org.apache.tomcat.util.res.StringManager;
@@ -67,21 +67,45 @@ public class SSIExec implements SSICommand {
boolean foundProgram = false;
try {
Runtime rt = Runtime.getRuntime();
- StringTokenizer st = new StringTokenizer(substitutedValue);
- String[] cmdArray = new String[st.countTokens()];
- for (int i = 0; i < cmdArray.length; i++) {
- cmdArray[i] = st.nextToken();
+ ArrayList<String> tokens = new ArrayList<>();
+ StringBuilder current = new StringBuilder();
+ boolean inDoubleQuote = false;
+ boolean inSingleQuote = false;
+ for (int j = 0; j < substitutedValue.length(); j++) {
+ char c = substitutedValue.charAt(j);
+ if (c == '"' && !inSingleQuote) {
+ inDoubleQuote = !inDoubleQuote;
+ } else if (c == '\'' && !inDoubleQuote) {
+ inSingleQuote = !inSingleQuote;
+ } else if (Character.isWhitespace(c) && !inDoubleQuote &&
!inSingleQuote) {
+ if (current.length() > 0) {
+ tokens.add(current.toString());
+ current.setLength(0);
+ }
+ } else {
+ current.append(c);
+ }
}
+ if (current.length() > 0) {
+ tokens.add(current.toString());
+ }
+ String[] cmdArray = tokens.toArray(new String[0]);
Process proc = rt.exec(cmdArray);
foundProgram = true;
char[] buf = new char[BUFFER_SIZE];
- try (BufferedReader stdOutReader = new BufferedReader(new
InputStreamReader(proc.getInputStream()));
- BufferedReader stdErrReader =
- new BufferedReader(new
InputStreamReader(proc.getErrorStream()))) {
- IOTools.flow(stdErrReader, writer, buf);
- IOTools.flow(stdOutReader, writer, buf);
+ try {
+ try (BufferedReader stdOutReader = new BufferedReader(new
InputStreamReader(proc.getInputStream()));
+ BufferedReader stdErrReader = new BufferedReader(
+ new
InputStreamReader(proc.getErrorStream()))) {
+ // We don't spawn a thread here, since this is costly.
stderr would be usually written
+ // right away and the amount written would be small
+ IOTools.flow(stdErrReader, writer, buf);
+ IOTools.flow(stdOutReader, writer, buf);
+ }
+ proc.waitFor();
+ } finally {
+ proc.destroy();
}
- proc.waitFor();
lastModified = System.currentTimeMillis();
} catch (InterruptedException e) {
ssiMediator.log(sm.getString("ssiExec.executeFailed",
substitutedValue), e);
diff --git a/java/org/apache/catalina/ssi/SSIExternalResolver.java
b/java/org/apache/catalina/ssi/SSIExternalResolver.java
index 7eeece8c67..25c4e189e3 100644
--- a/java/org/apache/catalina/ssi/SSIExternalResolver.java
+++ b/java/org/apache/catalina/ssi/SSIExternalResolver.java
@@ -56,7 +56,7 @@ public interface SSIExternalResolver {
* Returns the current date. This is useful for putting the SSI stuff in a
regression test. Since you can make the
* current date a constant, it makes testing easier since the output won't
change.
*
- * @return the data
+ * @return the date
*/
Date getCurrentDate();
diff --git a/java/org/apache/catalina/ssi/SSIFilter.java
b/java/org/apache/catalina/ssi/SSIFilter.java
index 275bb66866..6101c60adb 100644
--- a/java/org/apache/catalina/ssi/SSIFilter.java
+++ b/java/org/apache/catalina/ssi/SSIFilter.java
@@ -26,6 +26,7 @@ import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.io.Reader;
import java.io.Serial;
+import java.nio.charset.StandardCharsets;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -173,7 +174,11 @@ public class SSIFilter extends GenericFilter {
// Ignore, will try to use a writer
}
if (out == null) {
- res.getWriter().write(new String(bytes));
+ String encoding = res.getCharacterEncoding();
+ if (encoding == null) {
+ encoding = StandardCharsets.ISO_8859_1.name();
+ }
+ res.getWriter().write(new String(bytes, encoding));
} else {
out.write(bytes);
}
diff --git a/java/org/apache/catalina/ssi/SSIMediator.java
b/java/org/apache/catalina/ssi/SSIMediator.java
index d1401fda2e..7d0310fe88 100644
--- a/java/org/apache/catalina/ssi/SSIMediator.java
+++ b/java/org/apache/catalina/ssi/SSIMediator.java
@@ -327,7 +327,7 @@ public class SSIMediator {
* @return the value after variable substitution
*/
public String substituteVariables(String val) {
- // If it has no references or HTML entities then no work need to be
done
+ // If it contains no variable references ($) or HTML entities (&), no
processing is needed.
if (val.indexOf('$') < 0 && val.indexOf('&') < 0) {
return val;
}
@@ -343,15 +343,30 @@ public class SSIMediator {
while (charStart > -1) {
int charEnd = sb.indexOf(";", charStart);
if (charEnd > -1) {
- char c = (char) Integer.parseInt(sb.substring(charStart + 2,
charEnd));
- sb.delete(charStart, charEnd + 1);
- sb.insert(charStart, c);
- charStart = sb.indexOf("&#");
+ boolean processed = false;
+ try {
+ int codePoint = Integer.parseInt(sb.substring(charStart +
2, charEnd));
+ if (codePoint >= 0 && codePoint <= 0xFFFF) {
+ char c = (char) codePoint;
+ sb.delete(charStart, charEnd + 1);
+ sb.insert(charStart, c);
+ processed = true;
+ }
+ } catch (NumberFormatException e) {
+ // Not a valid numeric entity – leave as-is
+ }
+ if (processed) {
+ charStart = sb.indexOf("&#", charStart + 1);
+ } else {
+ charStart = sb.indexOf("&#", charStart + 2);
+ }
} else {
break;
}
}
+ int maxSubstitutions = 100;
+ int substitutionCount = 0;
for (int i = 0; i < sb.length();) {
// Find the next $
for (; i < sb.length(); i++) {
@@ -364,13 +379,36 @@ public class SSIMediator {
break;
}
// Check to see if the $ is escaped
- if (i > 1 && sb.charAt(i - 2) == '\\') {
- sb.deleteCharAt(i - 2);
- i--;
- continue;
+ int dollarIdx = i - 1;
+ // Count consecutive backslashes before $
+ int backslashCount = 0;
+ int pos = dollarIdx - 1;
+ while (pos >= 0 && sb.charAt(pos) == '\\') {
+ backslashCount++;
+ pos--;
+ }
+ if (backslashCount > 0) {
+ int backslashStart = dollarIdx - backslashCount;
+ if (backslashCount % 2 == 1) {
+ // Odd: $ is escaped. Remove backslashes, keep $ as
literal.
+ sb.delete(backslashStart, dollarIdx);
+ for (int b = 0; b < backslashCount / 2; b++) {
+ sb.insert(backslashStart, '\\');
+ }
+ i = backslashStart + backslashCount / 2 + 1;
+ continue;
+ } else {
+ // Even: reduce backslashes, $ is NOT escaped.
+ sb.delete(backslashStart, dollarIdx);
+ for (int b = 0; b < backslashCount / 2; b++) {
+ sb.insert(backslashStart, '\\');
+ }
+ dollarIdx = backslashStart + backslashCount / 2;
+ i = dollarIdx + 1;
+ }
}
int nameStart = i;
- int start = i - 1;
+ int start = dollarIdx;
int end;
int nameEnd;
char endChar = ' ';
@@ -398,8 +436,13 @@ public class SSIMediator {
}
// Replace the var name with its value
sb.replace(start, end, value);
- // Start searching for the next $ after the value that was just
substituted.
- i = start + value.length();
+ // Re-scan from the start of the substituted value. A max iteration
+ // count prevents infinite loops from circular variable references
+ // (e.g. X="$Y" and Y="$X")
+ if (++substitutionCount > maxSubstitutions) {
+ break;
+ }
+ i = start;
}
return sb.toString();
}
diff --git a/java/org/apache/catalina/ssi/SSIProcessor.java
b/java/org/apache/catalina/ssi/SSIProcessor.java
index 06cf0d2e6c..276194d5db 100644
--- a/java/org/apache/catalina/ssi/SSIProcessor.java
+++ b/java/org/apache/catalina/ssi/SSIProcessor.java
@@ -23,7 +23,6 @@ import java.io.Reader;
import java.io.StringWriter;
import java.util.HashMap;
import java.util.Locale;
-import java.util.StringTokenizer;
import org.apache.catalina.util.IOTools;
@@ -197,47 +196,62 @@ public class SSIProcessor {
* @return an array with the parameter names
*/
protected String[] parseParamNames(StringBuilder cmd, int start) {
+ // Count parameters first
+ int count = 0;
int bIdx = start;
- int i = 0;
- int quotes;
- boolean inside = false;
- StringBuilder retBuf = new StringBuilder();
+ boolean insideQuotes = false;
+ boolean escaped = false;
while (bIdx < cmd.length()) {
- if (!inside) {
- while (bIdx < cmd.length() && isSpace(cmd.charAt(bIdx))) {
- bIdx++;
+ char c = cmd.charAt(bIdx);
+ if (escaped) {
+ escaped = false;
+ } else if (c == '\\') {
+ escaped = true;
+ } else if (c == '"' || c == '\'' || c == '`') {
+ insideQuotes = !insideQuotes;
+ } else if (c == '=' && !insideQuotes) {
+ count++;
+ }
+ bIdx++;
+ }
+ String[] retString = new String[count];
+ // Extract parameter names (characters between spaces and the first
unquoted '=')
+ bIdx = start;
+ int idx = 0;
+ StringBuilder nameBuf = new StringBuilder();
+ boolean collectingName = false;
+ insideQuotes = false;
+ escaped = false;
+ while (bIdx < cmd.length() && idx < count) {
+ char c = cmd.charAt(bIdx);
+ if (escaped) {
+ escaped = false;
+ if (collectingName && !insideQuotes) {
+ nameBuf.append(c);
}
- if (bIdx >= cmd.length()) {
- break;
+ } else if (c == '\\') {
+ escaped = true;
+ if (collectingName && !insideQuotes) {
+ nameBuf.append(c);
}
- inside = true;
- } else {
- while (bIdx < cmd.length() && cmd.charAt(bIdx) != '=') {
- retBuf.append(cmd.charAt(bIdx));
- bIdx++;
- }
- retBuf.append('=');
- inside = false;
- quotes = 0;
- boolean escaped = false;
- for (; bIdx < cmd.length() && quotes != 2; bIdx++) {
- char c = cmd.charAt(bIdx);
- // Need to skip escaped characters
- if (c == '\\' && !escaped) {
- escaped = true;
- continue;
- }
- if (c == '"' && !escaped) {
- quotes++;
- }
- escaped = false;
+ } else if (!insideQuotes && (c == '"' || c == '\'' || c == '`')) {
+ insideQuotes = true;
+ } else if (insideQuotes && (c == '"' || c == '\'' || c == '`')) {
+ insideQuotes = false;
+ } else if (!insideQuotes && isSpace(c)) {
+ if (collectingName) {
+ nameBuf.setLength(0);
+ collectingName = false;
}
+ } else if (!insideQuotes && c == '=') {
+ retString[idx++] = nameBuf.toString().trim();
+ nameBuf.setLength(0);
+ collectingName = false;
+ } else if (!insideQuotes) {
+ collectingName = true;
+ nameBuf.append(c);
}
- }
- StringTokenizer str = new StringTokenizer(retBuf.toString(), "=");
- String[] retString = new String[str.countTokens()];
- while (str.hasMoreTokens()) {
- retString[i++] = str.nextToken().trim();
+ bIdx++;
}
return retString;
}
diff --git a/java/org/apache/catalina/ssi/SSIServlet.java
b/java/org/apache/catalina/ssi/SSIServlet.java
index e1c3a7ccca..73f95e8e54 100644
--- a/java/org/apache/catalina/ssi/SSIServlet.java
+++ b/java/org/apache/catalina/ssi/SSIServlet.java
@@ -101,7 +101,7 @@ public class SSIServlet extends HttpServlet {
/**
- * Process and forward the GET request to our
<code>requestHandler()</code>.
+ * Delegate the GET request to our <code>requestHandler()</code>.
*
* @param req a value of type 'HttpServletRequest'
* @param res a value of type 'HttpServletResponse'
@@ -119,7 +119,7 @@ public class SSIServlet extends HttpServlet {
/**
- * Process and forward the POST request to our
<code>requestHandler()</code>.
+ * Delegate the POST request to our <code>requestHandler()</code>.
*
* @param req a value of type 'HttpServletRequest'
* @param res a value of type 'HttpServletResponse'
diff --git a/test/org/apache/catalina/ssi/TestExpressionParseTree.java
b/test/org/apache/catalina/ssi/TestExpressionParseTree.java
index 50c3856cd8..18ccebb9a2 100644
--- a/test/org/apache/catalina/ssi/TestExpressionParseTree.java
+++ b/test/org/apache/catalina/ssi/TestExpressionParseTree.java
@@ -136,9 +136,80 @@ public class TestExpressionParseTree {
}
+ @Test
+ public void testSubstituteVariablesPlainVar() throws Exception {
+ SSIMediator mediator = new SSIMediator(new
TesterSSIExternalResolver(), LAST_MODIFIED);
+ mediator.setVariableValue("VAR", "value");
+ Assert.assertEquals("value", mediator.substituteVariables("$VAR"));
+ }
+
+
+ @Test
+ public void testSubstituteVariablesBracedVar() throws Exception {
+ SSIMediator mediator = new SSIMediator(new
TesterSSIExternalResolver(), LAST_MODIFIED);
+ mediator.setVariableValue("VAR", "value");
+ Assert.assertEquals("value", mediator.substituteVariables("${VAR}"));
+ }
+
+
+ @Test
+ public void testSubstituteVariablesNoVar() throws Exception {
+ SSIMediator mediator = new SSIMediator(new
TesterSSIExternalResolver(), LAST_MODIFIED);
+ Assert.assertEquals("", mediator.substituteVariables("$UNKNOWN"));
+ }
+
+
+ @Test
+ public void testSubstituteVariablesSingleBackslashEscapesDollar() throws
Exception {
+ // Input: \$VAR (1 backslash) -> $ is escaped, backslash consumed ->
literal $VAR
+ SSIMediator mediator = new SSIMediator(new
TesterSSIExternalResolver(), LAST_MODIFIED);
+ mediator.setVariableValue("VAR", "value");
+ String input = "\\" + "$VAR";
+ Assert.assertEquals("$VAR", mediator.substituteVariables(input));
+ }
+
+
+ @Test
+ public void testSubstituteVariablesTwoBackslashesVarSubstituted() throws
Exception {
+ // Input: \\$VAR (2 backslashes) -> even, $ not escaped, reduce to 1
-> \ + value
+ SSIMediator mediator = new SSIMediator(new
TesterSSIExternalResolver(), LAST_MODIFIED);
+ mediator.setVariableValue("VAR", "value");
+ String input = "\\\\" + "$VAR";
+ Assert.assertEquals("\\" + "value",
mediator.substituteVariables(input));
+ }
+
+
+ @Test
+ public void testSubstituteVariablesThreeBackslashesEscapesDollar() throws
Exception {
+ // Input: 3 backslashes + $VAR -> odd, $ escaped, keep 1 backslash ->
\$VAR literal
+ SSIMediator mediator = new SSIMediator(new
TesterSSIExternalResolver(), LAST_MODIFIED);
+ mediator.setVariableValue("VAR", "value");
+ String input = "\\\\" + "\\" + "$VAR";
+ Assert.assertEquals("\\" + "$VAR",
mediator.substituteVariables(input));
+ }
+
+
+ @Test
+ public void testSubstituteVariablesFourBackslashesVarSubstituted() throws
Exception {
+ // Input: 4 backslashes + $VAR -> even, $ not escaped, reduce to 2 ->
\\ + value
+ SSIMediator mediator = new SSIMediator(new
TesterSSIExternalResolver(), LAST_MODIFIED);
+ mediator.setVariableValue("VAR", "value");
+ String input = "\\\\" + "\\\\" + "$VAR";
+ Assert.assertEquals("\\\\" + "value",
mediator.substituteVariables(input));
+ }
+
+
+ @Test
+ public void testSubstituteVariablesEscapedDollarFollowedByVar() throws
Exception {
+ // Input: \$Y$Y -> escaped $Y (literal) followed by variable $Y -> $Y
+ value
+ SSIMediator mediator = new SSIMediator(new
TesterSSIExternalResolver(), LAST_MODIFIED);
+ mediator.setVariableValue("Y", "result");
+ String input = "\\" + "$Y$Y";
+ Assert.assertEquals("$Yresult", mediator.substituteVariables(input));
+ }
+
/**
- * Minimal implementation that provides the bare essentials require for the
- * unit tests.
+ * Minimal implementation that provides the bare essentials require for
the unit tests.
*/
private static class TesterSSIExternalResolver
implements SSIExternalResolver {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 75bfc3f0f3..31cd2492af 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -253,6 +253,9 @@
<code>JAASRealm</code> should do a logout if login does not fail
outright but
does not produce a Principal. (remm)
</fix>
+ <fix>
+ Various edge cases for SSI substitutions, quoting and escaping. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
