Skip to content

Commit 1cbf06d

Browse files
flavorjonesflavorjones
committed
fix: Document#canonicalize raises exception for incompatible modes
`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.
1 parent d9670ce commit 1cbf06d

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
@@ -38,6 +38,7 @@ This version of Nokogiri uses [`jar-dependencies`](https://github.com/mkristian/
3838

3939
### Improved
4040

41+
* `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.
4142
* 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)!)
4243
* Avoid compile-time conflict with system-installed `gumbo.h` on OpenBSD. [[#2464](https://github.com/sparklemotion/nokogiri/issues/2464)]
4344
* 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 (XMLSecurityException 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
@@ -141,6 +141,7 @@ def test_c14n_modes
141141
</n1:elem2>
142142
</n0:local>
143143
EOXML
144+
node1 = doc1.at_xpath("//n1:elem2", { "n1" => "http://example.net" })
144145

145146
doc2 = Nokogiri.XML(<<~EOXML)
146147
<n2:pdu xmlns:n1="http://example.com"
@@ -154,13 +155,14 @@ def test_c14n_modes
154155
</n1:elem2>
155156
</n2:pdu>
156157
EOXML
158+
node2 = doc2.at_xpath("//n1:elem2", { "n1" => "http://example.net" })
157159

158160
expected = <<~EOF.strip
159161
<n1:elem2 xmlns:n0="http://foobar.org" xmlns:n1="http://example.net" xmlns:n3="ftp://example.org" xml:lang="en">
160162
<n3:stuff></n3:stuff>
161163
</n1:elem2>
162164
EOF
163-
c14n = doc1.at_xpath("//n1:elem2", { "n1" => "http://example.net" }).canonicalize
165+
c14n = node1.canonicalize
164166
assert_equal(expected, c14n)
165167

166168
expected = <<~EOF.strip
@@ -169,15 +171,20 @@ def test_c14n_modes
169171
<n4:stuff></n4:stuff>
170172
</n1:elem2>
171173
EOF
172-
c14n = doc2.at_xpath("//n1:elem2", { "n1" => "http://example.net" }).canonicalize
174+
c14n = node2.canonicalize
173175
assert_equal(expected, c14n)
176+
c14n = node2.canonicalize(XML::XML_C14N_1_0)
177+
assert_equal(expected, c14n)
178+
assert_raises(RuntimeError) do
179+
node2.canonicalize(XML::XML_C14N_1_0, ["n2"])
180+
end
174181

175182
expected = <<~EOF.strip
176183
<n1:elem2 xmlns:n1="http://example.net" xml:lang="en">
177184
<n3:stuff xmlns:n3="ftp://example.org"></n3:stuff>
178185
</n1:elem2>
179186
EOF
180-
c14n = doc1.at_xpath("//n1:elem2", { "n1" => "http://example.net" }).canonicalize(XML::XML_C14N_EXCLUSIVE_1_0)
187+
c14n = node1.canonicalize(XML::XML_C14N_EXCLUSIVE_1_0)
181188
assert_equal(expected, c14n)
182189

183190
expected = <<~EOF.strip
@@ -186,7 +193,7 @@ def test_c14n_modes
186193
<n4:stuff xmlns:n4="http://foo.example"></n4:stuff>
187194
</n1:elem2>
188195
EOF
189-
c14n = doc2.at_xpath("//n1:elem2", { "n1" => "http://example.net" }).canonicalize(XML::XML_C14N_EXCLUSIVE_1_0)
196+
c14n = node2.canonicalize(XML::XML_C14N_EXCLUSIVE_1_0)
190197
assert_equal(expected, c14n)
191198

192199
expected = <<~EOF.strip
@@ -195,7 +202,7 @@ def test_c14n_modes
195202
<n4:stuff xmlns:n4="http://foo.example"></n4:stuff>
196203
</n1:elem2>
197204
EOF
198-
c14n = doc2.at_xpath("//n1:elem2", { "n1" => "http://example.net" }).canonicalize(XML::XML_C14N_EXCLUSIVE_1_0, ["n2"])
205+
c14n = node2.canonicalize(XML::XML_C14N_EXCLUSIVE_1_0, ["n2"])
199206
assert_equal(expected, c14n)
200207

201208
expected = <<~EOF.strip
@@ -204,8 +211,20 @@ def test_c14n_modes
204211
<n4:stuff></n4:stuff>
205212
</n1:elem2>
206213
EOF
207-
c14n = doc2.at_xpath("//n1:elem2", { "n1" => "http://example.net" }).canonicalize(XML::XML_C14N_EXCLUSIVE_1_0, ["n2", "n4"])
214+
c14n = node2.canonicalize(XML::XML_C14N_EXCLUSIVE_1_0, ["n2", "n4"])
208215
assert_equal(expected, c14n)
216+
217+
expected = <<~EOF.strip
218+
<n1:elem2 xmlns:n1="http://example.net" xmlns:n2="http://foo.example" xmlns:n4="http://foo.example" xml:lang="en" xml:space="retain">
219+
<n3:stuff xmlns:n3="ftp://example.org"></n3:stuff>
220+
<n4:stuff></n4:stuff>
221+
</n1:elem2>
222+
EOF
223+
c14n = node2.canonicalize(XML::XML_C14N_1_1)
224+
assert_equal(expected, c14n)
225+
assert_raises(RuntimeError) do
226+
node2.canonicalize(XML::XML_C14N_1_1, ["n2"])
227+
end
209228
end
210229

211230
def test_wrong_params

0 commit comments

Comments
 (0)