You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "gnodet (via GitHub)" <gi...@apache.org> on 2026/03/23 10:12:32 UTC
[PR] Normalize redundant namespace prefixes on XML attributes (fixes #11760) [maven]
gnodet opened a new pull request, #11823:
URL: https://github.com/apache/maven/pull/11823
## Summary
- When a POM uses a namespace prefix mapping to the same URI as the element's default namespace (e.g., `xmlns:mvn="http://maven.apache.org/POM/4.0.0"`), prefixed attributes like `mvn:combine.children` are now normalized to their unprefixed form (`combine.children`) at read time
- Redundant `xmlns:prefix` declarations are removed
- The write side now properly handles namespace declarations (`xmlns:prefix`) and prefixed attributes for foreign namespaces
Fixes #11760
## Problem
The [jdbi project](https://github.com/apache/maven/issues/11760) uses `mvn:combine.children="append"` with `xmlns:mvn="http://maven.apache.org/POM/4.0.0"` to combine compiler arguments. This caused two bugs:
1. **Merge directives silently ignored**: The merge logic in `DefaultXmlService` looks up `combine.children` (unprefixed), so `mvn:combine.children` was never found and the append directive was ignored
2. **Invalid consumer POM**: The consumer POM transformation dropped the `xmlns:mvn` declaration but kept the prefixed attributes, producing invalid XML that couldn't be parsed by downstream consumers
## Fix
**Read side** (`DefaultXmlService.doBuild()`):
- When an attribute's namespace URI matches the element's default namespace, strip the prefix (since `mvn:combine.children` and `combine.children` are semantically identical in the same namespace)
- Remove `xmlns:prefix` declarations that are redundant with the element's default namespace
**Write side** (`DefaultXmlService.writeNode()`):
- `xmlns:prefix` attributes are written as proper namespace declarations via `writeNamespace()`
- Prefixed attributes with a matching `xmlns:` declaration are written with the correct namespace URI
- `xml:` prefixed attributes use the XML namespace URI
## Test plan
- [x] `testNamespacePrefixNormalization` — `mvn:combine.children` normalized to `combine.children`, `xmlns:mvn` removed
- [x] `testPrefixedCombineChildrenMerge` — `mvn:combine.children="append"` works correctly in merge
- [x] `testPrefixedCombineSelfMerge` — `mvn:combine.self="override"` works correctly in merge
- [x] `testForeignNamespacePrefixPreserved` — different namespace prefix (`custom:`) is preserved
- [x] `testXmlSpacePreservedAfterNormalization` — `xml:space="preserve"` unaffected
- [x] `testNestedNamespacePrefixNormalization` — works in deeply nested elements
- [x] `testWriteWithNamespaceAttributes` — foreign namespace attributes round-trip correctly
- [x] `testNormalizedRoundTrip` — no `xmlns:mvn` or `mvn:` prefix in output after normalization
- [x] All existing tests pass (43 tests in maven-xml, full downstream build clean)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix XML write side to handle namespace declarations and prefixed attributes (fixes #11760) [maven]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #11823:
URL: https://github.com/apache/maven/pull/11823#discussion_r2981126677
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -178,6 +176,42 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode node) throws XMLStream
xmlWriter.writeEndElement();
}
+ static void writeAttributes(XMLStreamWriter xmlWriter, Map<String, String> attributes) throws XMLStreamException {
+ // Write namespace declarations first, then regular attributes
+ for (Map.Entry<String, String> 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<String, String> attr : attributes.entrySet()) {
Review Comment:
attr --> attribute
##########
impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java:
##########
@@ -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 = """
+ <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:mvn="http://maven.apache.org/POM/4.0.0">
+ <compilerArgs mvn:combine.children="append">
+ <arg>-Xlint:deprecation</arg>
+ </compilerArgs>
+ </project>
+ """;
+
+ 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
Review Comment:
Not sure what this is trying to test but this doesn't sound right. I don't think the prefix should be removed.
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -178,6 +176,42 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode node) throws XMLStream
xmlWriter.writeEndElement();
}
+ static void writeAttributes(XMLStreamWriter xmlWriter, Map<String, String> attributes) throws XMLStreamException {
Review Comment:
This looks like it can be private
##########
impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java:
##########
@@ -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 = """
+ <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:mvn="http://maven.apache.org/POM/4.0.0">
+ <compilerArgs mvn:combine.children="append">
+ <arg>-Xlint:deprecation</arg>
+ </compilerArgs>
+ </project>
+ """;
+
+ 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();
Review Comment:
I'd just import this class instead of using the fully package qualified name
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -178,6 +176,42 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode node) throws XMLStream
xmlWriter.writeEndElement();
}
+ static void writeAttributes(XMLStreamWriter xmlWriter, Map<String, String> attributes) throws XMLStreamException {
+ // Write namespace declarations first, then regular attributes
+ for (Map.Entry<String, String> 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<String, String> 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:")) {
Review Comment:
The xml prefix is predefined and understood by all parsers. It does not need to be (must not be?) explicitly declared. You do need the else if case here to keep it from being written, but it should be empty.
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -178,6 +176,42 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode node) throws XMLStream
xmlWriter.writeEndElement();
}
+ static void writeAttributes(XMLStreamWriter xmlWriter, Map<String, String> attributes) throws XMLStreamException {
+ // Write namespace declarations first, then regular attributes
+ for (Map.Entry<String, String> attr : attributes.entrySet()) {
Review Comment:
Not sure about what XML library you're using here. Some do not include namespace declarations in their attribute map. If you're using DOM I think it depends on how the parser is configured, though I'd really need to review to make remember exactly how that works.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix XML write side to handle namespace declarations and prefixed attributes (fixes #11760) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #11823:
URL: https://github.com/apache/maven/pull/11823#discussion_r2982270672
##########
impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java:
##########
@@ -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 = """
+ <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:mvn="http://maven.apache.org/POM/4.0.0">
+ <compilerArgs mvn:combine.children="append">
+ <arg>-Xlint:deprecation</arg>
+ </compilerArgs>
+ </project>
+ """;
+
+ 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
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
This test reproduces the exact bug from issue #11760. Here's what happens in the consumer POM transformation:
1. The original POM has `xmlns:mvn="http://maven.apache.org/POM/4.0.0"` on the `<project>` element
2. Plugin configuration (e.g., `<compilerArgs mvn:combine.children="append">`) is stored as `XmlNode` trees — the attribute `mvn:combine.children` survives
3. During consumer POM transformation, the model is re-serialized from the `Model` object. The `xmlns:mvn` declaration on `<project>` is **lost** (it's not part of the Maven model)
4. When the `XmlNode` is written, `mvn:combine.children` has no corresponding namespace declaration → "Undeclared namespace prefix" error
The choice is: produce invalid XML (crash) or strip the orphaned prefix. We chose to strip the prefix since it's the only way to produce valid XML when the namespace declaration has been lost. Updated the test javadoc to explain this context.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix XML write side to handle namespace declarations and prefixed attributes (fixes #11760) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #11823:
URL: https://github.com/apache/maven/pull/11823#discussion_r2982267531
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -178,6 +176,42 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode node) throws XMLStream
xmlWriter.writeEndElement();
}
+ static void writeAttributes(XMLStreamWriter xmlWriter, Map<String, String> attributes) throws XMLStreamException {
+ // Write namespace declarations first, then regular attributes
+ for (Map.Entry<String, String> 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<String, String> attr : attributes.entrySet()) {
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Fixed — renamed to `attribute`.
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -178,6 +176,42 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode node) throws XMLStream
xmlWriter.writeEndElement();
}
+ static void writeAttributes(XMLStreamWriter xmlWriter, Map<String, String> attributes) throws XMLStreamException {
+ // Write namespace declarations first, then regular attributes
+ for (Map.Entry<String, String> 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<String, String> 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:")) {
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
You're right that the `xml:` prefix is predefined and must not be declared. However, we can't make this branch empty — attributes like `xml:space="preserve"` still need to be written to the output. If we skip them, whitespace preservation would be lost on round-trip. The `writeAttribute(namespaceURI, localName, value)` call is the correct StAX way to write them — it writes the attribute with the proper namespace without declaring the prefix (since it's predefined). Added a comment to explain this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix XML write side to handle namespace declarations and prefixed attributes (fixes #11760) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #11823:
URL: https://github.com/apache/maven/pull/11823#discussion_r2982265699
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -178,6 +176,42 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode node) throws XMLStream
xmlWriter.writeEndElement();
}
+ static void writeAttributes(XMLStreamWriter xmlWriter, Map<String, String> attributes) throws XMLStreamException {
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Good catch — made it `private`. It's not called from the generated template (which has its own copy of the logic).
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -178,6 +176,42 @@ private void writeNode(XMLStreamWriter xmlWriter, XmlNode node) throws XMLStream
xmlWriter.writeEndElement();
}
+ static void writeAttributes(XMLStreamWriter xmlWriter, Map<String, String> attributes) throws XMLStreamException {
+ // Write namespace declarations first, then regular attributes
+ for (Map.Entry<String, String> attr : attributes.entrySet()) {
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Good point. Maven uses StAX (XMLStreamReader) for parsing. The namespace declarations are reported separately via `getNamespaceCount()`/`getNamespacePrefix()`/`getNamespaceURI()` — they are NOT included in the attribute list (`getAttributeCount()`). However, during parsing in `doBuild()`, we explicitly store them as `xmlns:prefix` entries in the attributes map (see line 99-103). So by the time we write, they are reliably present in the map if they were on the original element.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix XML write side to handle namespace declarations and prefixed attributes (fixes #11760) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #11823:
URL: https://github.com/apache/maven/pull/11823#discussion_r2982269300
##########
impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java:
##########
@@ -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 = """
+ <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:mvn="http://maven.apache.org/POM/4.0.0">
+ <compilerArgs mvn:combine.children="append">
+ <arg>-Xlint:deprecation</arg>
+ </compilerArgs>
+ </project>
+ """;
+
+ 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();
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Fixed — added proper imports for `StringWriter`, `Map`, and `List`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix XML write side to handle namespace declarations and prefixed attributes (fixes #11760) [maven]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #11823:
URL: https://github.com/apache/maven/pull/11823#discussion_r2983800298
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
Review Comment:
I'm not sure why you check lName == null here. It might be fine but naively I'd expect this to be set unconditionally on a startElement
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -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<String, String> parentNamespaces)
throws XMLStreamException {
boolean spacePreserve = false;
String lPrefix = null;
Review Comment:
I'm not sure what all these l's stand for. local? location? something else? Unabbreviated names for the win.
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -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()) {
Review Comment:
so you're not putting the default namespace in the map? That's fine as long as the calling code expects that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix XML write side to handle namespace declarations and prefixed attributes (fixes #11760) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #11823:
URL: https://github.com/apache/maven/pull/11823#discussion_r2983575214
##########
impl/maven-xml/src/test/java/org/apache/maven/internal/xml/XmlNodeImplTest.java:
##########
@@ -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 = """
+ <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:mvn="http://maven.apache.org/POM/4.0.0">
+ <compilerArgs mvn:combine.children="append">
+ <arg>-Xlint:deprecation</arg>
+ </compilerArgs>
+ </project>
+ """;
+
+ 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
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
You were right — the prefix should not be removed when the namespace URI is known. We've now addressed this properly in the latest commit by adding a `namespaces()` method to `XmlNode` that captures the full namespace context (prefix → URI) inherited from ancestor elements during parsing.
When writing, if a prefixed attribute like `mvn:combine.children` appears but `xmlns:mvn` is not in the local attribute map, the writer now looks up the prefix in the namespace context, auto-declares the namespace, and writes the attribute with its proper prefix and namespace URI.
The prefix is only stripped as a last resort when no namespace context is available at all (e.g., a programmatically built `XmlNode` with no namespace bindings). This produces valid XML in all cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix XML namespace handling for prefixed attributes in consumer POM (fixes #11760) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #11823:
URL: https://github.com/apache/maven/pull/11823#discussion_r2986057176
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Good question — the `elementName == null` check is how this method distinguishes the first `START_ELEMENT` event (which is "this" element being parsed) from subsequent `START_ELEMENT` events (which are children, handled in the `else` branch). The method is called recursively for each child, and `elementName` starts as `null` — once the first start tag is seen, it gets set, so all further start tags within the same call are children. Added a comment to clarify this.
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -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<String, String> parentNamespaces)
throws XMLStreamException {
boolean spacePreserve = false;
String lPrefix = null;
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Agreed — renamed `lPrefix` → `elementPrefix`, `lName` → `elementName`, `lNamespaceUri` → `elementNamespaceUri`, `lValue` → `elementValue`. Also renamed `aName`/`aValue`/`aPrefix` → `attrName`/`attrValue`/`attrPrefix` in the attribute loop.
##########
impl/maven-xml/src/main/java/org/apache/maven/internal/xml/DefaultXmlService.java:
##########
@@ -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()) {
Review Comment:
_Claude Code on behalf of Guillaume Nodet_
Correct — the default namespace (empty prefix) is intentionally excluded from `namespaces()`. Per the XML namespace spec (Section 6.2), default namespace declarations do NOT apply to attributes, so it would never be useful for resolving prefixed attributes. Added a comment explaining this. We also have `testParseDefaultNamespaceNotInNamespacesMap` that verifies this behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix consumer POM serialization of prefixed XML attributes (fixes #11760) [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #11823:
URL: https://github.com/apache/maven/pull/11823
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix consumer POM serialization of prefixed XML attributes (fixes #11760) [maven]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11823:
URL: https://github.com/apache/maven/pull/11823#issuecomment-4489968244
@gnodet Please assign appropriate label to PR according to the type of change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org