Skip to content

Commit 500eb8c

Browse files
committed
fix: prefer plain error collection to Nokogiri_error_raise
Related to #1610
1 parent 4e7ffd9 commit 500eb8c

File tree

2 files changed

+7
-16
lines changed

2 files changed

+7
-16
lines changed

ext/nokogiri/xml_xpath_context.c

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,7 @@ evaluate(int argc, VALUE *argv, VALUE self)
326326
xmlXPathContextPtr ctx;
327327
xmlXPathObjectPtr xpath;
328328
xmlChar *query;
329+
VALUE errors = rb_ary_new();
329330

330331
Data_Get_Struct(self, xmlXPathContext, ctx);
331332

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

344-
xmlResetLastError();
345-
#ifdef TRUFFLERUBY_NOKOGIRI_SYSTEM_LIBRARIES
346-
VALUE errors = rb_ary_new();
347-
xmlSetStructuredErrorFunc(errors, Nokogiri_error_array_pusher);
348-
#else
349-
xmlSetStructuredErrorFunc(NULL, Nokogiri_error_raise);
350-
#endif
345+
xmlSetStructuredErrorFunc((void*)errors, Nokogiri_error_array_pusher);
351346

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

360-
#ifdef TRUFFLERUBY_NOKOGIRI_SYSTEM_LIBRARIES
361-
if (RARRAY_LEN(errors) > 0) {
362-
rb_exc_raise(rb_ary_entry(errors, 0));
363-
}
364-
#endif
365-
366355
if (xpath == NULL) {
367-
xmlErrorPtr error = xmlGetLastError();
368-
rb_exc_raise(Nokogiri_wrap_xml_syntax_error(error));
356+
rb_exc_raise(rb_ary_entry(errors, 0));
369357
}
370358

371359
retval = xpath2ruby(xpath, ctx);

test/xml/test_document.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,10 @@ def test_namespace_should_not_exist
498498
end
499499

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

504507
e = assert_raises(e_class) do

0 commit comments

Comments
 (0)