Skip to content

Commit

Permalink
fix: prefer plain error collection to Nokogiri_error_raise
Browse files Browse the repository at this point in the history
Related to #1610
  • Loading branch information
flavorjones committed Oct 14, 2022
1 parent 4e7ffd9 commit 500eb8c
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 16 deletions.
18 changes: 3 additions & 15 deletions ext/nokogiri/xml_xpath_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ evaluate(int argc, VALUE *argv, VALUE self)
xmlXPathContextPtr ctx;
xmlXPathObjectPtr xpath;
xmlChar *query;
VALUE errors = rb_ary_new();

Data_Get_Struct(self, xmlXPathContext, ctx);

Expand All @@ -341,13 +342,7 @@ evaluate(int argc, VALUE *argv, VALUE self)
xmlXPathRegisterFuncLookup(ctx, lookup, (void *)xpath_handler);
}

xmlResetLastError();
#ifdef TRUFFLERUBY_NOKOGIRI_SYSTEM_LIBRARIES
VALUE errors = rb_ary_new();
xmlSetStructuredErrorFunc(errors, Nokogiri_error_array_pusher);
#else
xmlSetStructuredErrorFunc(NULL, Nokogiri_error_raise);
#endif
xmlSetStructuredErrorFunc((void*)errors, Nokogiri_error_array_pusher);

/* For some reason, xmlXPathEvalExpression will blow up with a generic error */
/* when there is a non existent function. */
Expand All @@ -357,15 +352,8 @@ evaluate(int argc, VALUE *argv, VALUE self)
xmlSetStructuredErrorFunc(NULL, NULL);
xmlSetGenericErrorFunc(NULL, NULL);

#ifdef TRUFFLERUBY_NOKOGIRI_SYSTEM_LIBRARIES
if (RARRAY_LEN(errors) > 0) {
rb_exc_raise(rb_ary_entry(errors, 0));
}
#endif

if (xpath == NULL) {
xmlErrorPtr error = xmlGetLastError();
rb_exc_raise(Nokogiri_wrap_xml_syntax_error(error));
rb_exc_raise(rb_ary_entry(errors, 0));
}

retval = xpath2ruby(xpath, ctx);
Expand Down
5 changes: 4 additions & 1 deletion test/xml/test_document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,10 @@ def test_namespace_should_not_exist
end

def test_non_existent_function
# TODO: we should not be raising different types on the different engines
# TODO: we should not be raising different types on the different engines. this happens
# because xpath_generic_exception_handler raises directly (and see #1610 for the dangers
# of that). we should either squash that exception (and rely on the structured errors) or
# we should create a SyntaxError from it and append it to the same errors array.
e_class = Nokogiri.uses_libxml? ? RuntimeError : Nokogiri::XML::XPath::SyntaxError

e = assert_raises(e_class) do
Expand Down

0 comments on commit 500eb8c

Please sign in to comment.