Skip to content

Commit

Permalink
fix(jruby): Schema#validate should not accumulate errors in @errors (#…
Browse files Browse the repository at this point in the history
…3258)

**What problem is this PR intended to solve?**

Fixes #1282

**Have you included adequate test coverage?**

Yes.

**Does this change affect the behavior of either the C or the Java
implementations?**

Brings the JRuby impl in line with the CRuby impl.
  • Loading branch information
flavorjones authored Jun 24, 2024
2 parents 6cb7f0f + 2a038fd commit 73d9f35
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 9 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion ext/java/nokogiri/XmlSchema.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions ext/nokogiri/xml_relax_ng.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
6 changes: 0 additions & 6 deletions ext/nokogiri/xml_schema.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
41 changes: 41 additions & 0 deletions test/xml/test_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
<?xml version="1.0" encoding="UTF-8"?>
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"
targetNamespace="http://www.example.org/contactExample">
<xs:element name="Contacts"></xs:element>
</xs:schema>
EOF
end

let(:good_xml) { %(<Contacts xmlns="http://www.example.org/contactExample"><Contact></Contact></Contacts>) }
let(:bad_xml) { %(<Contacts xmlns="http://www.example.org/wrongNs"><Contact></Contact></Contacts>) }

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
Expand Down

0 comments on commit 73d9f35

Please sign in to comment.