Skip to content

Commit 829041f

Browse files
flavorjonesflavorjones
authored
Merge pull request #2546 from sparklemotion/flavorjones-canonicalize-incompatible-modes
fix: Document#canonicalize raises exception for incompatible modes --- **What problem is this PR intended to solve?** `Document#canonicalize` now raises an exception if `inclusive_namespaces` is non-nil and the mode is inclusive, i.e. XML_C14N_1_0 or XML_C14N_1_1. `inclusive_namespaces` can only be passed with exclusive modes, and previously this silently failed. **Have you included adequate test coverage?** Yes, coverage has been added in this PR. **Does this change affect the behavior of either the C or the Java implementations?** Both the C and Java implementations have been updated.
2 parents 7661464 + 08823e0 commit 829041f

File tree

4 files changed

+40
-13
lines changed

4 files changed

+40
-13
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ This version of Nokogiri uses [`jar-dependencies`](https://github.com/mkristian/
3939

4040
### Improved
4141

42+
* `Document#canonicalize` now raises an exception if `inclusive_namespaces` is non-nil and the mode is inclusive, i.e. XML_C14N_1_0 or XML_C14N_1_1. `inclusive_namespaces` can only be passed with exclusive modes, and previously this silently failed.
4243
* Compare `Encoding` objects rather than compare their names. This is a slight performance improvement and is future-proof. [[#2454](https://github.com/sparklemotion/nokogiri/issues/2454)] (Thanks, [@casperisfine](https://github.com/casperisfine)!)
4344
* Avoid compile-time conflict with system-installed `gumbo.h` on OpenBSD. [[#2464](https://github.com/sparklemotion/nokogiri/issues/2464)]
4445
* Remove calls to `vasprintf` in favor of platform-independent `rb_vsprintf`

‎ext/java/nokogiri/XmlDocument.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -681,11 +681,9 @@ private static class DocumentBuilderFactoryHolder
681681
result = canonicalizer.canonicalizeSubtree(startingNode.getNode(), inclusive_namespace, filter);
682682
}
683683
return RubyString.newString(context.runtime, new ByteList(result, UTF8Encoding.INSTANCE));
684-
} catch (CanonicalizationException e) {
685-
// TODO Auto-generated catch block
686-
e.printStackTrace();
684+
} catch (Exception e) {
685+
throw context.getRuntime().newRuntimeError(e.getMessage());
687686
}
688-
return context.nil;
689687
}
690688

691689
private XmlNode

‎ext/nokogiri/xml_document.c

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,7 @@ rb_xml_document_canonicalize(int argc, VALUE *argv, VALUE self)
536536
VALUE rb_mode;
537537
VALUE rb_namespaces;
538538
VALUE rb_comments_p;
539+
int c_mode = 0;
539540
xmlChar **c_namespaces;
540541

541542
xmlDocPtr c_doc;
@@ -547,8 +548,16 @@ rb_xml_document_canonicalize(int argc, VALUE *argv, VALUE self)
547548
VALUE rb_io;
548549

549550
rb_scan_args(argc, argv, "03", &rb_mode, &rb_namespaces, &rb_comments_p);
550-
if (!NIL_P(rb_mode)) { Check_Type(rb_mode, T_FIXNUM); }
551-
if (!NIL_P(rb_namespaces)) { Check_Type(rb_namespaces, T_ARRAY); }
551+
if (!NIL_P(rb_mode)) {
552+
Check_Type(rb_mode, T_FIXNUM);
553+
c_mode = NUM2INT(rb_mode);
554+
}
555+
if (!NIL_P(rb_namespaces)) {
556+
Check_Type(rb_namespaces, T_ARRAY);
557+
if (c_mode == XML_C14N_1_0 || c_mode == XML_C14N_1_1) {
558+
rb_raise(rb_eRuntimeError, "This canonicalizer does not support this operation");
559+
}
560+
}
552561

553562
Data_Get_Struct(self, xmlDoc, c_doc);
554563

@@ -577,7 +586,7 @@ rb_xml_document_canonicalize(int argc, VALUE *argv, VALUE self)
577586
}
578587

579588
xmlC14NExecute(c_doc, c_callback_wrapper, rb_callback,
580-
(int)(NIL_P(rb_mode) ? 0 : NUM2INT(rb_mode)),
589+
c_mode,
581590
c_namespaces,
582591
(int)RTEST(rb_comments_p),
583592
c_obuf);

‎test/xml/test_c14n.rb

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ def test_c14n_modes
113113
</n1:elem2>
114114
</n0:local>
115115
EOXML
116+
node1 = doc1.at_xpath("//n1:elem2", { "n1" => "http://example.net" })
116117

117118
doc2 = Nokogiri.XML(<<~EOXML)
118119
<n2:pdu xmlns:n1="http://example.com"
@@ -126,13 +127,14 @@ def test_c14n_modes
126127
</n1:elem2>
127128
</n2:pdu>
128129
EOXML
130+
node2 = doc2.at_xpath("//n1:elem2", { "n1" => "http://example.net" })
129131

130132
expected = <<~EOF.strip
131133
<n1:elem2 xmlns:n0="http://foobar.org" xmlns:n1="http://example.net" xmlns:n3="ftp://example.org" xml:lang="en">
132134
<n3:stuff></n3:stuff>
133135
</n1:elem2>
134136
EOF
135-
c14n = doc1.at_xpath("//n1:elem2", { "n1" => "http://example.net" }).canonicalize
137+
c14n = node1.canonicalize
136138
assert_equal(expected, c14n)
137139

138140
expected = <<~EOF.strip
@@ -141,15 +143,20 @@ def test_c14n_modes
141143
<n4:stuff></n4:stuff>
142144
</n1:elem2>
143145
EOF
144-
c14n = doc2.at_xpath("//n1:elem2", { "n1" => "http://example.net" }).canonicalize
146+
c14n = node2.canonicalize
145147
assert_equal(expected, c14n)
148+
c14n = node2.canonicalize(XML::XML_C14N_1_0)
149+
assert_equal(expected, c14n)
150+
assert_raises(RuntimeError) do
151+
node2.canonicalize(XML::XML_C14N_1_0, ["n2"])
152+
end
146153

147154
expected = <<~EOF.strip
148155
<n1:elem2 xmlns:n1="http://example.net" xml:lang="en">
149156
<n3:stuff xmlns:n3="ftp://example.org"></n3:stuff>
150157
</n1:elem2>
151158
EOF
152-
c14n = doc1.at_xpath("//n1:elem2", { "n1" => "http://example.net" }).canonicalize(XML::XML_C14N_EXCLUSIVE_1_0)
159+
c14n = node1.canonicalize(XML::XML_C14N_EXCLUSIVE_1_0)
153160
assert_equal(expected, c14n)
154161

155162
expected = <<~EOF.strip
@@ -158,7 +165,7 @@ def test_c14n_modes
158165
<n4:stuff xmlns:n4="http://foo.example"></n4:stuff>
159166
</n1:elem2>
160167
EOF
161-
c14n = doc2.at_xpath("//n1:elem2", { "n1" => "http://example.net" }).canonicalize(XML::XML_C14N_EXCLUSIVE_1_0)
168+
c14n = node2.canonicalize(XML::XML_C14N_EXCLUSIVE_1_0)
162169
assert_equal(expected, c14n)
163170

164171
expected = <<~EOF.strip
@@ -167,7 +174,7 @@ def test_c14n_modes
167174
<n4:stuff xmlns:n4="http://foo.example"></n4:stuff>
168175
</n1:elem2>
169176
EOF
170-
c14n = doc2.at_xpath("//n1:elem2", { "n1" => "http://example.net" }).canonicalize(XML::XML_C14N_EXCLUSIVE_1_0, ["n2"])
177+
c14n = node2.canonicalize(XML::XML_C14N_EXCLUSIVE_1_0, ["n2"])
171178
assert_equal(expected, c14n)
172179

173180
expected = <<~EOF.strip
@@ -176,8 +183,20 @@ def test_c14n_modes
176183
<n4:stuff></n4:stuff>
177184
</n1:elem2>
178185
EOF
179-
c14n = doc2.at_xpath("//n1:elem2", { "n1" => "http://example.net" }).canonicalize(XML::XML_C14N_EXCLUSIVE_1_0, ["n2", "n4"])
186+
c14n = node2.canonicalize(XML::XML_C14N_EXCLUSIVE_1_0, ["n2", "n4"])
180187
assert_equal(expected, c14n)
188+
189+
expected = <<~EOF.strip
190+
<n1:elem2 xmlns:n1="http://example.net" xmlns:n2="http://foo.example" xmlns:n4="http://foo.example" xml:lang="en" xml:space="retain">
191+
<n3:stuff xmlns:n3="ftp://example.org"></n3:stuff>
192+
<n4:stuff></n4:stuff>
193+
</n1:elem2>
194+
EOF
195+
c14n = node2.canonicalize(XML::XML_C14N_1_1)
196+
assert_equal(expected, c14n)
197+
assert_raises(RuntimeError) do
198+
node2.canonicalize(XML::XML_C14N_1_1, ["n2"])
199+
end
181200
end
182201

183202
def test_wrong_params

0 commit comments

Comments
 (0)