(tomcat) 02/11: Keep the Digester loggers usable after clear()

[re...@apache.org]

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

commit ced49783cb538461f75e66f3bb0eb8d758e5338d
Author: opencode <re...@apache.org>
AuthorDate: Wed Oct 14 16:52:23 2026 +0200

    Keep the Digester loggers usable after clear()
    
    clear() nulled log and saxLog. It runs at the end of every successful
    parse via endDocument(), on every failed parse via the reset added in
    the previous commit, and via the public reset() method, so afterwards
    the loggers stayed null until the next parse re-ran configure().
    
    That left several public methods broken in exactly the empty state
    their Javadoc promises a null return or a plain call: peek(), peek(int)
    and pop() threw a NullPointerException inside their
    catch(EmptyStackException) handler instead of returning null, peekParams
    and popParams behaved the same way, popParams(), pushParams() and
    register() hit a NullPointerException unconditionally on the trace
    enablement check, and createSAXException(String, Exception) did so on
    its log.error path when no document locator has been set, for example
    after a parse failure on a file that could not be opened.
    
    The nulling also served no purpose. JULI loggers are cached by name by
    the logging implementation, so retaining the two handles between parses
    costs nothing, and configure() re-assigned equivalent handles on every
    subsequent parse anyway. Nothing reads the fields to test for null and
    no Digester subclass in the tree overrides configure().
    
    Adds regression tests to TestDigester for the stack and parameter
    methods after a successful parse and for createSAXException() in the
    no-locator state left by a failed parse; both fail with a
    NullPointerException without this change.
---
 java/org/apache/tomcat/util/digester/Digester.java |  2 -
 .../apache/tomcat/util/digester/TestDigester.java  | 44 ++++++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/tomcat/util/digester/Digester.java b/java/org/apache/tomcat/util/digester/Digester.java
index 9d9b1693bb..4690c3991e 100644
--- a/java/org/apache/tomcat/util/digester/Digester.java
+++ b/java/org/apache/tomcat/util/digester/Digester.java
@@ -1851,8 +1851,6 @@ public class Digester extends DefaultHandler2 {
         params.clear();
         publicId = null;
         stack.clear();
-        log = null;
-        saxLog = null;
         configured = false;
 
     }
diff --git a/test/org/apache/tomcat/util/digester/TestDigester.java b/test/org/apache/tomcat/util/digester/TestDigester.java
index f0930c64bd..25eaf04f66 100644
--- a/test/org/apache/tomcat/util/digester/TestDigester.java
+++ b/test/org/apache/tomcat/util/digester/TestDigester.java
@@ -16,6 +16,8 @@
  */
 package org.apache.tomcat.util.digester;
 
+import java.io.File;
+import java.io.IOException;
 import java.io.StringReader;
 
 import org.junit.Assert;
@@ -141,4 +143,46 @@ public class TestDigester {
         Assert.assertEquals(1, rule.getBegins());
     }
 
+
+    /*
+     * clear() runs at the end of every successful parse. The methods that are
+     * documented to return null on an empty stack must keep working after the
+     * digester has been cleared.
+     */
+    @Test
+    public void testStackMethodsAfterSuccessfulParse() throws Exception {
+        Digester digester = new Digester();
+        digester.addObjectCreate("root", "java.lang.Object");
+
+        digester.parse(new InputSource(new StringReader("<root/>")));
+
+        Assert.assertNull(digester.peek());
+        Assert.assertNull(digester.peek(0));
+        Assert.assertNull(digester.pop());
+        Assert.assertNull(digester.peekParams());
+        Assert.assertNull(digester.popParams());
+        digester.pushParams(new Object());
+        digester.register("public-id", "system-id");
+    }
+
+
+    /*
+     * A parse failure before setDocumentLocator() leaves the locator null, so
+     * createSAXException() takes the log.error path. That path must work even
+     * though the failed parse has run clear() via the parse() failure handling.
+     */
+    @Test
+    public void testCreateSaxExceptionAfterFailedParseWithoutLocator() throws Exception {
+        Digester digester = new Digester();
+
+        try {
+            digester.parse(new File("does-not-exist.xml"));
+            Assert.fail("Expected an IOException for the missing file");
+        } catch (IOException e) {
+            // Expected
+        }
+
+        Assert.assertNotNull(digester.createSAXException("message", new RuntimeException("cause")));
+    }
+
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org