From 2a038fdd710135dd44a636414648596a646ccb33 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Mon, 24 Jun 2024 15:42:28 -0400 Subject: [PATCH] fix(jruby): Schema#validate should not accumulate errors in @errors Fixes #1282 --- CHANGELOG.md | 2 ++ ext/java/nokogiri/XmlSchema.java | 2 +- ext/nokogiri/xml_relax_ng.c | 4 ++-- ext/nokogiri/xml_schema.c | 6 ----- test/xml/test_schema.rb | 41 ++++++++++++++++++++++++++++++++ 5 files changed, 46 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09293b987fe..222aec55782 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,8 @@ Nokogiri follows [Semantic Versioning](https://semver.org/), please see the [REA * [CRuby] Update node GC lifecycle to avoid a potential memory leak with fragments in libxml 2.13.0 caused by changes in `xmlAddChild`. [#3156] @flavorjones * [CRuby] libgumbo correctly prints nonstandard element names in error messages. [#3219] @stevecheckoway * [JRuby] Fixed some bugs in how `Node#attributes` handles attributes with namespaces. [#2677, #2679] @flavorjones +* [JRuby] Fix `Schema#validate` to only return the most recent Document's errors. Previously, if multiple documents were validated, this method returned the accumulated errors of all previous documents. [#1282] @flavorjones +* [JRuby] Fix `Schema#validate` to not clobber the `@errors` instance variable. [#1282] @ flavorjones ### Changed diff --git a/ext/java/nokogiri/XmlSchema.java b/ext/java/nokogiri/XmlSchema.java index 0fbb7ad2ddb..2a58d31a498 100644 --- a/ext/java/nokogiri/XmlSchema.java +++ b/ext/java/nokogiri/XmlSchema.java @@ -222,7 +222,7 @@ public class XmlSchema extends RubyObject IRubyObject validate_document_or_file(ThreadContext context, XmlDocument xmlDocument) { - RubyArray errors = (RubyArray) this.getInstanceVariable("@errors"); + RubyArray errors = (RubyArray)context.runtime.newEmptyArray(); ErrorHandler errorHandler = new SchemaErrorHandler(context.runtime, errors); setErrorHandler(errorHandler); diff --git a/ext/nokogiri/xml_relax_ng.c b/ext/nokogiri/xml_relax_ng.c index 73b31328190..4cf74f7cd13 100644 --- a/ext/nokogiri/xml_relax_ng.c +++ b/ext/nokogiri/xml_relax_ng.c @@ -24,7 +24,7 @@ static const rb_data_type_t xml_relax_ng_type = { * Validate a Nokogiri::XML::Document against this RelaxNG schema. */ static VALUE -validate_document(VALUE self, VALUE document) +noko_xml_relax_ng__validate_document(VALUE self, VALUE document) { xmlDocPtr doc; xmlRelaxNGPtr schema; @@ -162,5 +162,5 @@ noko_init_xml_relax_ng(void) rb_define_singleton_method(cNokogiriXmlRelaxNG, "read_memory", read_memory, -1); rb_define_singleton_method(cNokogiriXmlRelaxNG, "from_document", from_document, -1); - rb_define_private_method(cNokogiriXmlRelaxNG, "validate_document", validate_document, 1); + rb_define_private_method(cNokogiriXmlRelaxNG, "validate_document", noko_xml_relax_ng__validate_document, 1); } diff --git a/ext/nokogiri/xml_schema.c b/ext/nokogiri/xml_schema.c index bc2e795cfd6..36f423c64ed 100644 --- a/ext/nokogiri/xml_schema.c +++ b/ext/nokogiri/xml_schema.c @@ -17,12 +17,6 @@ static const rb_data_type_t xml_schema_type = { .flags = RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED, }; -/* - * call-seq: - * validate_document(document) - * - * Validate a Nokogiri::XML::Document against this Schema. - */ static VALUE validate_document(VALUE self, VALUE document) { diff --git a/test/xml/test_schema.rb b/test/xml/test_schema.rb index 1a19c871d51..147fd1541d9 100644 --- a/test/xml/test_schema.rb +++ b/test/xml/test_schema.rb @@ -197,6 +197,47 @@ class TestNokogiriXMLSchema < Nokogiri::TestCase assert(xsd.valid?(valid_doc)) refute(xsd.valid?(invalid_doc)) end + + describe "error handling" do + let(:xsd) do + <<~EOF + + + + + EOF + end + + let(:good_xml) { %() } + let(:bad_xml) { %() } + + it "does not clobber @errors" do + schema = Nokogiri::XML::Schema.new(xsd) + bad_doc = Nokogiri::XML(bad_xml) + + # assert on setup + assert_empty(schema.errors) + refute_empty(schema.validate(bad_doc)) + + # this is the bit under test + assert_empty(schema.errors) + end + + it "returns only the most recent document's errors" do + # https://github.com/sparklemotion/nokogiri/issues/1282 + schema = Nokogiri::XML::Schema.new(xsd) + good_doc = Nokogiri::XML(good_xml) + bad_doc = Nokogiri::XML(bad_xml) + + # assert on setup + assert_empty(schema.validate(good_doc)) + refute_empty(schema.validate(bad_doc)) + + # this is the bit under test + assert_empty(schema.validate(good_doc)) + end + end end it "xsd_with_dtd" do