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 9e3c68adc057f86c168bfa5c53bcf724f2285710 Author: opencode <[email protected]> AuthorDate: Mon Oct 19 14:33:08 2026 +0200 Share the Import tag plugin URL with its params via the plugin context The Import and Param tag plugins exchanged the working URL of the import tag through the page scope attribute url_without_param: the Import plugin wrote the attribute before generating the tag body, read it back afterwards and removed it. The fixed name overwrote any application page attribute of the same name while the body was processed, removed it at the end, and let nested plugin enabled URL tags interfere with the value read back. A c:import nested in the body of a plugin enabled c:url, a nesting the JSTL tag library validator accepts, therefore always failed at runtime, because the inner tag removed the attribute the outer tag then read as null. Share the name of the generated local variable that holds the working URL through the plugin context instead, the mechanism the Choose plugin already uses to mark its context for When and Otherwise. The generated code of the Param plugin appends directly to that variable when the parent provides it, and keeps the former attribute protocol otherwise, so the Url and Redirect plugins, which still use the attribute, generate byte-identical code. The nesting of an import inside a non-varReader import, the path the report assumed was legal from the body-content of the TLD, never reaches code generation since the JSTL tag library validator rejects any child other than c:param there. Add tests pinning the user attribute, the parameter appending and the validator rejection. Found by code review. --- .../apache/jasper/tagplugins/jstl/core/Import.java | 21 ++++- .../apache/jasper/tagplugins/jstl/core/Param.java | 31 +++++-- .../jasper/tagplugins/jstl/core/TestImport.java | 95 ++++++++++++++++++++++ .../tagplugins/jstl/core/import-tagPlugins.xml | 27 ++++++ test/webapp/jstl/import-clobber.jsp | 24 ++++++ test/webapp/jstl/import-nested.jsp | 23 ++++++ test/webapp/jstl/import-params.jsp | 23 ++++++ test/webapp/jstl/import-target.jsp | 19 +++++ webapps/docs/changelog.xml | 11 +++ 9 files changed, 262 insertions(+), 12 deletions(-) diff --git a/java/org/apache/jasper/tagplugins/jstl/core/Import.java b/java/org/apache/jasper/tagplugins/jstl/core/Import.java index 3f564c805d..706cb48fb1 100644 --- a/java/org/apache/jasper/tagplugins/jstl/core/Import.java +++ b/java/org/apache/jasper/tagplugins/jstl/core/Import.java @@ -25,6 +25,16 @@ import org.apache.jasper.tagplugins.jstl.Util; */ public class Import implements TagPlugin { + /** + * Key of the plugin attribute through which a URL tag shares its working + * URL with the {@link Param} tags in its body. The value is the name of + * the generated local variable that holds the URL and to which the + * parameters are appended. Sharing the variable rather than a page scope + * attribute with a fixed name makes the protocol re-entrant: a nested URL + * tag in the body cannot overwrite or remove the outer tag's URL. + */ + static final String URL_VARIABLE_ATTRIBUTE = "urlVariable"; + /** * Creates a new instance of the Import tag plugin. */ @@ -279,10 +289,15 @@ public class Import implements TagPlugin { // if the varReader is not specified else { - ctxt.generateJavaSource("_jspx_page_context.setAttribute(\"url_without_param\"," + urlName + ");"); + // Share the working URL with the <c:param> tags in the body by + // handing them the name of the local variable that holds it. + // Appending to the variable directly is re-entrant: a nested URL + // tag in the body of this tag must not be able to overwrite or + // remove it, which a page scope attribute with a fixed name + // would allow. + ctxt.setPluginAttribute(URL_VARIABLE_ATTRIBUTE, urlName); ctxt.generateBody(); - ctxt.generateJavaSource(urlName + " = (String)_jspx_page_context.getAttribute(\"url_without_param\");"); - ctxt.generateJavaSource("_jspx_page_context.removeAttribute(\"url_without_param\");"); + String strScope = "page"; if (hasScope) { strScope = ctxt.getConstantAttribute("scope"); diff --git a/java/org/apache/jasper/tagplugins/jstl/core/Param.java b/java/org/apache/jasper/tagplugins/jstl/core/Param.java index 006bf2675d..76e0d680f4 100644 --- a/java/org/apache/jasper/tagplugins/jstl/core/Param.java +++ b/java/org/apache/jasper/tagplugins/jstl/core/Param.java @@ -52,9 +52,20 @@ public class Param implements TagPlugin { return; } - // get the url string before adding this param - ctxt.generateJavaSource( - "String " + urlName + " = " + "(String)_jspx_page_context.getAttribute(\"url_without_param\");"); + // Append the parameter to the parent tag's working URL. The Import + // plugin shares the name of the local variable holding that URL + // through the plugin context, so that nested URL tags cannot + // interfere with it through a shared page scope attribute. The Url + // and Redirect plugins still use such an attribute. + Object urlVariable = parent.getPluginAttribute(Import.URL_VARIABLE_ATTRIBUTE); + String urlTargetName; + if (urlVariable != null) { + urlTargetName = (String) urlVariable; + } else { + ctxt.generateJavaSource( + "String " + urlName + " = " + "(String)_jspx_page_context.getAttribute(\"url_without_param\");"); + urlTargetName = urlName; + } // get the value of "name" ctxt.generateJavaSource("String " + nameName + " = "); @@ -76,17 +87,19 @@ public class Param implements TagPlugin { ctxt.generateJavaSource( " " + valueName + " = java.net.URLEncoder.encode(" + valueName + ", " + encName + ");"); ctxt.generateJavaSource(" int " + index + ";"); - ctxt.generateJavaSource(" " + index + " = " + urlName + ".indexOf(\'?\');"); + ctxt.generateJavaSource(" " + index + " = " + urlTargetName + ".indexOf(\'?\');"); // if the current param is the first one, add a "?" ahead of it // else add a "&" ahead of it ctxt.generateJavaSource(" if(" + index + " == -1){"); - ctxt.generateJavaSource( - " " + urlName + " = " + urlName + " + \"?\" + " + nameName + " + \"=\" + " + valueName + ";"); + ctxt.generateJavaSource(" " + urlTargetName + " = " + urlTargetName + " + \"?\" + " + nameName + + " + \"=\" + " + valueName + ";"); ctxt.generateJavaSource(" }else{"); - ctxt.generateJavaSource( - " " + urlName + " = " + urlName + " + \"&\" + " + nameName + " + \"=\" + " + valueName + ";"); + ctxt.generateJavaSource(" " + urlTargetName + " = " + urlTargetName + " + \"&\" + " + nameName + + " + \"=\" + " + valueName + ";"); ctxt.generateJavaSource(" }"); - ctxt.generateJavaSource(" _jspx_page_context.setAttribute(\"url_without_param\"," + urlName + ");"); + if (urlVariable == null) { + ctxt.generateJavaSource(" _jspx_page_context.setAttribute(\"url_without_param\"," + urlName + ");"); + } ctxt.generateJavaSource("}"); } } diff --git a/test/org/apache/jasper/tagplugins/jstl/core/TestImport.java b/test/org/apache/jasper/tagplugins/jstl/core/TestImport.java new file mode 100644 index 0000000000..2c59d5ce37 --- /dev/null +++ b/test/org/apache/jasper/tagplugins/jstl/core/TestImport.java @@ -0,0 +1,95 @@ +/* + * 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.jasper.tagplugins.jstl.core; + +import java.io.File; + +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.tomcat.util.buf.ByteChunk; + +public class TestImport extends AbstractTestTag { + + @Override + protected File getTagPluginsFile() { + return new File("test/org/apache/jasper/tagplugins/jstl/core/import-tagPlugins.xml"); + } + + /* + * The <c:param> plugin must add its parameters to the URL of its own + * <c:import> tag. The plugins previously exchanged the working URL + * through a page scope attribute named "url_without_param". That fixed + * name overwrote any application attribute of the same name while the + * body was processed and removed it at the end. The URL is now shared + * with the body through the plugin context instead. + */ + @Test + public void testParamKeepsUserAttribute() throws Exception { + ByteChunk res = new ByteChunk(); + + int rc = getUrl("http://localhost:"; + getPort() + + "/test/jstl/import-clobber.jsp", res, null); + + Assert.assertEquals(HttpServletResponse.SC_OK, rc); + + String body = res.toString(); + Assert.assertTrue(body, body.contains("KEEP=[keepme]")); + } + + + /* + * Parameters are appended to the URL, URL-encoded, the first one + * following the existing query string with "&" (the target reads the + * query string of its include dispatch from the Servlet include + * attribute). + */ + @Test + public void testParams() throws Exception { + ByteChunk res = new ByteChunk(); + + int rc = getUrl("http://localhost:"; + getPort() + + "/test/jstl/import-params.jsp", res, null); + + Assert.assertEquals(HttpServletResponse.SC_OK, rc); + + String body = res.toString(); + Assert.assertTrue(body, body.contains("QS=[a=b&sp=x+y&u=%26%3D]")); + } + + + /* + * A nested <c:import> in the body of a non-varReader <c:import> is + * rejected by the JSTL tag library validator before code generation + * (only <c:param> children are allowed there), which is why the body of + * the tag can never remove the working URL of the outer tag. + */ + @Test + public void testNestedImportRejected() throws Exception { + ByteChunk res = new ByteChunk(); + + int rc = getUrl("http://localhost:"; + getPort() + + "/test/jstl/import-nested.jsp", res, null); + + Assert.assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, rc); + + String body = res.toString(); + Assert.assertTrue(body, body.contains("Illegal child tag")); + } +} diff --git a/test/org/apache/jasper/tagplugins/jstl/core/import-tagPlugins.xml b/test/org/apache/jasper/tagplugins/jstl/core/import-tagPlugins.xml new file mode 100644 index 0000000000..2ed6156c07 --- /dev/null +++ b/test/org/apache/jasper/tagplugins/jstl/core/import-tagPlugins.xml @@ -0,0 +1,27 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + 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. +--> +<tag-plugins> + <tag-plugin> + <tag-class>org.apache.taglibs.standard.tag.rt.core.ImportTag</tag-class> + <plugin-class>org.apache.jasper.tagplugins.jstl.core.Import</plugin-class> + </tag-plugin> + <tag-plugin> + <tag-class>org.apache.taglibs.standard.tag.rt.core.ParamTag</tag-class> + <plugin-class>org.apache.jasper.tagplugins.jstl.core.Param</plugin-class> + </tag-plugin> +</tag-plugins> diff --git a/test/webapp/jstl/import-clobber.jsp b/test/webapp/jstl/import-clobber.jsp new file mode 100644 index 0000000000..a48880f440 --- /dev/null +++ b/test/webapp/jstl/import-clobber.jsp @@ -0,0 +1,24 @@ +<%-- + 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. +--%> +<%@ page session="false" contentType="text/html; charset=UTF-8" + pageEncoding="UTF-8" %> +<%@ taglib uri="http://java.sun.com/jsp/jstl/core"; prefix="c" %> +<c:set var="url_without_param" value="keepme" scope="page" /> +<c:import url="import-target.jsp" var="outer"><c:param + name="p" value="q"/></c:import> +<p>KEEP=[${url_without_param}]</p> +<p>OUTER=[${outer}]</p> diff --git a/test/webapp/jstl/import-nested.jsp b/test/webapp/jstl/import-nested.jsp new file mode 100644 index 0000000000..34ad028b28 --- /dev/null +++ b/test/webapp/jstl/import-nested.jsp @@ -0,0 +1,23 @@ +<%-- + 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. +--%> +<%@ page session="false" contentType="text/html; charset=UTF-8" + pageEncoding="UTF-8" %> +<%@ taglib uri="http://java.sun.com/jsp/jstl/core"; prefix="c" %> +<c:import url="import-target.jsp" var="outer"><c:import + url="import-target.jsp?i=1" var="inner"/></c:import> +<p>OUTER=[${outer}]</p> +<p>INNER=[${inner}]</p> diff --git a/test/webapp/jstl/import-params.jsp b/test/webapp/jstl/import-params.jsp new file mode 100644 index 0000000000..d46323c33d --- /dev/null +++ b/test/webapp/jstl/import-params.jsp @@ -0,0 +1,23 @@ +<%-- + 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. +--%> +<%@ page session="false" contentType="text/html; charset=UTF-8" + pageEncoding="UTF-8" %> +<%@ taglib uri="http://java.sun.com/jsp/jstl/core"; prefix="c" %> +<c:import url="import-target.jsp?a=b" var="outer"><c:param + name="sp" value="x y"/><c:param name="u" + value="${'&='}"/></c:import> +<p>OUTER=[${outer}]</p> diff --git a/test/webapp/jstl/import-target.jsp b/test/webapp/jstl/import-target.jsp new file mode 100644 index 0000000000..edec2b4b86 --- /dev/null +++ b/test/webapp/jstl/import-target.jsp @@ -0,0 +1,19 @@ +<%-- + 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. +--%> +<%@ page session="false" contentType="text/html; charset=UTF-8" + pageEncoding="UTF-8" %> +QS=[${requestScope['jakarta.servlet.include.query_string']}] diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index fdd1c74457..b1ca142caa 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -439,6 +439,17 @@ illegal nesting at runtime as the reference implementation does. (remm) </fix> + <fix> + The tag plugins for <code>c:import</code> and <code>c:param</code> now + share the working URL of the import tag through the plugin context + rather than the page scope attribute <code>url_without_param</code>. + The import plugin set that attribute while generating the tag body and + removed it afterwards, clobbering an application attribute of the same + name, and the fixed name made the exchange prone to interference + between nested URL tags. This matches the reference implementation, + which keeps the parameters of each tag to itself and never touches + page scope attributes. (remm) + </fix> </changelog> </subsection> <subsection name="WebSocket"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
