Skip to content

Commit

Permalink
fix: prefer plain error collection to raising an exception
Browse files Browse the repository at this point in the history
in xml_xpath_context.c:evaluate generic error handling

Related to #1610
  • Loading branch information
flavorjones committed Oct 14, 2022
1 parent 500eb8c commit e00857f
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 15 deletions.
18 changes: 10 additions & 8 deletions ext/nokogiri/xml_xpath_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -292,11 +292,14 @@ lookup(void *ctx,
}

PRINTFLIKE_DECL(2, 3)
NORETURN_DECL
static void
xpath_generic_exception_handler(void *ctx, const char *msg, ...)
xpath_generic_exception_pusher(void *ctx, const char *msg, ...)
{
VALUE list = (VALUE)ctx;
VALUE rb_message;
VALUE rb_exception;

Check_Type(list, T_ARRAY);

#ifdef TRUFFLERUBY_NOKOGIRI_SYSTEM_LIBRARIES
/* It is not currently possible to pass var args from native
Expand All @@ -309,7 +312,8 @@ xpath_generic_exception_handler(void *ctx, const char *msg, ...)
va_end(args);
#endif

rb_exc_raise(rb_exc_new3(rb_eRuntimeError, rb_message));
rb_exception = rb_exc_new_str(cNokogiriXmlXpathSyntaxError, rb_message);
rb_ary_push(list, rb_exception);
}

/*
Expand Down Expand Up @@ -342,13 +346,11 @@ evaluate(int argc, VALUE *argv, VALUE self)
xmlXPathRegisterFuncLookup(ctx, lookup, (void *)xpath_handler);
}

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. */
xmlSetGenericErrorFunc(NULL, xpath_generic_exception_handler);
xmlSetStructuredErrorFunc((void *)errors, Nokogiri_error_array_pusher);
xmlSetGenericErrorFunc((void *)errors, xpath_generic_exception_pusher);

xpath = xmlXPathEvalExpression(query, ctx);

xmlSetStructuredErrorFunc(NULL, NULL);
xmlSetGenericErrorFunc(NULL, NULL);

Expand Down
8 changes: 1 addition & 7 deletions test/xml/test_document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -498,13 +498,7 @@ def test_namespace_should_not_exist
end

def test_non_existent_function
# 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
e = assert_raises(Nokogiri::XML::XPath::SyntaxError) do
xml.xpath("//name[foo()]")
end
assert_match(/function.*not found|Could not find function/, e.to_s)
Expand Down

0 comments on commit e00857f

Please sign in to comment.