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 511586603b03b219ba5b9e628162677a54c99454 Author: opencode <[email protected]> AuthorDate: Mon Oct 19 17:04:45 2026 +0200 Keep UniqueAttributesImpl tracking in step with its attributes setQName() removed the attribute's old name from the duplicate tracking set but never added the new one, so after a rename the new name was no longer detected as a duplicate and the old name was forgotten even though the rename could still be rejected later. setAttribute(int,...) lost the old name the same way whenever a rejected or tolerated duplicate left the attribute unchanged. Rename the tracking entry alongside the attribute, and restore the old name before consulting handleDuplicate() so the tracking stays consistent whether the duplicate is tolerated or rejected with an exception. Add TestUniqueAttributesImpl covering the rename, rename-to-existing and replacement cases. Found by code review. --- .../apache/jasper/util/UniqueAttributesImpl.java | 20 +++- .../jasper/util/TestUniqueAttributesImpl.java | 120 +++++++++++++++++++++ 2 files changed, 137 insertions(+), 3 deletions(-) diff --git a/java/org/apache/jasper/util/UniqueAttributesImpl.java b/java/org/apache/jasper/util/UniqueAttributesImpl.java index b7ba09e5c4..e36268417d 100644 --- a/java/org/apache/jasper/util/UniqueAttributesImpl.java +++ b/java/org/apache/jasper/util/UniqueAttributesImpl.java @@ -78,10 +78,15 @@ public class UniqueAttributesImpl extends AttributesImpl { @Override public void setAttribute(int index, String uri, String localName, String qName, String type, String value) { - qNames.remove(super.getQName(index)); + String oldQName = super.getQName(index); + qNames.remove(oldQName); if (qNames.add(qName)) { super.setAttribute(index, uri, localName, qName, type, value); } else { + // A duplicate, whether it is tolerated by the rules below or + // rejected with an exception, leaves this attribute unchanged, so + // restore its name in the tracking set + qNames.add(oldQName); handleDuplicate(qName, value); } } @@ -94,8 +99,17 @@ public class UniqueAttributesImpl extends AttributesImpl { @Override public void setQName(int index, String qName) { - qNames.remove(super.getQName(index)); - super.setQName(index, qName); + String oldQName = super.getQName(index); + qNames.remove(oldQName); + if (qNames.add(qName)) { + super.setQName(index, qName); + } else { + // A duplicate, whether it is tolerated by the rules below or + // rejected with an exception, leaves this attribute unchanged, so + // restore its name in the tracking set + qNames.add(oldQName); + handleDuplicate(qName, super.getValue(index)); + } } private void handleDuplicate(String qName, String value) { diff --git a/test/org/apache/jasper/util/TestUniqueAttributesImpl.java b/test/org/apache/jasper/util/TestUniqueAttributesImpl.java new file mode 100644 index 0000000000..68b2810e2c --- /dev/null +++ b/test/org/apache/jasper/util/TestUniqueAttributesImpl.java @@ -0,0 +1,120 @@ +/* + * 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.util; + +import org.junit.Assert; +import org.junit.Test; + + +public class TestUniqueAttributesImpl { + + private static UniqueAttributesImpl createAttributes() { + UniqueAttributesImpl attrs = new UniqueAttributesImpl(); + attrs.addAttribute("", "a", "a", "CDATA", "1"); + attrs.addAttribute("", "b", "b", "CDATA", "2"); + return attrs; + } + + + /* + * Renaming an attribute to a new name must register that name, so a + * later attribute of the same name is still detected as a duplicate. + * The method used to remove the old name without adding the new one, + * leaving the tracking set behind the attributes. + */ + @Test + public void testSetQNameRegistersNewName() throws Exception { + UniqueAttributesImpl attrs = createAttributes(); + + attrs.setQName(1, "c"); + Assert.assertEquals("c", attrs.getQName(1)); + + try { + attrs.addAttribute("", "c", "c", "CDATA", "3"); + Assert.fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + // expected + } + } + + + /* + * Renaming an attribute to the name of another attribute must be + * rejected as a duplicate rather than renaming onto it. + */ + @Test + public void testSetQNameToExistingNameRejected() throws Exception { + UniqueAttributesImpl attrs = createAttributes(); + + try { + attrs.setQName(1, "a"); + Assert.fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + // expected + } + // The rejected rename left the attribute unchanged + Assert.assertEquals("b", attrs.getQName(1)); + } + + + /* + * Renaming an attribute to the name it already has must not unregister + * the name from duplicate detection. + */ + @Test + public void testSetQNameToSameName() throws Exception { + UniqueAttributesImpl attrs = createAttributes(); + + attrs.setQName(0, "a"); + Assert.assertEquals("a", attrs.getQName(0)); + + try { + attrs.addAttribute("", "a", "a", "CDATA", "3"); + Assert.fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + // expected + } + } + + + /* + * Replacing an attribute with a duplicate name must be rejected and must + * keep the tracking set in step with the attributes, so the replaced + * attribute's name is still detected as a duplicate afterwards. + */ + @Test + public void testSetAttributeToExistingNameRejected() throws Exception { + UniqueAttributesImpl attrs = createAttributes(); + + try { + attrs.setAttribute(1, "", "a", "a", "CDATA", "3"); + Assert.fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + // expected + } + // The rejected replacement left the attribute unchanged + Assert.assertEquals("b", attrs.getQName(1)); + + // The unchanged attribute's name must still be tracked + try { + attrs.addAttribute("", "b", "b", "CDATA", "4"); + Assert.fail("Expected IllegalArgumentException"); + } catch (IllegalArgumentException e) { + // expected + } + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
