You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/10/08 15:48:20 UTC
(tomcat) 11/23: Report the configured error message when a conditional SSI directive fails
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 0265a5d6e8f6cfb92587bca04b4a32f420ba6036
Author: opencode <re...@apache.org>
AuthorDate: Thu Oct 8 15:04:00 2026 +0200
Report the configured error message when a conditional SSI directive fails
The single catch site for SSIStopProcessingException in
SSIProcessor.process() assumes the command that threw has already
logged the failure and written the configured error message to the
response. That assumption did not hold for SSIConditional, which threw
bare when the expr attribute was missing or renamed and when the
expression could not be parsed, and for the unknown conditional
command branch. A malformed <!--#if--> therefore truncated the rest
of the document from the response with no error message and no log
entry, making the failure invisible and hard to diagnose.
SSIConditional now logs the failure and writes the configured error
message before throwing, mirroring the established error path of the
other commands such as SSISet, using the new ssiConditional.*
messages. In addition, SSIProcessor.process() now logs the exception
whenever it carries a wrapped cause, which previously was discarded
at the catch site by ExpressionParseTree.evaluateTree() wrapping
arbitrary Throwables, and whenever debugging is enabled, so no
failure path can remain silent.
Document truncation at a failing directive is unchanged; it mimics
Apache behaviour. Add TestSsiServlet.testBadConditionalExpressionReportsError
to cover the end-to-end behaviour.
---
.../apache/catalina/ssi/LocalStrings.properties | 4 +++
java/org/apache/catalina/ssi/SSIConditional.java | 16 ++++++++--
java/org/apache/catalina/ssi/SSIProcessor.java | 7 +++-
test/org/apache/catalina/ssi/TestSsiServlet.java | 37 ++++++++++++++++++++++
4 files changed, 60 insertions(+), 4 deletions(-)
diff --git a/java/org/apache/catalina/ssi/LocalStrings.properties b/java/org/apache/catalina/ssi/LocalStrings.properties
index 5cb718e6bd..5969272c58 100644
--- a/java/org/apache/catalina/ssi/LocalStrings.properties
+++ b/java/org/apache/catalina/ssi/LocalStrings.properties
@@ -25,6 +25,10 @@ expressionParseTree.unusedOpCodes=Unused nodes exist
ssiCommand.invalidAttribute=Invalid attribute [{0}]
+ssiConditional.errorEvaluatingExpression=Error evaluating expression [{0}]
+ssiConditional.noExpression=No expression specified
+ssiConditional.unknownCommand=Unknown conditional command [{0}]
+
ssiEcho.invalidEncoding=Invalid encoding [{0}]
ssiExec.executeFailed=Cannot execute file [{0}]
diff --git a/java/org/apache/catalina/ssi/SSIConditional.java b/java/org/apache/catalina/ssi/SSIConditional.java
index d122298265..014f196eaf 100644
--- a/java/org/apache/catalina/ssi/SSIConditional.java
+++ b/java/org/apache/catalina/ssi/SSIConditional.java
@@ -20,11 +20,15 @@ package org.apache.catalina.ssi;
import java.io.PrintWriter;
import java.text.ParseException;
+import org.apache.tomcat.util.res.StringManager;
+
/**
* SSI command that handles all conditional directives.
*/
public class SSIConditional implements SSICommand {
+ private static final StringManager sm = StringManager.getManager(SSIConditional.class);
+
/**
* Default constructor.
@@ -50,7 +54,7 @@ public class SSIConditional implements SSICommand {
}
state.nestingCount = 0;
// Evaluate the expression
- if (evaluateArguments(paramNames, paramValues, ssiMediator)) {
+ if (evaluateArguments(paramNames, paramValues, ssiMediator, writer)) {
// No more branches can be taken for this if block
state.branchTaken = true;
} else {
@@ -71,7 +75,7 @@ public class SSIConditional implements SSICommand {
return lastModified;
}
// Evaluate the expression
- if (evaluateArguments(paramNames, paramValues, ssiMediator)) {
+ if (evaluateArguments(paramNames, paramValues, ssiMediator, writer)) {
// Turn back on output and mark the branch
state.processConditionalCommandsOnly = false;
state.branchTaken = true;
@@ -106,6 +110,8 @@ public class SSIConditional implements SSICommand {
// in the first place.
state.branchTaken = true;
} else {
+ ssiMediator.log(sm.getString("ssiConditional.unknownCommand", commandName));
+ writer.write(ssiMediator.getConfigErrMsg());
throw new SSIStopProcessingException();
}
return lastModified;
@@ -115,16 +121,20 @@ public class SSIConditional implements SSICommand {
/**
* Retrieves the expression from the specified arguments and performs the necessary evaluation steps.
*/
- private boolean evaluateArguments(String[] names, String[] values, SSIMediator ssiMediator)
+ private boolean evaluateArguments(String[] names, String[] values, SSIMediator ssiMediator, PrintWriter writer)
throws SSIStopProcessingException {
String expr = getExpression(names, values);
if (expr == null) {
+ ssiMediator.log(sm.getString("ssiConditional.noExpression"));
+ writer.write(ssiMediator.getConfigErrMsg());
throw new SSIStopProcessingException();
}
try {
ExpressionParseTree tree = new ExpressionParseTree(expr, ssiMediator);
return tree.evaluateTree();
} catch (ParseException e) {
+ ssiMediator.log(sm.getString("ssiConditional.errorEvaluatingExpression", expr), e);
+ writer.write(ssiMediator.getConfigErrMsg());
throw new SSIStopProcessingException();
}
}
diff --git a/java/org/apache/catalina/ssi/SSIProcessor.java b/java/org/apache/catalina/ssi/SSIProcessor.java
index bfd3e0cce1..ca0a4aa41d 100644
--- a/java/org/apache/catalina/ssi/SSIProcessor.java
+++ b/java/org/apache/catalina/ssi/SSIProcessor.java
@@ -181,7 +181,12 @@ public class SSIProcessor {
}
}
} catch (SSIStopProcessingException e) {
- // If we are here, then we have already stopped processing, so all is good
+ // If we are here, then processing has already been stopped by a command that
+ // reported its own error. Log any wrapped cause so nothing can fail silently,
+ // and log everything when debugging is enabled.
+ if (debug > 0 || e.getCause() != null) {
+ ssiExternalResolver.log("SSI processing stopped", e);
+ }
}
return lastModifiedDate;
}
diff --git a/test/org/apache/catalina/ssi/TestSsiServlet.java b/test/org/apache/catalina/ssi/TestSsiServlet.java
index d1363782c3..3599530415 100644
--- a/test/org/apache/catalina/ssi/TestSsiServlet.java
+++ b/test/org/apache/catalina/ssi/TestSsiServlet.java
@@ -117,4 +117,41 @@ public class TestSsiServlet extends TomcatBaseTest {
}
Assert.assertNotNull(lastModified);
}
+
+
+ @Test
+ public void testBadConditionalExpressionReportsError() throws Exception {
+ // A conditional directive with an expression that cannot be parsed
+ // stops processing of the document, but it must report the default
+ // error message rather than truncating the response silently.
+ Tomcat tomcat = getTomcatInstance();
+
+ File appDir = new File(getTemporaryDirectory(), "ssi-badif");
+ Assert.assertTrue(appDir.mkdirs() || appDir.isDirectory());
+ addDeleteOnTearDown(appDir);
+ File doc = new File(appDir, "badif.shtml");
+ try (Writer writer = new OutputStreamWriter(new FileOutputStream(doc), StandardCharsets.ISO_8859_1)) {
+ writer.write("BEFORE-CONTENT\n");
+ writer.write("<!--#if expr=\"this is not valid (\"-->\n");
+ writer.write("AFTER-CONTENT\n");
+ }
+
+ Context ctxt = tomcat.addContext("", appDir.getAbsolutePath());
+ Tomcat.addServlet(ctxt, "ssi", new SSIServlet());
+ ctxt.addServletMapping("*.shtml", "ssi");
+
+ tomcat.start();
+
+ Map<String,List<String>> resHeaders = new HashMap<>();
+ String path = "http://localhost:" + getPort() + "/badif.shtml";
+ ByteChunk out = new ByteChunk();
+
+ int rc = getUrl(path, out, resHeaders);
+ Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+ String body = new String(out.getBuffer(), 0, out.getLength(), StandardCharsets.ISO_8859_1);
+ Assert.assertTrue(body.contains("BEFORE-CONTENT"));
+ Assert.assertTrue(body.contains("[an error occurred"));
+ // Processing of the remainder of the document stops at the failing directive
+ Assert.assertFalse(body.contains("AFTER-CONTENT"));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org