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
commit 4027a4e0cb16e313a9acfe37baa56e6a2791a5c0 Author: opencode <[email protected]> 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 b489bfd691..2c4d04e039 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: [email protected] For additional commands, e-mail: [email protected]
