From e3794f4e5e8867db564110abe4a8f0a68394b34b Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Fri, 23 Jun 2023 19:06:47 +0200 Subject: [PATCH 1/2] JSPWIKI-388 - ParamTag loops forever when parent tag is not ParamHandler This commit addresses an issue in the ParamTag class where, if the parent tag was not an instance of ParamHandler, an infinite loop would occur. This was due to the 'getParent()' method always returning the same object. The fix involves getting the parent of the current tag in the loop, thereby ensuring that we keep moving up the tree until we find a parent that is a ParamHandler or until we run out of parents. This prevents the possibility of an infinite loop and provides a more robust handling of the tag structure. --- .../java/org/apache/wiki/tags/ParamTag.java | 26 ++-- .../java/org/apache/wiki/ParamTagTest.java | 147 ++++++++++++++++++ 2 files changed, 164 insertions(+), 9 deletions(-) create mode 100644 jspwiki-main/src/test/java/org/apache/wiki/ParamTagTest.java diff --git a/jspwiki-main/src/main/java/org/apache/wiki/tags/ParamTag.java b/jspwiki-main/src/main/java/org/apache/wiki/tags/ParamTag.java index 9592532f84..83c42badd4 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/tags/ParamTag.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/tags/ParamTag.java @@ -21,6 +21,8 @@ Licensed to the Apache Software Foundation (ASF) under one import javax.servlet.jsp.tagext.BodyContent; import javax.servlet.jsp.tagext.BodyTagSupport; import javax.servlet.jsp.tagext.Tag; +import java.util.HashSet; +import java.util.Set; /** * ParamTag submits name-value pairs to the first enclosing @@ -74,11 +76,19 @@ public void setValue(final String s ) @Override public int doEndTag() { - Tag t; - do - { - t = getParent(); - } while (t != null && !(t instanceof ParamHandler)); + // Start with the direct parent of this tag + Tag t = getParent(); + final Set visited = new HashSet<>(); + + // Keep moving up the tree until we find a parent that is a ParamHandler, + // or until we run out of parents. This prevents an infinite loop in case + // the parent of this tag is not a ParamHandler. + while (t != null && !(t instanceof ParamHandler)) { + if (!visited.add(t)) { + break; // We've seen this tag before, so break out of the loop + } + t = t.getParent(); + } if( t != null ) { @@ -86,18 +96,16 @@ public int doEndTag() if( val == null ) { final BodyContent bc = getBodyContent(); - if( bc != null ) + if( bc != null ) { val = bc.getString(); } } - if( val != null ) + if( val != null ) { ((ParamHandler)t).setContainedParameter( m_name, val ); } } - - return EVAL_PAGE; } } diff --git a/jspwiki-main/src/test/java/org/apache/wiki/ParamTagTest.java b/jspwiki-main/src/test/java/org/apache/wiki/ParamTagTest.java new file mode 100644 index 0000000000..e2ae2f735a --- /dev/null +++ b/jspwiki-main/src/test/java/org/apache/wiki/ParamTagTest.java @@ -0,0 +1,147 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + */ + +package org.apache.wiki; + +import org.apache.wiki.tags.ParamHandler; +import org.apache.wiki.tags.ParamTag; +import org.junit.jupiter.api.Test; + +import javax.servlet.jsp.tagext.BodyContent; +import javax.servlet.jsp.tagext.BodyTagSupport; +import javax.servlet.jsp.tagext.Tag; +import java.time.Duration; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTimeout; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class ParamTagTest { + + + @Test + public void testDoEndTagWithNonParamHandlerParent() { + // Create a mock parent tag that is not a ParamHandler + final Tag parentTag = mock(Tag.class); + when(parentTag.getParent()).thenReturn(null); // End of the parent chain + + // Create the ParamTag under test and set its parent + final ParamTag paramTag = new ParamTag(); + paramTag.setParent(parentTag); + + // Call the method under test + final int result = paramTag.doEndTag(); + + // Assert that the method terminated and returned the expected value + assertEquals(Tag.EVAL_PAGE, result); + } + + + @Test + public void testDoEndTagWithInfiniteLoop() throws Exception { + // Create a tag that will cause an infinite loop + final Tag parentTag = mock(Tag.class); + when(parentTag.getParent()).thenReturn(parentTag); + + final ParamTagOld paramTag = new ParamTagOld(); + paramTag.setParent(parentTag); + + // Run the code that should cause an infinite loop in a separate thread + final ExecutorService executorService = Executors.newSingleThreadExecutor(); + final Future future = executorService.submit(paramTag::doEndTag); + + try { + // Allow the test to run for 5 seconds. If it takes longer, consider it an infinite loop. + future.get(5, TimeUnit.SECONDS); + fail("Expected an infinite loop, but code terminated normally."); + } catch (final TimeoutException e) { + // This is the expected outcome, as the code should run indefinitely. + } finally { + // Clean up by shutting down the executor + executorService.shutdownNow(); + } + } + + @Test + void testNewCodeDoesNotCauseInfiniteLoop() { + final ParamTag paramTag = new ParamTag(); // New code class + + final Tag parentTag = mock(Tag.class); + when(parentTag.getParent()).thenReturn(parentTag); // Parent returns itself + + paramTag.setParent(parentTag); + + assertTimeout(Duration.ofSeconds(5), () -> { + paramTag.doEndTag(); + }, "Execution should not have taken more than 5 seconds"); + } + + + static class ParamTagOld + extends BodyTagSupport { + + private static final long serialVersionUID = -4671059568218551633L; + private String m_name; + private String m_value; + + @Override + public void release() { + m_name = m_value = null; + } + + public void setName(final String s) { + m_name = s; + } + + public void setValue(final String s) { + m_value = s; + } + + @Override + public int doEndTag() { + Tag t; + do { + t = getParent(); + } while (t != null && !(t instanceof ParamHandler)); + + if (t != null) { + String val = m_value; + if (val == null) { + final BodyContent bc = getBodyContent(); + if (bc != null) { + val = bc.getString(); + } + } + if (val != null) { + ((ParamHandler) t).setContainedParameter(m_name, val); + } + } + + + return EVAL_PAGE; + } + } +} \ No newline at end of file From f2addbfa5c488fd113ba4f05189ecb2c6a2a464d Mon Sep 17 00:00:00 2001 From: Alex O'Ree Date: Thu, 13 Nov 2025 10:19:19 -0500 Subject: [PATCH 2/2] compile fix for unit tests --- .../src/test/java/org/apache/wiki/ParamTagTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/jspwiki-main/src/test/java/org/apache/wiki/ParamTagTest.java b/jspwiki-main/src/test/java/org/apache/wiki/ParamTagTest.java index e2ae2f735a..d28c0b9a2e 100644 --- a/jspwiki-main/src/test/java/org/apache/wiki/ParamTagTest.java +++ b/jspwiki-main/src/test/java/org/apache/wiki/ParamTagTest.java @@ -23,9 +23,9 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.tags.ParamTag; import org.junit.jupiter.api.Test; -import javax.servlet.jsp.tagext.BodyContent; -import javax.servlet.jsp.tagext.BodyTagSupport; -import javax.servlet.jsp.tagext.Tag; +import jakarta.servlet.jsp.tagext.BodyContent; +import jakarta.servlet.jsp.tagext.BodyTagSupport; +import jakarta.servlet.jsp.tagext.Tag; import java.time.Duration; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors;