Skip to content

Commit

Permalink
Ensuring error handling in HTML docs and fragments
Browse files Browse the repository at this point in the history
Specifically:
- when copying a node between documents, errors are propagated to the
  destination document
- when duplicating a node, silence errors

Fixes #1196
  • Loading branch information
flavorjones committed Dec 31, 2014
1 parent 9bbd666 commit e4d587e
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 1 deletion.
16 changes: 15 additions & 1 deletion ext/nokogiri/xml_node.c
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,15 @@ static VALUE reparent_node_with(VALUE pivot_obj, VALUE reparentee_obj, pivot_rep
* reparent the actual reparentee, so we reparent a duplicate.
*/
nokogiri_root_node(reparentee);
if (!(reparentee = xmlDocCopyNode(reparentee, pivot->doc, 1))) {

xmlResetLastError();
xmlSetStructuredErrorFunc((void *)rb_iv_get(DOC_RUBY_OBJECT(pivot->doc), "@errors"), Nokogiri_error_array_pusher);

reparentee = xmlDocCopyNode(reparentee, pivot->doc, 1) ;

xmlSetStructuredErrorFunc(NULL, NULL);

if (! reparentee) {
rb_raise(rb_eRuntimeError, "Could not reparent node (xmlDocCopyNode)");
}
}
Expand Down Expand Up @@ -473,7 +481,13 @@ static VALUE duplicate_node(int argc, VALUE *argv, VALUE self)

Data_Get_Struct(self, xmlNode, node);

xmlResetLastError();
xmlSetStructuredErrorFunc(NULL, Nokogiri_error_silencer);

dup = xmlDocCopyNode(node, node->doc, (int)NUM2INT(level));

xmlSetStructuredErrorFunc(NULL, NULL);

if(dup == NULL) return Qnil;

nokogiri_root_node(dup);
Expand Down
4 changes: 4 additions & 0 deletions ext/nokogiri/xml_syntax_error.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ void Nokogiri_error_array_pusher(void * ctx, xmlErrorPtr error)
rb_ary_push(list, Nokogiri_wrap_xml_syntax_error(error));
}

void Nokogiri_error_silencer(void * ctx, xmlErrorPtr error)
{
}

void Nokogiri_error_raise(void * ctx, xmlErrorPtr error)
{
rb_exc_raise(Nokogiri_wrap_xml_syntax_error(error));
Expand Down
1 change: 1 addition & 0 deletions ext/nokogiri/xml_syntax_error.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
void init_xml_syntax_error();
VALUE Nokogiri_wrap_xml_syntax_error(xmlErrorPtr error);
void Nokogiri_error_array_pusher(void * ctx, xmlErrorPtr error);
void Nokogiri_error_silencer(void * ctx, xmlErrorPtr error);
NORETURN(void Nokogiri_error_raise(void * ctx, xmlErrorPtr error));

extern VALUE cNokogiriXmlSyntaxError;
Expand Down
14 changes: 14 additions & 0 deletions test/html/test_document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,20 @@ def test_capturing_nonparse_errors_during_document_clone
copy = original.dup
assert_equal original_errors, copy.errors
end

def test_capturing_nonparse_errors_during_node_copy_between_docs
doc1 = Nokogiri::HTML("<div id='unique'>one</div>")
doc2 = Nokogiri::HTML("<div id='unique'>two</div>")
node1 = doc1.at_css("#unique")
node2 = doc2.at_css("#unique")

original_errors = doc1.errors.dup

node1.add_child node2

assert_equal original_errors.length+1, doc1.errors.length
assert_match /ID unique already defined/, doc1.errors.last.to_s
end
end
end
end
23 changes: 23 additions & 0 deletions test/html/test_document_fragment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,29 @@ def test_error_propagation_on_fragment_parse_in_node_context_should_not_include_
assert frag.errors.any?{|err| err.to_s =~ /Tag hello invalid/}, "errors should be on the context node's document"
assert frag.errors.none?{|err| err.to_s =~ /jimmy/}, "errors should not include pre-existing document errors"
end

def test_capturing_nonparse_errors_during_fragment_clone
# see https://github.com/sparklemotion/nokogiri/issues/1196 for background
original = Nokogiri::HTML.fragment("<div id='unique'></div>")
original_errors = original.errors.dup

copy = original.dup
assert_equal original_errors, copy.errors
end

def test_capturing_nonparse_errors_during_node_copy_between_fragments
frag1 = Nokogiri::HTML.fragment("<div id='unique'>one</div>")
frag2 = Nokogiri::HTML.fragment("<div id='unique'>two</div>")
node1 = frag1.at_css("#unique")
node2 = frag2.at_css("#unique")

original_errors = frag1.errors.dup

node1.add_child node2 # we should also not see an error on stderr

assert_equal original_errors.length+1, frag1.errors.length
assert_match /ID unique already defined/, frag1.errors.last.to_s
end
end
end
end

0 comments on commit e4d587e

Please sign in to comment.