Skip to content

Commit 4473261

Browse files
authored
Fix MRI Ruby vs. JRuby XML child namespace output differences (backport v1.18x) (#3476)
**What problem is this PR intended to solve?** Fix MRI Ruby vs. JRuby XML child namespace output differences Fixes #3455 Backport of #3456 cc @johnnyshields
2 parents 80edf1c + 6cac169 commit 4473261

File tree

3 files changed

+344
-3
lines changed

3 files changed

+344
-3
lines changed

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ Nokogiri follows [Semantic Versioning](https://semver.org/), please see the [REA
44

55
---
66

7+
## next
8+
9+
### Fixed
10+
11+
* [JRuby] Update JRuby's XML serialization so it outputs namespaces exactly like CRuby. (#3455, #3456) @johnnyshields
12+
13+
714
## v1.18.4 / 2025-03-14
815

916
### Security

ext/java/nokogiri/internals/SaveContextVisitor.java

Lines changed: 82 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public class SaveContextVisitor
5858
private final Deque<Attr[]> c14nNamespaceStack;
5959
private final Deque<Attr[]> c14nAttrStack;
6060
//private List<String> c14nExclusiveInclusivePrefixes = null;
61+
private final Stack<Map<String, String>> xmlnsNamespaceStack;
6162

6263
/*
6364
* U can't touch this.
@@ -117,6 +118,7 @@ public class SaveContextVisitor
117118
if ((asBuilder && indent == null) || (asBuilder && indent.length() == 0)) { indent = " "; } // default, two spaces
118119
indentString = indent;
119120
if (!asXml && !asHtml && !asXhtml && !asBuilder) { asXml = true; }
121+
xmlnsNamespaceStack = asXml ? new Stack<Map<String, String>>() : null;
120122
}
121123

122124
@Override
@@ -432,19 +434,24 @@ public class SaveContextVisitor
432434
public boolean
433435
enter(Element element)
434436
{
437+
pushXmlnsNamespaceStack();
438+
435439
if (canonical) {
436440
c14nNodeList.add(element);
437441
if (element == element.getOwnerDocument().getDocumentElement()) {
438442
c14nNodeList.add(element.getOwnerDocument());
439443
}
440444
}
445+
441446
String current = indentation.peek();
442447
buffer.append(current);
443448
if (needIndent(element)) {
444449
indentation.push(current + indentString);
445450
}
451+
446452
String name = element.getTagName();
447453
buffer.append('<').append(name);
454+
448455
Attr[] attrs = getAttrsAndNamespaces(element);
449456
for (Attr attr : attrs) {
450457
if (attr.getSpecified()) {
@@ -453,11 +460,13 @@ public class SaveContextVisitor
453460
leave(attr);
454461
}
455462
}
463+
456464
if (element.hasChildNodes()) {
457465
buffer.append('>');
458466
if (needBreakInOpening(element)) { buffer.append('\n'); }
459467
return true;
460468
}
469+
461470
// no child
462471
if (asHtml) {
463472
buffer.append('>');
@@ -472,12 +481,38 @@ public class SaveContextVisitor
472481
} else {
473482
buffer.append("/>");
474483
}
484+
475485
if (needBreakInOpening(element)) {
476486
buffer.append('\n');
477487
}
478488
return true;
479489
}
480490

491+
private Map<String, String>
492+
pushXmlnsNamespaceStack() {
493+
if (!asXml || xmlnsNamespaceStack == null) { return null; }
494+
Map<String, String> newContext;
495+
if (xmlnsNamespaceStack.isEmpty()) {
496+
newContext = new HashMap<String, String>();
497+
} else {
498+
Map<String, String> parentContext = xmlnsNamespaceStack.peek();
499+
newContext = new HashMap<String, String>(parentContext);
500+
}
501+
return xmlnsNamespaceStack.push(newContext);
502+
}
503+
504+
private Map<String, String>
505+
popXmlnsNamespaceStack() {
506+
if (!asXml || xmlnsNamespaceStack == null || xmlnsNamespaceStack.isEmpty()) { return null; }
507+
return xmlnsNamespaceStack.pop();
508+
}
509+
510+
private Map<String, String>
511+
peekXmlnsNamespaceStack() {
512+
if (!asXml || xmlnsNamespaceStack == null || xmlnsNamespaceStack.isEmpty()) { return null; }
513+
return xmlnsNamespaceStack.peek();
514+
}
515+
481516
private boolean
482517
needIndent(Element element)
483518
{
@@ -511,11 +546,15 @@ public class SaveContextVisitor
511546
NamedNodeMap attrs = element.getAttributes();
512547
if (!canonical) {
513548
if (attrs == null || attrs.getLength() == 0) { return new Attr[0]; }
514-
Attr[] attrsAndNamespaces = new Attr[attrs.getLength()];
549+
Map<String, String> xmlnsContext = peekXmlnsNamespaceStack();
550+
List<Attr> filteredAttrsAndNamespaces = new ArrayList<Attr>();
515551
for (int i = 0; i < attrs.getLength(); i++) {
516-
attrsAndNamespaces[i] = (Attr) attrs.item(i);
552+
Attr attr = (Attr) attrs.item(i);
553+
if (!findOrAddRedundantNamespaceAttr(xmlnsContext, attr)) {
554+
filteredAttrsAndNamespaces.add(attr);
555+
}
517556
}
518-
return attrsAndNamespaces;
557+
return filteredAttrsAndNamespaces.toArray(new Attr[0]);
519558
} else {
520559
List<Attr> namespaces = new ArrayList<Attr>();
521560
List<Attr> attributes = new ArrayList<Attr>();
@@ -544,7 +583,45 @@ public class SaveContextVisitor
544583
c14nAttrStack.push(attributeArray);
545584
return allAttrs;
546585
}
586+
}
587+
588+
/**
589+
* Detects whether a given attribute is a redundant xmlns namespace
590+
* already present within xmlnsContext.
591+
*
592+
* As a side-effect, if the attribute is a non-redundant namespace,
593+
* it is added to the xmlnsContext, so that it will be considered redundant
594+
* for subsequent checks.
595+
*
596+
* @param xmlnsContext The namespace context, which should be the top object
597+
* of xmlnsNamespaceStack.
598+
* @param attr The attribute to check.
599+
* @return True if the object is redundant, false otherwise.
600+
*/
601+
private boolean
602+
findOrAddRedundantNamespaceAttr(Map<String, String> xmlnsContext, Attr attr) {
603+
if (xmlnsContext == null || !attr.getSpecified()) { return false; }
604+
605+
String xmlnsPrefix;
606+
String attrName = attr.getNodeName();
607+
if (attrName.equals("xmlns")) {
608+
xmlnsPrefix = "";
609+
} else if (attrName.startsWith("xmlns:")) {
610+
xmlnsPrefix = attrName.substring(6);
611+
} else {
612+
// Not a namespace attribute
613+
return false;
614+
}
547615

616+
String xmlnsUri = attr.getNodeValue();
617+
if (xmlnsContext.containsKey(xmlnsPrefix) && xmlnsUri.equals(xmlnsContext.get(xmlnsPrefix))) {
618+
// Redundant namespace detected
619+
return true;
620+
} else {
621+
// Add non-redundant namespace to the top of xmlnsNamespaceStack
622+
xmlnsContext.put(xmlnsPrefix, xmlnsUri);
623+
return false;
624+
}
548625
}
549626

550627
private void
@@ -653,6 +730,8 @@ public int compare(Attr attr0, Attr attr1) {
653730
public void
654731
leave(Element element)
655732
{
733+
popXmlnsNamespaceStack();
734+
656735
if (canonical) {
657736
c14nNamespaceStack.poll();
658737
c14nAttrStack.poll();

0 commit comments

Comments
 (0)