From ef7d06c53e4fff53c23cd809d826a9bb68818bf6 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 24 Mar 2026 13:03:03 +0100 Subject: [PATCH 1/6] Fix XML write side to properly handle namespace declarations and prefixed attributes (fixes #11760) The consumer POM transformation was producing invalid XML when a POM used namespace-prefixed attributes like mvn:combine.children with xmlns:mvn="http://maven.apache.org/POM/4.0.0". The xmlns:mvn declaration was lost during model transformation, but the prefixed attributes remained, resulting in undeclared namespace prefix errors. Fix the write side (DefaultXmlService.writeNode and generated MavenStaxWriter.writeDom) to: - Write xmlns:prefix entries as proper namespace declarations via writeNamespace() - Write prefixed attributes with their namespace URI via writeAttribute(prefix, nsUri, localName, value) - Strip the prefix from orphaned prefixed attributes (where the xmlns declaration is missing) to produce valid XML Note: mvn:combine.children does NOT trigger merge behavior, and never did (even in Maven 3). Per the XML namespace spec, unprefixed attributes are in no namespace, so combine.children and mvn:combine.children are different attributes. Only the unprefixed form works as a merge directive. Co-Authored-By: Claude Opus 4.6 --- .../maven/internal/xml/DefaultXmlService.java | 40 +++- .../maven/internal/xml/XmlNodeImplTest.java | 194 ++++++++++++++++++ src/mdo/writer-stax.vm | 49 ++++- 3 files changed, 272 insertions(+), 11 deletions(-) diff --git a/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java b/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java index e97a2213805e..be3ca400db1d 100644 --- a/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java +++ b/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java @@ -162,9 +162,7 @@ public void doWrite(XmlNode node, Writer writer) throws IOException { private void writeNode(XMLStreamWriter xmlWriter, XmlNode node) throws XMLStreamException { xmlWriter.writeStartElement(node.prefix(), node.name(), node.namespaceUri()); - for (Map.Entry attr : node.attributes().entrySet()) { - xmlWriter.writeAttribute(attr.getKey(), attr.getValue()); - } + writeAttributes(xmlWriter, node.attributes()); for (XmlNode child : node.children()) { writeNode(xmlWriter, child); @@ -178,6 +176,42 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode node) throws XMLStream xmlWriter.writeEndElement(); } + static void writeAttributes(XMLStreamWriter xmlWriter, Map attributes) throws XMLStreamException { + // Write namespace declarations first, then regular attributes + for (Map.Entry attr : attributes.entrySet()) { + String key = attr.getKey(); + if ("xmlns".equals(key)) { + xmlWriter.writeDefaultNamespace(attr.getValue()); + } else if (key.startsWith("xmlns:")) { + xmlWriter.writeNamespace(key.substring(6), attr.getValue()); + } + } + for (Map.Entry attr : attributes.entrySet()) { + String key = attr.getKey(); + String value = attr.getValue(); + if ("xmlns".equals(key) || key.startsWith("xmlns:")) { + continue; // already written above + } else if (key.startsWith("xml:")) { + xmlWriter.writeAttribute("http://www.w3.org/XML/1998/namespace", key.substring(4), value); + } else if (key.contains(":")) { + int colon = key.indexOf(':'); + String prefix = key.substring(0, colon); + String localName = key.substring(colon + 1); + String nsUri = attributes.get("xmlns:" + prefix); + if (nsUri != null) { + xmlWriter.writeAttribute(prefix, nsUri, localName, value); + } else { + // No namespace declaration found for this prefix; write as unprefixed + // to produce valid XML (the namespace declaration may have been lost + // during consumer POM transformation) + xmlWriter.writeAttribute(localName, value); + } + } else { + xmlWriter.writeAttribute(key, value); + } + } + } + /** * Merges one DOM into another, given a specific algorithm and possible override points for that algorithm.

* The algorithm is as follows: diff --git a/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java b/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java index e9805171948d..c424bc46f7dd 100644 --- a/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java +++ b/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java @@ -36,10 +36,12 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; class XmlNodeImplTest { @@ -715,6 +717,198 @@ public Object toInputLocation(XMLStreamReader parser) { } } + /** + * Verifies that when an XmlNode has xmlns:prefix declarations and prefixed + * attributes, the write side produces valid XML with proper namespace handling. + */ + @Test + void testWriteWithNamespaceDeclarationsAndPrefixedAttributes() throws Exception { + String xml = """ + + + -Xlint:deprecation + + + """; + + XmlNode node = toXmlNode(xml); + + // The xmlns:mvn declaration should be preserved in the parsed node + assertEquals("http://maven.apache.org/POM/4.0.0", node.attribute("xmlns:mvn")); + + // Write and re-read to verify round-trip produces valid XML + java.io.StringWriter writer = new java.io.StringWriter(); + XmlService.write(node, writer); + String output = writer.toString(); + + // The output should be parseable XML (no undeclared namespace prefix errors) + XmlNode reRead = toXmlNode(output); + assertNotNull(reRead); + } + + /** + * Verifies that when a prefixed attribute has no corresponding xmlns declaration + * (as happens in consumer POM transformation), the prefix is stripped on write + * to produce valid XML. + */ + @Test + void testWriteStripsOrphanedPrefixOnAttributes() throws Exception { + // Simulate a consumer POM scenario: mvn:combine.children exists + // but xmlns:mvn was lost during model transformation + XmlNode node = XmlNode.newBuilder() + .name("compilerArgs") + .attributes(java.util.Map.of("mvn:combine.children", "append")) + .children(java.util.List.of(XmlNode.newBuilder() + .name("arg") + .value("-Xlint:deprecation") + .build())) + .build(); + + java.io.StringWriter writer = new java.io.StringWriter(); + XmlService.write(node, writer); + String output = writer.toString(); + + // Should not contain the undeclared mvn: prefix + assertFalse(output.contains("mvn:combine"), "Output should not contain orphaned mvn: prefix"); + // Should contain the unprefixed attribute instead + assertTrue(output.contains("combine.children=\"append\""), "Attribute should be written unprefixed"); + + // The output should be parseable XML + XmlNode reRead = toXmlNode(output); + assertNotNull(reRead); + assertEquals("append", reRead.attribute("combine.children")); + } + + /** + * Verifies that foreign namespace prefixed attributes round-trip correctly + * when the xmlns declaration is on the same element as the prefixed attribute. + */ + @Test + void testWriteForeignNamespaceAttributeRoundTrip() throws Exception { + // Build a node where xmlns:custom and custom:myattr are on the same element + XmlNode node = XmlNode.newBuilder() + .name("compilerArgs") + .attributes(java.util.Map.of( + "xmlns:custom", "http://example.com/custom", + "custom:myattr", "value")) + .children(java.util.List.of(XmlNode.newBuilder() + .name("arg") + .value("-Xlint:deprecation") + .build())) + .build(); + + java.io.StringWriter writer = new java.io.StringWriter(); + XmlService.write(node, writer); + String output = writer.toString(); + + XmlNode reRead = toXmlNode(output); + assertNotNull(reRead); + assertEquals("value", reRead.attribute("custom:myattr")); + assertEquals("http://example.com/custom", reRead.attribute("xmlns:custom")); + } + + /** + * Verifies that when a prefixed attribute's xmlns declaration is on a parent + * element (and thus not in the child's attribute map), the prefix is stripped + * on write to produce valid XML. + */ + @Test + void testWriteStripsOrphanedPrefixFromChildElement() throws Exception { + String xml = """ + + + -Xlint:deprecation + + + """; + + XmlNode node = toXmlNode(xml); + + // The child element has custom:myattr but NOT xmlns:custom in its attributes + XmlNode compilerArgs = node.child("compilerArgs"); + assertNotNull(compilerArgs); + assertEquals("value", compilerArgs.attribute("custom:myattr")); + assertNull(compilerArgs.attribute("xmlns:custom"), "xmlns:custom should be on parent, not child"); + + // Writing the child alone should strip the orphaned prefix + java.io.StringWriter writer = new java.io.StringWriter(); + XmlService.write(compilerArgs, writer); + String output = writer.toString(); + + XmlNode reRead = toXmlNode(output); + assertNotNull(reRead); + // Prefix was stripped, so the attribute is now unprefixed + assertEquals("value", reRead.attribute("myattr")); + } + + /** + * Verifies that mvn:combine.children does NOT trigger merge behavior + * (only unprefixed combine.children works). This is correct per the XML + * namespace spec: unprefixed attributes are in no namespace, while + * prefixed attributes are in their declared namespace. + */ + @Test + void testPrefixedCombineChildrenDoesNotMerge() throws Exception { + String dominant = """ + + + -Xlint:deprecation + + + """; + + String recessive = """ + + + -Xlint:unchecked + + + """; + + XmlNode dominantNode = toXmlNode(dominant); + XmlNode recessiveNode = toXmlNode(recessive); + XmlNode merged = XmlService.merge(dominantNode, recessiveNode); + + // mvn:combine.children should NOT trigger append behavior + // Default merge replaces children by name, so only 1 arg should be present + XmlNode compilerArgs = merged.child("compilerArgs"); + assertNotNull(compilerArgs); + assertEquals( + 1, + compilerArgs.children().size(), + "mvn:combine.children should not trigger append; only unprefixed combine.children works"); + } + + /** + * Verifies that unprefixed combine.children continues to work correctly. + */ + @Test + void testUnprefixedCombineChildrenStillWorks() throws Exception { + String dominant = """ + + + -Xlint:deprecation + + + """; + + String recessive = """ + + + -Xlint:unchecked + + + """; + + XmlNode dominantNode = toXmlNode(dominant); + XmlNode recessiveNode = toXmlNode(recessive); + XmlNode merged = XmlService.merge(dominantNode, recessiveNode); + + XmlNode compilerArgs = merged.child("compilerArgs"); + assertNotNull(compilerArgs); + assertEquals(2, compilerArgs.children().size(), "Unprefixed combine.children=append should work"); + } + public static Xpp3Dom build(Reader reader) throws XmlPullParserException, IOException { try (Reader closeMe = reader) { return new Xpp3Dom(XmlNodeBuilder.build(reader, true, null)); diff --git a/src/mdo/writer-stax.vm b/src/mdo/writer-stax.vm index 9f12f7fc4e51..d0c4ad16af12 100644 --- a/src/mdo/writer-stax.vm +++ b/src/mdo/writer-stax.vm @@ -366,14 +366,7 @@ public class ${className} { private void writeDom(XmlNode dom, XMLStreamWriter serializer) throws IOException, XMLStreamException { if (dom != null) { serializer.writeStartElement(namespace, dom.name()); - for (Map.Entry attr : dom.attributes().entrySet()) { - if (attr.getKey().startsWith("xml:")) { - serializer.writeAttribute("http://www.w3.org/XML/1998/namespace", - attr.getKey().substring(4), attr.getValue()); - } else { - serializer.writeAttribute(attr.getKey(), attr.getValue()); - } - } + writeXmlNodeAttributes(serializer, dom.attributes()); for (XmlNode child : dom.children()) { writeDom(child, serializer); } @@ -410,6 +403,46 @@ public class ${className} { serializer.writeAttribute(attrName, value); } } + + /** + * Writes XmlNode attributes, properly handling namespace declarations + * ({@code xmlns:prefix}) and prefixed attributes ({@code prefix:localName}). + */ + private static void writeXmlNodeAttributes(XMLStreamWriter serializer, Map attributes) throws XMLStreamException { + // Write namespace declarations first + for (Map.Entry attr : attributes.entrySet()) { + String key = attr.getKey(); + if ("xmlns".equals(key)) { + serializer.writeDefaultNamespace(attr.getValue()); + } else if (key.startsWith("xmlns:")) { + serializer.writeNamespace(key.substring(6), attr.getValue()); + } + } + // Then write regular attributes + for (Map.Entry attr : attributes.entrySet()) { + String key = attr.getKey(); + String value = attr.getValue(); + if ("xmlns".equals(key) || key.startsWith("xmlns:")) { + continue; // already written above + } else if (key.startsWith("xml:")) { + serializer.writeAttribute("http://www.w3.org/XML/1998/namespace", key.substring(4), value); + } else if (key.contains(":")) { + int colon = key.indexOf(':'); + String prefix = key.substring(0, colon); + String localName = key.substring(colon + 1); + String nsUri = attributes.get("xmlns:" + prefix); + if (nsUri != null) { + serializer.writeAttribute(prefix, nsUri, localName, value); + } else { + // No namespace declaration for this prefix; write as unprefixed + // to produce valid XML + serializer.writeAttribute(localName, value); + } + } else { + serializer.writeAttribute(key, value); + } + } + } #if ( $locationTracking ) /** From 9bfed154ab40ea8f3d5b661d61cb9a0678fc6602 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 24 Mar 2026 16:01:49 +0100 Subject: [PATCH 2/6] Address review feedback - Make writeAttributes private (not called from generated code) - Rename attr -> attribute in iteration variables - Add comment explaining xml: prefix handling (predefined, must not be declared, but xml:space attributes still need to be written) - Use proper imports instead of FQCNs in tests (StringWriter, Map, List) - Improve test javadoc to explain the consumer POM scenario Co-Authored-By: Claude Opus 4.6 --- .../maven/internal/xml/DefaultXmlService.java | 20 ++++++----- .../maven/internal/xml/XmlNodeImplTest.java | 35 ++++++++++++------- src/mdo/writer-stax.vm | 20 ++++++----- 3 files changed, 46 insertions(+), 29 deletions(-) diff --git a/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java b/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java index be3ca400db1d..434981bca091 100644 --- a/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java +++ b/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java @@ -176,22 +176,26 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode node) throws XMLStream xmlWriter.writeEndElement(); } - static void writeAttributes(XMLStreamWriter xmlWriter, Map attributes) throws XMLStreamException { + private static void writeAttributes(XMLStreamWriter xmlWriter, Map attributes) + throws XMLStreamException { // Write namespace declarations first, then regular attributes - for (Map.Entry attr : attributes.entrySet()) { - String key = attr.getKey(); + for (Map.Entry attribute : attributes.entrySet()) { + String key = attribute.getKey(); if ("xmlns".equals(key)) { - xmlWriter.writeDefaultNamespace(attr.getValue()); + xmlWriter.writeDefaultNamespace(attribute.getValue()); } else if (key.startsWith("xmlns:")) { - xmlWriter.writeNamespace(key.substring(6), attr.getValue()); + xmlWriter.writeNamespace(key.substring(6), attribute.getValue()); } } - for (Map.Entry attr : attributes.entrySet()) { - String key = attr.getKey(); - String value = attr.getValue(); + for (Map.Entry attribute : attributes.entrySet()) { + String key = attribute.getKey(); + String value = attribute.getValue(); if ("xmlns".equals(key) || key.startsWith("xmlns:")) { continue; // already written above } else if (key.startsWith("xml:")) { + // The xml: prefix is predefined and bound to the XML namespace. + // It must not be declared, but attributes like xml:space still need + // to be written using the proper namespace URI. xmlWriter.writeAttribute("http://www.w3.org/XML/1998/namespace", key.substring(4), value); } else if (key.contains(":")) { int colon = key.indexOf(':'); diff --git a/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java b/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java index c424bc46f7dd..293a4db1a4da 100644 --- a/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java +++ b/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.io.Reader; import java.io.StringReader; +import java.io.StringWriter; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -737,7 +738,7 @@ void testWriteWithNamespaceDeclarationsAndPrefixedAttributes() throws Exception assertEquals("http://maven.apache.org/POM/4.0.0", node.attribute("xmlns:mvn")); // Write and re-read to verify round-trip produces valid XML - java.io.StringWriter writer = new java.io.StringWriter(); + StringWriter writer = new StringWriter(); XmlService.write(node, writer); String output = writer.toString(); @@ -757,14 +758,14 @@ void testWriteStripsOrphanedPrefixOnAttributes() throws Exception { // but xmlns:mvn was lost during model transformation XmlNode node = XmlNode.newBuilder() .name("compilerArgs") - .attributes(java.util.Map.of("mvn:combine.children", "append")) - .children(java.util.List.of(XmlNode.newBuilder() + .attributes(Map.of("mvn:combine.children", "append")) + .children(List.of(XmlNode.newBuilder() .name("arg") .value("-Xlint:deprecation") .build())) .build(); - java.io.StringWriter writer = new java.io.StringWriter(); + StringWriter writer = new StringWriter(); XmlService.write(node, writer); String output = writer.toString(); @@ -788,16 +789,16 @@ void testWriteForeignNamespaceAttributeRoundTrip() throws Exception { // Build a node where xmlns:custom and custom:myattr are on the same element XmlNode node = XmlNode.newBuilder() .name("compilerArgs") - .attributes(java.util.Map.of( + .attributes(Map.of( "xmlns:custom", "http://example.com/custom", "custom:myattr", "value")) - .children(java.util.List.of(XmlNode.newBuilder() + .children(List.of(XmlNode.newBuilder() .name("arg") .value("-Xlint:deprecation") .build())) .build(); - java.io.StringWriter writer = new java.io.StringWriter(); + StringWriter writer = new StringWriter(); XmlService.write(node, writer); String output = writer.toString(); @@ -808,9 +809,14 @@ void testWriteForeignNamespaceAttributeRoundTrip() throws Exception { } /** - * Verifies that when a prefixed attribute's xmlns declaration is on a parent - * element (and thus not in the child's attribute map), the prefix is stripped - * on write to produce valid XML. + * This test reproduces the consumer POM bug from issue #11760. + * In the consumer POM transformation, the model is re-serialized from the + * Model object. Namespace declarations like xmlns:custom on the <project> + * element are lost (they are not part of the Maven model), but prefixed + * attributes like custom:myattr survive in plugin configuration XmlNode trees. + * When these orphaned prefixed attributes are written without their namespace + * declaration, writing them as-is would produce invalid XML ("Undeclared + * namespace prefix"). Instead, the prefix is stripped to produce valid XML. */ @Test void testWriteStripsOrphanedPrefixFromChildElement() throws Exception { @@ -824,14 +830,17 @@ void testWriteStripsOrphanedPrefixFromChildElement() throws Exception { XmlNode node = toXmlNode(xml); - // The child element has custom:myattr but NOT xmlns:custom in its attributes + // The child element has custom:myattr but NOT xmlns:custom in its own attributes + // (the namespace declaration is on the parent element) XmlNode compilerArgs = node.child("compilerArgs"); assertNotNull(compilerArgs); assertEquals("value", compilerArgs.attribute("custom:myattr")); assertNull(compilerArgs.attribute("xmlns:custom"), "xmlns:custom should be on parent, not child"); - // Writing the child alone should strip the orphaned prefix - java.io.StringWriter writer = new java.io.StringWriter(); + // Writing the child alone (as happens during consumer POM serialization) + // must produce valid XML. Since xmlns:custom is not available, the prefix + // is stripped to avoid an "Undeclared namespace prefix" error. + StringWriter writer = new StringWriter(); XmlService.write(compilerArgs, writer); String output = writer.toString(); diff --git a/src/mdo/writer-stax.vm b/src/mdo/writer-stax.vm index d0c4ad16af12..5c8cb58137a0 100644 --- a/src/mdo/writer-stax.vm +++ b/src/mdo/writer-stax.vm @@ -410,22 +410,26 @@ public class ${className} { */ private static void writeXmlNodeAttributes(XMLStreamWriter serializer, Map attributes) throws XMLStreamException { // Write namespace declarations first - for (Map.Entry attr : attributes.entrySet()) { - String key = attr.getKey(); + for (Map.Entry attribute : attributes.entrySet()) { + String key = attribute.getKey(); if ("xmlns".equals(key)) { - serializer.writeDefaultNamespace(attr.getValue()); + serializer.writeDefaultNamespace(attribute.getValue()); } else if (key.startsWith("xmlns:")) { - serializer.writeNamespace(key.substring(6), attr.getValue()); + serializer.writeNamespace(key.substring(6), attribute.getValue()); } } // Then write regular attributes - for (Map.Entry attr : attributes.entrySet()) { - String key = attr.getKey(); - String value = attr.getValue(); + for (Map.Entry attribute : attributes.entrySet()) { + String key = attribute.getKey(); + String value = attribute.getValue(); if ("xmlns".equals(key) || key.startsWith("xmlns:")) { continue; // already written above } else if (key.startsWith("xml:")) { - serializer.writeAttribute("http://www.w3.org/XML/1998/namespace", key.substring(4), value); + // The xml: prefix is predefined and bound to the XML namespace. + // It must not be declared, but attributes like xml:space still need + // to be written using the proper namespace URI. + serializer.writeAttribute( + "http://www.w3.org/XML/1998/namespace", key.substring(4), value); } else if (key.contains(":")) { int colon = key.indexOf(':'); String prefix = key.substring(0, colon); From 572ce40d289cd5906e420cb44683c20b535fd988 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 24 Mar 2026 19:37:54 +0100 Subject: [PATCH 3/6] Add namespace context to XmlNode for proper prefix resolution on write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit XmlNode now carries a namespaces() map (prefix → URI) that captures the full namespace context inherited from ancestor elements during parsing. This allows the write side to resolve and auto-declare namespace prefixes for attributes like mvn:combine.children even when the xmlns:mvn declaration was on an ancestor element and not in the local attribute map. - Add namespaces() default method to XmlNode API (returns Map.of()) - Add namespaces field to XmlNode.Builder and Impl record - Update DefaultXmlService.doBuild() to accumulate namespace context - Update writeAttributes() and writeXmlNodeAttributes() (template) to resolve prefixes from namespace context and auto-declare namespaces - Propagate namespaces through merge operations - Add tests for inherited namespace context and orphaned prefix stripping Co-Authored-By: Claude Opus 4.6 --- .../org/apache/maven/api/xml/XmlNode.java | 38 +++++++++++- .../maven/internal/xml/DefaultXmlService.java | 60 ++++++++++++++++--- .../maven/internal/xml/XmlNodeImplTest.java | 55 ++++++++++++----- src/mdo/writer-stax.vm | 26 ++++---- 4 files changed, 144 insertions(+), 35 deletions(-) diff --git a/api/maven-api-xml/src/main/java/org/apache/maven/api/xml/XmlNode.java b/api/maven-api-xml/src/main/java/org/apache/maven/api/xml/XmlNode.java index a78357a09109..a9634585d5d2 100644 --- a/api/maven-api-xml/src/main/java/org/apache/maven/api/xml/XmlNode.java +++ b/api/maven-api-xml/src/main/java/org/apache/maven/api/xml/XmlNode.java @@ -159,6 +159,24 @@ public interface XmlNode { @Nullable String attribute(@Nonnull String name); + /** + * Returns the namespace context for this node — a map of namespace prefix to URI + * for all namespace bindings in scope, including those declared on this element + * and those inherited from ancestor elements. + *

+ * This is used by the write side to properly resolve prefixed attributes. + * For example, if an attribute {@code mvn:combine.children} exists on a child element + * but {@code xmlns:mvn} was declared on the root element, this map will contain + * the {@code mvn → http://maven.apache.org/POM/4.0.0} binding. + * + * @return map of namespace prefix to URI, never {@code null} + * @since 4.1.0 + */ + @Nonnull + default Map namespaces() { + return Map.of(); + } + /** * Returns an immutable list of all child nodes. * @@ -358,6 +376,7 @@ class Builder { private String namespaceUri; private String prefix; private Map attributes; + private Map namespaces; private List children; private Object inputLocation; @@ -421,6 +440,21 @@ public Builder attributes(Map attributes) { return this; } + /** + * Sets the namespace context for this node. + *

+ * This map contains all namespace prefix to URI bindings in scope, + * including inherited ones from ancestor elements. + * + * @param namespaces the map of namespace prefix to URI + * @return this builder instance + * @since 4.1.0 + */ + public Builder namespaces(Map namespaces) { + this.namespaces = namespaces; + return this; + } + /** * Sets the child nodes of the XML node. *

@@ -454,7 +488,7 @@ public Builder inputLocation(Object inputLocation) { * @throws NullPointerException if name has not been set */ public XmlNode build() { - return new Impl(prefix, namespaceUri, name, value, attributes, children, inputLocation); + return new Impl(prefix, namespaceUri, name, value, attributes, namespaces, children, inputLocation); } private record Impl( @@ -463,6 +497,7 @@ private record Impl( @Nonnull String name, String value, @Nonnull Map attributes, + @Nonnull Map namespaces, @Nonnull List children, Object inputLocation) implements XmlNode, Serializable { @@ -473,6 +508,7 @@ private record Impl( namespaceUri = namespaceUri == null ? "" : namespaceUri; name = Objects.requireNonNull(name); attributes = ImmutableCollections.copy(attributes); + namespaces = ImmutableCollections.copy(namespaces); children = ImmutableCollections.copy(children); } diff --git a/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java b/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java index 434981bca091..326671d1a7ed 100644 --- a/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java +++ b/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java @@ -30,6 +30,7 @@ import java.io.Writer; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -68,10 +69,14 @@ public XmlNode doRead(Reader reader, @Nullable XmlService.InputLocationBuilder l @Override public XmlNode doRead(XMLStreamReader parser, @Nullable XmlService.InputLocationBuilder locationBuilder) throws XMLStreamException { - return doBuild(parser, DEFAULT_TRIM, locationBuilder); + return doBuild(parser, DEFAULT_TRIM, locationBuilder, new HashMap<>()); } - private XmlNode doBuild(XMLStreamReader parser, boolean trim, InputLocationBuilder locationBuilder) + private XmlNode doBuild( + XMLStreamReader parser, + boolean trim, + InputLocationBuilder locationBuilder, + Map parentNamespaces) throws XMLStreamException { boolean spacePreserve = false; String lPrefix = null; @@ -80,6 +85,7 @@ private XmlNode doBuild(XMLStreamReader parser, boolean trim, InputLocationBuild String lValue = null; Object location = null; Map attrs = null; + Map nsContext = null; List children = null; int eventType = parser.getEventType(); int lastStartTag = -1; @@ -93,6 +99,15 @@ private XmlNode doBuild(XMLStreamReader parser, boolean trim, InputLocationBuild lNamespaceUri = parser.getNamespaceURI(); lName = parser.getLocalName(); location = locationBuilder != null ? locationBuilder.toInputLocation(parser) : null; + // Build the namespace context: start with inherited, add local declarations + nsContext = new HashMap<>(parentNamespaces); + for (int i = 0; i < namespacesSize; i++) { + String nsPrefix = parser.getNamespacePrefix(i); + String nsUri = parser.getNamespaceURI(i); + if (nsPrefix != null && !nsPrefix.isEmpty()) { + nsContext.put(nsPrefix, nsUri); + } + } int attributesSize = parser.getAttributeCount(); if (attributesSize > 0 || namespacesSize > 0) { attrs = new HashMap<>(); @@ -116,7 +131,7 @@ private XmlNode doBuild(XMLStreamReader parser, boolean trim, InputLocationBuild if (children == null) { children = new ArrayList<>(); } - XmlNode child = doBuild(parser, trim, locationBuilder); + XmlNode child = doBuild(parser, trim, locationBuilder, nsContext != null ? nsContext : Map.of()); children.add(child); } } else if (eventType == XMLStreamReader.CHARACTERS || eventType == XMLStreamReader.CDATA) { @@ -135,6 +150,7 @@ private XmlNode doBuild(XMLStreamReader parser, boolean trim, InputLocationBuild .name(lName) .value(children == null ? (lValue != null ? lValue : emptyTag ? null : "") : null) .attributes(attrs) + .namespaces(nsContext) .children(children) .inputLocation(location) .build(); @@ -162,7 +178,7 @@ public void doWrite(XmlNode node, Writer writer) throws IOException { private void writeNode(XMLStreamWriter xmlWriter, XmlNode node) throws XMLStreamException { xmlWriter.writeStartElement(node.prefix(), node.name(), node.namespaceUri()); - writeAttributes(xmlWriter, node.attributes()); + writeAttributes(xmlWriter, node.attributes(), node.namespaces()); for (XmlNode child : node.children()) { writeNode(xmlWriter, child); @@ -176,17 +192,35 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode node) throws XMLStream xmlWriter.writeEndElement(); } - private static void writeAttributes(XMLStreamWriter xmlWriter, Map attributes) + /** + * Writes XmlNode attributes, properly handling namespace declarations + * ({@code xmlns:prefix}) and prefixed attributes ({@code prefix:localName}). + * The namespace context is used to resolve prefixes when the {@code xmlns:} + * declaration is not present in the attribute map (e.g., it was declared on + * an ancestor element). + * + * @param xmlWriter the StAX writer + * @param attributes the attribute map (may contain xmlns: entries) + * @param namespaces the namespace context (prefix → URI) for resolving prefixed attributes + */ + static void writeAttributes( + XMLStreamWriter xmlWriter, Map attributes, Map namespaces) throws XMLStreamException { - // Write namespace declarations first, then regular attributes + // Collect which namespace prefixes need to be declared on this element: + // start with those explicitly in attributes (xmlns:prefix), then add + // any prefixes used by attributes that are resolved from the namespace context + Set declaredPrefixes = new HashSet<>(); for (Map.Entry attribute : attributes.entrySet()) { String key = attribute.getKey(); if ("xmlns".equals(key)) { xmlWriter.writeDefaultNamespace(attribute.getValue()); } else if (key.startsWith("xmlns:")) { - xmlWriter.writeNamespace(key.substring(6), attribute.getValue()); + String prefix = key.substring(6); + xmlWriter.writeNamespace(prefix, attribute.getValue()); + declaredPrefixes.add(prefix); } } + // Write prefixed attributes, declaring their namespace if needed for (Map.Entry attribute : attributes.entrySet()) { String key = attribute.getKey(); String value = attribute.getValue(); @@ -201,13 +235,20 @@ private static void writeAttributes(XMLStreamWriter xmlWriter, Map @@ -831,23 +826,53 @@ void testWriteStripsOrphanedPrefixFromChildElement() throws Exception { XmlNode node = toXmlNode(xml); // The child element has custom:myattr but NOT xmlns:custom in its own attributes - // (the namespace declaration is on the parent element) XmlNode compilerArgs = node.child("compilerArgs"); assertNotNull(compilerArgs); assertEquals("value", compilerArgs.attribute("custom:myattr")); assertNull(compilerArgs.attribute("xmlns:custom"), "xmlns:custom should be on parent, not child"); + // But the namespace context has the binding from the parent + assertEquals("http://example.com/custom", compilerArgs.namespaces().get("custom")); - // Writing the child alone (as happens during consumer POM serialization) - // must produce valid XML. Since xmlns:custom is not available, the prefix - // is stripped to avoid an "Undeclared namespace prefix" error. + // Writing the child alone should produce valid XML with the namespace declared StringWriter writer = new StringWriter(); XmlService.write(compilerArgs, writer); String output = writer.toString(); XmlNode reRead = toXmlNode(output); assertNotNull(reRead); - // Prefix was stripped, so the attribute is now unprefixed - assertEquals("value", reRead.attribute("myattr")); + // Prefix is preserved because the namespace context provided the URI + assertEquals("value", reRead.attribute("custom:myattr")); + } + + /** + * Verifies that when namespace context is not available (e.g., XmlNode built + * programmatically without namespaces), orphaned prefixes are stripped on write + * to produce valid XML. + */ + @Test + void testWriteStripsOrphanedPrefixWithoutNamespaceContext() throws Exception { + // Build a node with a prefixed attribute but no namespace context at all + XmlNode node = XmlNode.newBuilder() + .name("compilerArgs") + .attributes(Map.of("mvn:combine.children", "append")) + .children(List.of(XmlNode.newBuilder() + .name("arg") + .value("-Xlint:deprecation") + .build())) + .build(); + + assertTrue(node.namespaces().isEmpty(), "No namespace context"); + + StringWriter writer = new StringWriter(); + XmlService.write(node, writer); + String output = writer.toString(); + + assertFalse(output.contains("mvn:combine"), "Output should not contain orphaned mvn: prefix"); + assertTrue(output.contains("combine.children=\"append\""), "Attribute should be written unprefixed"); + + XmlNode reRead = toXmlNode(output); + assertNotNull(reRead); + assertEquals("append", reRead.attribute("combine.children")); } /** diff --git a/src/mdo/writer-stax.vm b/src/mdo/writer-stax.vm index 5c8cb58137a0..fb2aaf7bd3a2 100644 --- a/src/mdo/writer-stax.vm +++ b/src/mdo/writer-stax.vm @@ -366,7 +366,7 @@ public class ${className} { private void writeDom(XmlNode dom, XMLStreamWriter serializer) throws IOException, XMLStreamException { if (dom != null) { serializer.writeStartElement(namespace, dom.name()); - writeXmlNodeAttributes(serializer, dom.attributes()); + writeXmlNodeAttributes(serializer, dom.attributes(), dom.namespaces()); for (XmlNode child : dom.children()) { writeDom(child, serializer); } @@ -407,27 +407,29 @@ public class ${className} { /** * Writes XmlNode attributes, properly handling namespace declarations * ({@code xmlns:prefix}) and prefixed attributes ({@code prefix:localName}). + * The namespace context is used to resolve prefixes when the {@code xmlns:} + * declaration is not present in the attribute map. */ - private static void writeXmlNodeAttributes(XMLStreamWriter serializer, Map attributes) throws XMLStreamException { - // Write namespace declarations first + private static void writeXmlNodeAttributes(XMLStreamWriter serializer, Map attributes, Map namespaces) throws XMLStreamException { + // Collect which namespace prefixes need to be declared on this element + Set declaredPrefixes = new HashSet<>(); for (Map.Entry attribute : attributes.entrySet()) { String key = attribute.getKey(); if ("xmlns".equals(key)) { serializer.writeDefaultNamespace(attribute.getValue()); } else if (key.startsWith("xmlns:")) { - serializer.writeNamespace(key.substring(6), attribute.getValue()); + String prefix = key.substring(6); + serializer.writeNamespace(prefix, attribute.getValue()); + declaredPrefixes.add(prefix); } } - // Then write regular attributes + // Write prefixed attributes, declaring their namespace if needed for (Map.Entry attribute : attributes.entrySet()) { String key = attribute.getKey(); String value = attribute.getValue(); if ("xmlns".equals(key) || key.startsWith("xmlns:")) { continue; // already written above } else if (key.startsWith("xml:")) { - // The xml: prefix is predefined and bound to the XML namespace. - // It must not be declared, but attributes like xml:space still need - // to be written using the proper namespace URI. serializer.writeAttribute( "http://www.w3.org/XML/1998/namespace", key.substring(4), value); } else if (key.contains(":")) { @@ -435,11 +437,15 @@ public class ${className} { String prefix = key.substring(0, colon); String localName = key.substring(colon + 1); String nsUri = attributes.get("xmlns:" + prefix); + if (nsUri == null) { + nsUri = namespaces.get(prefix); + } if (nsUri != null) { + if (declaredPrefixes.add(prefix)) { + serializer.writeNamespace(prefix, nsUri); + } serializer.writeAttribute(prefix, nsUri, localName, value); } else { - // No namespace declaration for this prefix; write as unprefixed - // to produce valid XML serializer.writeAttribute(localName, value); } } else { From fdc8cbddc36cfe7c58d83ea9e79b088cd228fabf Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 24 Mar 2026 22:35:01 +0100 Subject: [PATCH 4/6] Add comprehensive tests for XmlNode namespace context support Add 28 tests covering all aspects of the namespaces() feature: - Parsing: single/multiple prefixes, inheritance across levels, prefix shadowing, default namespace exclusion, immutability - Writing: round-trip, orphaned prefix stripping, multiple prefixes, local xmlns precedence, xml:space, no duplicate declarations - Merging: namespace propagation, combine.children/self preservation - Builder: explicit namespaces, null defaults, immutability - Round-trip fidelity: deep nesting, overridden namespaces - Consumer POM simulation: context-based and fallback scenarios - Merge directive interaction: prefixed directives ignored Co-Authored-By: Claude Opus 4.6 --- .../maven/internal/xml/XmlNodeImplTest.java | 671 ++++++++++++++++-- 1 file changed, 621 insertions(+), 50 deletions(-) diff --git a/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java b/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java index bf1596b86b92..4a50e0e6cab2 100644 --- a/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java +++ b/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java @@ -42,6 +42,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; class XmlNodeImplTest { @@ -718,10 +719,149 @@ public Object toInputLocation(XMLStreamReader parser) { } } - /** - * Verifies that when an XmlNode has xmlns:prefix declarations and prefixed - * attributes, the write side produces valid XML with proper namespace handling. - */ + // ======================================================================================== + // Namespace context - Parsing tests + // ======================================================================================== + + @Test + void testParseNamespaceContextSinglePrefixOnRoot() throws Exception { + String xml = """ + + + + """; + XmlNode node = toXmlNode(xml); + assertEquals("http://maven.apache.org/POM/4.0.0", node.namespaces().get("mvn")); + } + + @Test + void testParseNamespaceContextMultiplePrefixes() throws Exception { + String xml = """ + + + + """; + XmlNode node = toXmlNode(xml); + assertEquals(3, node.namespaces().size()); + assertEquals("http://maven.apache.org/POM/4.0.0", node.namespaces().get("mvn")); + assertEquals("http://example.com/custom", node.namespaces().get("custom")); + assertEquals("http://example.com/other", node.namespaces().get("other")); + } + + @Test + void testParseNamespaceContextInheritedByChild() throws Exception { + String xml = """ + + + + """; + XmlNode node = toXmlNode(xml); + XmlNode child = node.child("child"); + assertNotNull(child); + // Child inherits parent's namespace context + assertEquals("http://maven.apache.org/POM/4.0.0", child.namespaces().get("mvn")); + // Child does NOT have xmlns:mvn in its own attributes + assertNull(child.attribute("xmlns:mvn")); + } + + @Test + void testParseNamespaceContextInheritedAcrossThreeLevels() throws Exception { + String xml = """ + + + + + + + + """; + XmlNode root = toXmlNode(xml); + XmlNode level1 = root.child("level1"); + XmlNode level2 = level1.child("level2"); + XmlNode leaf = level2.child("leaf"); + + // root has only "a" + assertEquals("http://example.com/a", root.namespaces().get("a")); + assertNull(root.namespaces().get("b")); + + // level1 has both "a" (inherited) and "b" (own) + assertEquals("http://example.com/a", level1.namespaces().get("a")); + assertEquals("http://example.com/b", level1.namespaces().get("b")); + + // level2 inherits both + assertEquals("http://example.com/a", level2.namespaces().get("a")); + assertEquals("http://example.com/b", level2.namespaces().get("b")); + + // leaf also inherits both + assertEquals("http://example.com/a", leaf.namespaces().get("a")); + assertEquals("http://example.com/b", leaf.namespaces().get("b")); + } + + @Test + void testParseDefaultNamespaceNotInNamespacesMap() throws Exception { + String xml = """ + + + + """; + XmlNode node = toXmlNode(xml); + // Default namespace (no prefix) should NOT be in the namespaces map + // since namespaces() tracks prefix→URI bindings for resolving prefixed attributes + assertNull(node.namespaces().get("")); + assertNull(node.namespaces().get("xmlns")); + // The default namespace is stored as an attribute instead + assertEquals("http://maven.apache.org/POM/4.0.0", node.attribute("xmlns")); + } + + @Test + void testParseNamespaceContextChildOverridesPrefix() throws Exception { + String xml = """ + + + + + + """; + XmlNode root = toXmlNode(xml); + XmlNode child = root.child("child"); + XmlNode grandchild = child.child("grandchild"); + + // Root has original binding + assertEquals("http://example.com/original", root.namespaces().get("ns")); + // Child overrides + assertEquals("http://example.com/overridden", child.namespaces().get("ns")); + // Grandchild inherits the overridden version + assertEquals("http://example.com/overridden", grandchild.namespaces().get("ns")); + } + + @Test + void testParseNoNamespaceDeclarationsProducesEmptyMap() throws Exception { + String xml = ""; + XmlNode root = toXmlNode(xml); + assertTrue(root.namespaces().isEmpty()); + XmlNode child = root.child("child"); + assertNotNull(child); + assertTrue(child.namespaces().isEmpty()); + } + + @Test + void testParseNamespacesMapIsImmutable() throws Exception { + String xml = """ + + + + """; + XmlNode node = toXmlNode(xml); + assertThrows( + UnsupportedOperationException.class, () -> node.namespaces().put("foo", "bar")); + } + + // ======================================================================================== + // Namespace context - Writing tests + // ======================================================================================== + @Test void testWriteWithNamespaceDeclarationsAndPrefixedAttributes() throws Exception { String xml = """ @@ -733,29 +873,18 @@ void testWriteWithNamespaceDeclarationsAndPrefixedAttributes() throws Exception """; XmlNode node = toXmlNode(xml); - - // The xmlns:mvn declaration should be preserved in the parsed node assertEquals("http://maven.apache.org/POM/4.0.0", node.attribute("xmlns:mvn")); - // Write and re-read to verify round-trip produces valid XML StringWriter writer = new StringWriter(); XmlService.write(node, writer); String output = writer.toString(); - // The output should be parseable XML (no undeclared namespace prefix errors) XmlNode reRead = toXmlNode(output); assertNotNull(reRead); } - /** - * Verifies that when a prefixed attribute has no corresponding xmlns declaration - * (as happens in consumer POM transformation), the prefix is stripped on write - * to produce valid XML. - */ @Test void testWriteStripsOrphanedPrefixOnAttributes() throws Exception { - // Simulate a consumer POM scenario: mvn:combine.children exists - // but xmlns:mvn was lost during model transformation XmlNode node = XmlNode.newBuilder() .name("compilerArgs") .attributes(Map.of("mvn:combine.children", "append")) @@ -769,24 +898,16 @@ void testWriteStripsOrphanedPrefixOnAttributes() throws Exception { XmlService.write(node, writer); String output = writer.toString(); - // Should not contain the undeclared mvn: prefix assertFalse(output.contains("mvn:combine"), "Output should not contain orphaned mvn: prefix"); - // Should contain the unprefixed attribute instead assertTrue(output.contains("combine.children=\"append\""), "Attribute should be written unprefixed"); - // The output should be parseable XML XmlNode reRead = toXmlNode(output); assertNotNull(reRead); assertEquals("append", reRead.attribute("combine.children")); } - /** - * Verifies that foreign namespace prefixed attributes round-trip correctly - * when the xmlns declaration is on the same element as the prefixed attribute. - */ @Test void testWriteForeignNamespaceAttributeRoundTrip() throws Exception { - // Build a node where xmlns:custom and custom:myattr are on the same element XmlNode node = XmlNode.newBuilder() .name("compilerArgs") .attributes(Map.of( @@ -808,11 +929,6 @@ void testWriteForeignNamespaceAttributeRoundTrip() throws Exception { assertEquals("http://example.com/custom", reRead.attribute("xmlns:custom")); } - /** - * Verifies that when a prefixed attribute's xmlns declaration is on a parent - * element, the child node's namespace context (inherited from parsing) allows - * the prefix to be preserved when writing the child alone. - */ @Test void testWritePreservesPrefixFromInheritedNamespaceContext() throws Exception { String xml = """ @@ -824,34 +940,23 @@ void testWritePreservesPrefixFromInheritedNamespaceContext() throws Exception { """; XmlNode node = toXmlNode(xml); - - // The child element has custom:myattr but NOT xmlns:custom in its own attributes XmlNode compilerArgs = node.child("compilerArgs"); assertNotNull(compilerArgs); assertEquals("value", compilerArgs.attribute("custom:myattr")); assertNull(compilerArgs.attribute("xmlns:custom"), "xmlns:custom should be on parent, not child"); - // But the namespace context has the binding from the parent assertEquals("http://example.com/custom", compilerArgs.namespaces().get("custom")); - // Writing the child alone should produce valid XML with the namespace declared StringWriter writer = new StringWriter(); XmlService.write(compilerArgs, writer); String output = writer.toString(); XmlNode reRead = toXmlNode(output); assertNotNull(reRead); - // Prefix is preserved because the namespace context provided the URI assertEquals("value", reRead.attribute("custom:myattr")); } - /** - * Verifies that when namespace context is not available (e.g., XmlNode built - * programmatically without namespaces), orphaned prefixes are stripped on write - * to produce valid XML. - */ @Test void testWriteStripsOrphanedPrefixWithoutNamespaceContext() throws Exception { - // Build a node with a prefixed attribute but no namespace context at all XmlNode node = XmlNode.newBuilder() .name("compilerArgs") .attributes(Map.of("mvn:combine.children", "append")) @@ -875,12 +980,456 @@ void testWriteStripsOrphanedPrefixWithoutNamespaceContext() throws Exception { assertEquals("append", reRead.attribute("combine.children")); } - /** - * Verifies that mvn:combine.children does NOT trigger merge behavior - * (only unprefixed combine.children works). This is correct per the XML - * namespace spec: unprefixed attributes are in no namespace, while - * prefixed attributes are in their declared namespace. - */ + @Test + void testWriteMultiplePrefixedAttributesFromDifferentNamespaces() throws Exception { + String xml = """ + + + + """; + XmlNode root = toXmlNode(xml); + XmlNode child = root.child("child"); + assertNotNull(child); + + // Write only the child (which has prefixed attrs but no local xmlns:) + StringWriter writer = new StringWriter(); + XmlService.write(child, writer); + String output = writer.toString(); + + // Both namespace declarations should be auto-declared + assertTrue(output.contains("xmlns:a="), "Should auto-declare xmlns:a"); + assertTrue(output.contains("xmlns:b="), "Should auto-declare xmlns:b"); + + // Round-trip should preserve attributes + XmlNode reRead = toXmlNode(output); + assertEquals("1", reRead.attribute("a:x")); + assertEquals("2", reRead.attribute("b:y")); + } + + @Test + void testWriteLocalXmlnsOverridesNamespaceContext() throws Exception { + // Build a node where the local attribute has xmlns:ns with one URI + // but the namespace context has a different URI for the same prefix. + // The local declaration should win. + XmlNode node = XmlNode.newBuilder() + .name("elem") + .attributes(Map.of( + "xmlns:ns", "http://example.com/local", + "ns:attr", "value")) + .namespaces(Map.of("ns", "http://example.com/context")) + .build(); + + StringWriter writer = new StringWriter(); + XmlService.write(node, writer); + String output = writer.toString(); + + // The local xmlns:ns should be used, not the one from context + assertTrue(output.contains("http://example.com/local"), "Local xmlns: should take precedence"); + + XmlNode reRead = toXmlNode(output); + assertEquals("value", reRead.attribute("ns:attr")); + assertEquals("http://example.com/local", reRead.attribute("xmlns:ns")); + } + + @Test + void testWriteXmlSpaceAttributeRoundTrip() throws Exception { + String xml = """ + content with spaces + """; + XmlNode node = toXmlNode(xml); + assertEquals("preserve", node.attribute("xml:space")); + + StringWriter writer = new StringWriter(); + XmlService.write(node, writer); + String output = writer.toString(); + + // xml: prefix should be handled without explicit declaration + assertFalse(output.contains("xmlns:xml"), "xml: prefix must not be declared"); + XmlNode reRead = toXmlNode(output); + assertEquals("preserve", reRead.attribute("xml:space")); + assertEquals(" content with spaces ", reRead.value()); + } + + @Test + void testWriteUnprefixedAttributeUnchanged() throws Exception { + XmlNode node = XmlNode.newBuilder() + .name("elem") + .attributes(Map.of("simple", "value", "another", "val2")) + .build(); + + StringWriter writer = new StringWriter(); + XmlService.write(node, writer); + String output = writer.toString(); + + XmlNode reRead = toXmlNode(output); + assertEquals("value", reRead.attribute("simple")); + assertEquals("val2", reRead.attribute("another")); + } + + @Test + void testWriteNamespaceNotDeclaredTwice() throws Exception { + // When xmlns:mvn is both in attributes AND namespace context, + // it should only be declared once + XmlNode node = XmlNode.newBuilder() + .name("elem") + .attributes(Map.of( + "xmlns:mvn", "http://maven.apache.org/POM/4.0.0", + "mvn:combine.children", "append")) + .namespaces(Map.of("mvn", "http://maven.apache.org/POM/4.0.0")) + .build(); + + StringWriter writer = new StringWriter(); + XmlService.write(node, writer); + String output = writer.toString(); + + // Count occurrences of xmlns:mvn - should be exactly 1 + int count = 0; + int idx = 0; + while ((idx = output.indexOf("xmlns:mvn", idx)) != -1) { + count++; + idx += "xmlns:mvn".length(); + } + assertEquals(1, count, "xmlns:mvn should be declared exactly once"); + + XmlNode reRead = toXmlNode(output); + assertEquals("append", reRead.attribute("mvn:combine.children")); + } + + @Test + void testWriteChildInheritsContextAndWritesStandalone() throws Exception { + // Parse a 3-level structure, then write the grandchild standalone + String xml = """ + + + + + + """; + XmlNode root = toXmlNode(xml); + XmlNode leaf = root.child("mid").child("leaf"); + + StringWriter writer = new StringWriter(); + XmlService.write(leaf, writer); + String output = writer.toString(); + + XmlNode reRead = toXmlNode(output); + assertEquals("1", reRead.attribute("a:x")); + assertEquals("2", reRead.attribute("b:y")); + assertEquals("3", reRead.attribute("plain")); + } + + // ======================================================================================== + // Namespace context - Merge tests + // ======================================================================================== + + @Test + void testMergePreservesDominantNamespaces() throws Exception { + String dominant = """ + + + dom + + + """; + String recessive = """ + + + rec + + + """; + XmlNode merged = XmlService.merge(toXmlNode(dominant), toXmlNode(recessive)); + + // The merged root should keep dominant's namespace context + assertEquals("http://maven.apache.org/POM/4.0.0", merged.namespaces().get("mvn")); + + // The merged child should also have the namespace context + XmlNode child = merged.child("child"); + assertNotNull(child); + assertEquals("http://maven.apache.org/POM/4.0.0", child.namespaces().get("mvn")); + } + + @Test + void testMergeCombineChildrenAppendPreservesNamespaces() throws Exception { + String dominant = """ + + + a + + + """; + String recessive = """ + + + b + + + """; + XmlNode merged = XmlService.merge(toXmlNode(dominant), toXmlNode(recessive)); + XmlNode items = merged.child("items"); + + assertEquals(2, items.children().size(), "append should merge children"); + // Namespace context should be preserved on the merged element + assertEquals("http://maven.apache.org/POM/4.0.0", items.namespaces().get("mvn")); + } + + @Test + void testMergeCombineSelfOverridePreservesNamespaces() throws Exception { + String dominant = """ + + + dom + + + """; + String recessive = """ + + + rec1 + rec2 + + + """; + XmlNode merged = XmlService.merge(toXmlNode(dominant), toXmlNode(recessive)); + XmlNode child = merged.child("child"); + + // override means dominant completely replaces recessive + assertEquals(1, child.children().size()); + assertEquals("dom", child.children().get(0).value()); + // Namespace context preserved + assertEquals("http://example.com/ns", child.namespaces().get("ns")); + } + + @Test + void testMergedNodeWriteProducesValidXml() throws Exception { + String dominant = """ + + + a + + + """; + String recessive = """ + + + b + + + """; + XmlNode merged = XmlService.merge(toXmlNode(dominant), toXmlNode(recessive)); + + // Write the merged child alone - it should produce valid XML + // because it has the namespace context from the dominant + XmlNode child = merged.child("child"); + StringWriter writer = new StringWriter(); + XmlService.write(child, writer); + String output = writer.toString(); + + // mvn:combine.children should be preserved with namespace declaration + assertTrue(output.contains("mvn:combine.children"), "Prefix should be preserved from context"); + assertTrue(output.contains("xmlns:mvn="), "Namespace should be auto-declared"); + + XmlNode reRead = toXmlNode(output); + assertEquals("append", reRead.attribute("mvn:combine.children")); + } + + // ======================================================================================== + // Namespace context - Builder tests + // ======================================================================================== + + @Test + void testBuilderWithExplicitNamespaces() throws Exception { + XmlNode node = XmlNode.newBuilder() + .name("elem") + .attributes(Map.of("ns:attr", "value")) + .namespaces(Map.of("ns", "http://example.com/ns")) + .build(); + + assertEquals("http://example.com/ns", node.namespaces().get("ns")); + + StringWriter writer = new StringWriter(); + XmlService.write(node, writer); + String output = writer.toString(); + + assertTrue(output.contains("xmlns:ns="), "Namespace should be auto-declared from builder context"); + XmlNode reRead = toXmlNode(output); + assertEquals("value", reRead.attribute("ns:attr")); + } + + @Test + void testBuilderWithNullNamespacesDefaultsToEmpty() { + XmlNode node = XmlNode.newBuilder().name("elem").build(); + assertNotNull(node.namespaces()); + assertTrue(node.namespaces().isEmpty()); + } + + @Test + void testBuilderNamespacesAreImmutable() { + Map mutableNs = new HashMap<>(Map.of("ns", "http://example.com")); + XmlNode node = XmlNode.newBuilder().name("elem").namespaces(mutableNs).build(); + + // Mutating the original map should not affect the node + mutableNs.put("other", "http://other.com"); + assertNull(node.namespaces().get("other")); + + // The namespaces map itself should be immutable + assertThrows( + UnsupportedOperationException.class, () -> node.namespaces().put("foo", "bar")); + } + + @Test + void testDefaultNamespacesMethodReturnsEmptyMap() { + // XmlNode built with newInstance (which doesn't set namespaces) + // should return empty map from the default namespaces() method + XmlNode node = XmlNode.newInstance("test"); + assertNotNull(node.namespaces()); + assertTrue(node.namespaces().isEmpty()); + } + + // ======================================================================================== + // Namespace context - Round-trip fidelity tests + // ======================================================================================== + + @Test + void testRoundTripPreservesNamespaceContext() throws Exception { + String xml = """ + + + + """; + XmlNode original = toXmlNode(xml); + + StringWriter writer = new StringWriter(); + XmlService.write(original, writer); + XmlNode reRead = toXmlNode(writer.toString()); + + // Root namespace context should be preserved + assertEquals(original.namespaces().get("a"), reRead.namespaces().get("a")); + assertEquals(original.namespaces().get("b"), reRead.namespaces().get("b")); + + // Child namespace context should be preserved + XmlNode origChild = original.child("child"); + XmlNode reReadChild = reRead.child("child"); + assertEquals(origChild.namespaces().get("a"), reReadChild.namespaces().get("a")); + assertEquals(origChild.namespaces().get("b"), reReadChild.namespaces().get("b")); + } + + @Test + void testRoundTripDeepNestedStructure() throws Exception { + String xml = """ + + + + text + + + + """; + XmlNode original = toXmlNode(xml); + + StringWriter writer = new StringWriter(); + XmlService.write(original, writer); + XmlNode reRead = toXmlNode(writer.toString()); + + XmlNode level3 = reRead.child("level1").child("level2").child("level3"); + assertEquals("value", level3.attribute("ns:deep")); + assertEquals("text", level3.value()); + assertEquals("http://example.com/ns", level3.namespaces().get("ns")); + } + + @Test + void testRoundTripWithOverriddenNamespace() throws Exception { + String xml = """ + + + + """; + XmlNode original = toXmlNode(xml); + XmlNode child = original.child("child"); + assertEquals("http://example.com/v2", child.namespaces().get("ns")); + + // Write and re-read just the child + StringWriter writer = new StringWriter(); + XmlService.write(child, writer); + XmlNode reRead = toXmlNode(writer.toString()); + + assertEquals("val", reRead.attribute("ns:attr")); + assertEquals("http://example.com/v2", reRead.namespaces().get("ns")); + } + + // ======================================================================================== + // Namespace context - Consumer POM simulation tests + // ======================================================================================== + + @Test + void testConsumerPomScenarioPrefixFromContext() throws Exception { + // Simulate: parse a full POM with xmlns:mvn on project, mvn:combine.children on child + String xml = """ + + + + + + + -Xlint + + + + + + + """; + XmlNode project = toXmlNode(xml); + XmlNode compilerArgs = project.child("build") + .child("plugins") + .child("plugin") + .child("configuration") + .child("compilerArgs"); + assertNotNull(compilerArgs); + assertEquals("append", compilerArgs.attribute("mvn:combine.children")); + assertEquals( + "http://maven.apache.org/POM/4.0.0", compilerArgs.namespaces().get("mvn")); + + // Simulate consumer POM: write only the configuration subtree + XmlNode config = project.child("build").child("plugins").child("plugin").child("configuration"); + StringWriter writer = new StringWriter(); + XmlService.write(config, writer); + String output = writer.toString(); + + // Should produce valid XML with auto-declared xmlns:mvn + XmlNode reRead = toXmlNode(output); + XmlNode reReadArgs = reRead.child("compilerArgs"); + assertEquals("append", reReadArgs.attribute("mvn:combine.children")); + } + + @Test + void testConsumerPomScenarioNoContextFallback() throws Exception { + // Simulate: programmatically-built XmlNode without namespace context + // (as might happen if someone builds configuration in code) + XmlNode config = XmlNode.newBuilder() + .name("configuration") + .children(List.of(XmlNode.newBuilder() + .name("compilerArgs") + .attributes(Map.of("mvn:combine.children", "append")) + .children(List.of( + XmlNode.newBuilder().name("arg").value("-Xlint").build())) + .build())) + .build(); + + StringWriter writer = new StringWriter(); + XmlService.write(config, writer); + String output = writer.toString(); + + // Without namespace context, prefix should be stripped + assertFalse(output.contains("mvn:"), "No mvn: prefix without context"); + XmlNode reRead = toXmlNode(output); + assertEquals("append", reRead.child("compilerArgs").attribute("combine.children")); + } + + // ======================================================================================== + // Namespace context - Merge directive interaction tests + // ======================================================================================== + @Test void testPrefixedCombineChildrenDoesNotMerge() throws Exception { String dominant = """ @@ -903,8 +1452,6 @@ void testPrefixedCombineChildrenDoesNotMerge() throws Exception { XmlNode recessiveNode = toXmlNode(recessive); XmlNode merged = XmlService.merge(dominantNode, recessiveNode); - // mvn:combine.children should NOT trigger append behavior - // Default merge replaces children by name, so only 1 arg should be present XmlNode compilerArgs = merged.child("compilerArgs"); assertNotNull(compilerArgs); assertEquals( @@ -913,9 +1460,6 @@ void testPrefixedCombineChildrenDoesNotMerge() throws Exception { "mvn:combine.children should not trigger append; only unprefixed combine.children works"); } - /** - * Verifies that unprefixed combine.children continues to work correctly. - */ @Test void testUnprefixedCombineChildrenStillWorks() throws Exception { String dominant = """ @@ -943,6 +1487,33 @@ void testUnprefixedCombineChildrenStillWorks() throws Exception { assertEquals(2, compilerArgs.children().size(), "Unprefixed combine.children=append should work"); } + @Test + void testPrefixedCombineSelfDoesNotOverride() throws Exception { + String dominant = """ + + + dom + + + """; + String recessive = """ + + + rec + bonus + + + """; + XmlNode merged = XmlService.merge(toXmlNode(dominant), toXmlNode(recessive)); + XmlNode child = merged.child("child"); + + // mvn:combine.self should NOT trigger override (only unprefixed combine.self works) + // Default merge behavior merges children by name + assertEquals("dom", child.child("item").value()); + // The "extra" child from recessive should survive since combine.self wasn't triggered + assertNotNull(child.child("extra"), "Recessive children should survive since mvn:combine.self is ignored"); + } + public static Xpp3Dom build(Reader reader) throws XmlPullParserException, IOException { try (Reader closeMe = reader) { return new Xpp3Dom(XmlNodeBuilder.build(reader, true, null)); From 322cffdfb037a225dfb193532cd959ccc8aafdbe Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 24 Mar 2026 22:59:28 +0100 Subject: [PATCH 5/6] Fix XPP3 writer template to handle namespace declarations and prefixed attributes Apply the same namespace handling to writer.vm (deprecated XPP3 writer) as was done for writer-stax.vm: xmlns: entries are written as namespace declarations via setPrefix(), prefixed attributes are resolved from the namespace context, and orphaned prefixes are stripped to produce valid XML. Co-Authored-By: Claude Opus 4.6 --- src/mdo/writer.vm | 54 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/src/mdo/writer.vm b/src/mdo/writer.vm index 3795700a2ddf..6a63c6e3d4ef 100644 --- a/src/mdo/writer.vm +++ b/src/mdo/writer.vm @@ -252,9 +252,7 @@ public class ${className} { private void writeDom(XmlNode dom, XmlSerializer serializer) throws IOException { if (dom != null) { serializer.startTag(NAMESPACE, dom.getName()); - for (Map.Entry attr : dom.getAttributes().entrySet()) { - serializer.attribute(NAMESPACE, attr.getKey(), attr.getValue()); - } + writeXmlNodeAttributes(serializer, dom.getAttributes(), dom.namespaces()); for (XmlNode child : dom.getChildren()) { writeDom(child, serializer); } @@ -266,6 +264,56 @@ public class ${className} { } } + /** + * Writes XmlNode attributes, properly handling namespace declarations + * ({@code xmlns:prefix}) and prefixed attributes ({@code prefix:localName}). + * The namespace context is used to resolve prefixes when the {@code xmlns:} + * declaration is not present in the attribute map. + */ + private static void writeXmlNodeAttributes(XmlSerializer serializer, Map attributes, Map namespaces) throws IOException { + // Collect which namespace prefixes need to be declared on this element + Set declaredPrefixes = new HashSet<>(); + for (Map.Entry attribute : attributes.entrySet()) { + String key = attribute.getKey(); + if ("xmlns".equals(key)) { + serializer.setPrefix("", attribute.getValue()); + } else if (key.startsWith("xmlns:")) { + String prefix = key.substring(6); + serializer.setPrefix(prefix, attribute.getValue()); + declaredPrefixes.add(prefix); + } + } + // Write prefixed attributes, declaring their namespace if needed + for (Map.Entry attribute : attributes.entrySet()) { + String key = attribute.getKey(); + String value = attribute.getValue(); + if ("xmlns".equals(key) || key.startsWith("xmlns:")) { + continue; // already handled above + } else if (key.startsWith("xml:")) { + serializer.attribute("http://www.w3.org/XML/1998/namespace", key.substring(4), value); + } else if (key.contains(":")) { + int colon = key.indexOf(':'); + String prefix = key.substring(0, colon); + String localName = key.substring(colon + 1); + String nsUri = attributes.get("xmlns:" + prefix); + if (nsUri == null) { + nsUri = namespaces.get(prefix); + } + if (nsUri != null) { + if (declaredPrefixes.add(prefix)) { + serializer.setPrefix(prefix, nsUri); + } + serializer.attribute(nsUri, localName, value); + } else { + // No namespace declaration for this prefix; write as unprefixed + serializer.attribute(NAMESPACE, localName, value); + } + } else { + serializer.attribute(NAMESPACE, key, value); + } + } + } + private void writeTag(String tagName, String defaultValue, String value, XmlSerializer serializer) throws IOException { if (value != null && !Objects.equals(defaultValue, value)) { serializer.startTag(NAMESPACE, tagName).text(value).endTag(NAMESPACE, tagName); From 734360191e8fa2e0d3074dfec11c7541bb9c9b9a Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Wed, 25 Mar 2026 07:17:32 +0100 Subject: [PATCH 6/6] Address review feedback: rename variables and add clarifying comments - Rename l-prefixed variables (lPrefix, lName, etc.) to descriptive names (elementPrefix, elementName, etc.) - Add comment explaining why elementName == null distinguishes the current element's START_ELEMENT from child START_ELEMENTs - Add comment explaining why default namespace is excluded from the namespace context (XML spec Section 6.2) - Combine duplicate namespace iteration into a single loop - Remove unnecessary null check on nsContext (always non-null) - Make writeAttributes private (template has its own copy) - Rename aName/aValue/aPrefix to attrName/attrValue/attrPrefix Co-Authored-By: Claude Opus 4.6 --- .../maven/internal/xml/DefaultXmlService.java | 69 ++++++++++--------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java b/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java index 326671d1a7ed..4b0bb6fbf063 100644 --- a/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java +++ b/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java @@ -79,10 +79,10 @@ private XmlNode doBuild( Map parentNamespaces) throws XMLStreamException { boolean spacePreserve = false; - String lPrefix = null; - String lNamespaceUri = null; - String lName = null; - String lValue = null; + String elementPrefix = null; + String elementNamespaceUri = null; + String elementName = null; + String elementValue = null; Object location = null; Map attrs = null; Map nsContext = null; @@ -93,62 +93,65 @@ private XmlNode doBuild( if (eventType == XMLStreamReader.START_ELEMENT) { lastStartTag = parser.getLocation().getLineNumber() * 1000 + parser.getLocation().getColumnNumber(); - if (lName == null) { + // The first START_ELEMENT we encounter is "this" element; + // subsequent START_ELEMENTs are children, handled in the else branch. + if (elementName == null) { int namespacesSize = parser.getNamespaceCount(); - lPrefix = parser.getPrefix(); - lNamespaceUri = parser.getNamespaceURI(); - lName = parser.getLocalName(); + elementPrefix = parser.getPrefix(); + elementNamespaceUri = parser.getNamespaceURI(); + elementName = parser.getLocalName(); location = locationBuilder != null ? locationBuilder.toInputLocation(parser) : null; - // Build the namespace context: start with inherited, add local declarations + // Build the namespace context: start with inherited, add local declarations. + // The default namespace (empty prefix) is excluded because per the XML namespace + // spec (Section 6.2), default namespace declarations do NOT apply to attributes. nsContext = new HashMap<>(parentNamespaces); - for (int i = 0; i < namespacesSize; i++) { - String nsPrefix = parser.getNamespacePrefix(i); - String nsUri = parser.getNamespaceURI(i); - if (nsPrefix != null && !nsPrefix.isEmpty()) { - nsContext.put(nsPrefix, nsUri); - } - } int attributesSize = parser.getAttributeCount(); if (attributesSize > 0 || namespacesSize > 0) { attrs = new HashMap<>(); for (int i = 0; i < namespacesSize; i++) { String nsPrefix = parser.getNamespacePrefix(i); String nsUri = parser.getNamespaceURI(i); - attrs.put(nsPrefix != null && !nsPrefix.isEmpty() ? "xmlns:" + nsPrefix : "xmlns", nsUri); + if (nsPrefix != null && !nsPrefix.isEmpty()) { + nsContext.put(nsPrefix, nsUri); + attrs.put("xmlns:" + nsPrefix, nsUri); + } else { + attrs.put("xmlns", nsUri); + } } for (int i = 0; i < attributesSize; i++) { - String aName = parser.getAttributeLocalName(i); - String aValue = parser.getAttributeValue(i); - String aPrefix = parser.getAttributePrefix(i); - if (aPrefix != null && !aPrefix.isEmpty()) { - aName = aPrefix + ":" + aName; + String attrName = parser.getAttributeLocalName(i); + String attrValue = parser.getAttributeValue(i); + String attrPrefix = parser.getAttributePrefix(i); + if (attrPrefix != null && !attrPrefix.isEmpty()) { + attrName = attrPrefix + ":" + attrName; } - attrs.put(aName, aValue); - spacePreserve = spacePreserve || ("xml:space".equals(aName) && "preserve".equals(aValue)); + attrs.put(attrName, attrValue); + spacePreserve = + spacePreserve || ("xml:space".equals(attrName) && "preserve".equals(attrValue)); } } } else { if (children == null) { children = new ArrayList<>(); } - XmlNode child = doBuild(parser, trim, locationBuilder, nsContext != null ? nsContext : Map.of()); + XmlNode child = doBuild(parser, trim, locationBuilder, nsContext); children.add(child); } } else if (eventType == XMLStreamReader.CHARACTERS || eventType == XMLStreamReader.CDATA) { String text = parser.getText(); - lValue = lValue != null ? lValue + text : text; + elementValue = elementValue != null ? elementValue + text : text; } else if (eventType == XMLStreamReader.END_ELEMENT) { boolean emptyTag = lastStartTag == parser.getLocation().getLineNumber() * 1000 + parser.getLocation().getColumnNumber(); - if (lValue != null && trim && !spacePreserve) { - lValue = lValue.trim(); + if (elementValue != null && trim && !spacePreserve) { + elementValue = elementValue.trim(); } return XmlNode.newBuilder() - .prefix(lPrefix) - .namespaceUri(lNamespaceUri) - .name(lName) - .value(children == null ? (lValue != null ? lValue : emptyTag ? null : "") : null) + .prefix(elementPrefix) + .namespaceUri(elementNamespaceUri) + .name(elementName) + .value(children == null ? (elementValue != null ? elementValue : emptyTag ? null : "") : null) .attributes(attrs) .namespaces(nsContext) .children(children) @@ -203,7 +206,7 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode node) throws XMLStream * @param attributes the attribute map (may contain xmlns: entries) * @param namespaces the namespace context (prefix → URI) for resolving prefixed attributes */ - static void writeAttributes( + private static void writeAttributes( XMLStreamWriter xmlWriter, Map attributes, Map namespaces) throws XMLStreamException { // Collect which namespace prefixes need to be declared on this element: