Skip to content

Commit

Permalink
Start with a failing test that exercises desired functionality
Browse files Browse the repository at this point in the history
Write down a plan

Add notes about replacing transmute

Add notes about whether calling xmlResetError is needed
  • Loading branch information
JDSeiler authored and dginev committed Jul 18, 2023
1 parent 7724e23 commit 414d2ba
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 7 deletions.
27 changes: 27 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,33 @@ use super::bindings;

use std::ffi::CStr;

/*
TODO:
The reason that the validation errors are duplicated is because libxml mutates
the same memory location for each error. This means that every StructuredError
object is actually wrapping the same pointer, and so every StructuredError ends
up with the contents of the last error that was generated.
My tentative plan is to rewrite StructuredError so that it contains all the same
fields as libxml's xmlError struct, and update the plumbing so that the errors
are copied into a new StructuredError object for each error that is generated.
After rummaging around in the libxml source code, I don't think we need to be calling
xmlResetError, at least in the new version where we'll be copying the data out into
a normal Rust struct.
In error.c, the __xmlRaiseError function appears to be what ultimately calls the
structured error handler (defined in schema/common, in this crate). __xmlRaiseError
does not allocate any new memory for the error it reports. Instead, it refers to a
a global: `xmlLastError`. In a threaded context, `xmlLastError` is buried inside
a "global state" struct that is unique per thread. Otherwise, xmlLastError is file
level variable that is globally shared through macro magic I don't quite grasp.
Point being, I don't think we need to call resetError as part of a Drop impl.
Even in a threaded context I don't think it'd make a difference, because internally
it calls memset instead of free, so it doesn't release any memory to the system.
*/

/// Wrapper around xmlErrorPtr
#[derive(Debug)]
pub struct StructuredError(*mut bindings::_xmlError);
Expand Down
5 changes: 5 additions & 0 deletions src/schemas/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ use crate::tree::document::Document;
use std::ffi::CString;
use std::os::raw::c_char;

/*
TODO:
- Can we replace the usage of transmute with Box::into_row and Box::from_raw?
*/

/// Wrapper on xmlSchemaParserCtxt
pub struct SchemaParserContext {
inner: *mut bindings::_xmlSchemaParserCtxt,
Expand Down
5 changes: 5 additions & 0 deletions src/schemas/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ use crate::error::StructuredError;
use std::ffi::CString;
use std::os::raw::c_char;

/*
TODO:
- Can we replace the usage of transmute with Box::into_row and Box::from_raw?
*/

/// Wrapper on xmlSchemaValidCtxt
pub struct SchemaValidationContext {
ctxt: *mut bindings::_xmlSchemaValidCtxt,
Expand Down
85 changes: 78 additions & 7 deletions tests/schema_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use libxml::schemas::SchemaValidationContext;

use libxml::parser::Parser;

static SCHEMA: &'static str = r#"<?xml version="1.0"?>
static NOTE_SCHEMA: &'static str = r#"<?xml version="1.0"?>
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema">
<xs:element name="note">
<xs:complexType>
Expand All @@ -21,7 +21,28 @@ static SCHEMA: &'static str = r#"<?xml version="1.0"?>
</xs:schema>
"#;

static XML: &'static str = r#"<?xml version="1.0"?>
static STOCK_SCHEMA: &'static str = r#"<?xml version="1.0"?>
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema">
<xs:element name="stock">
<xs:complexType>
<xs:sequence maxOccurs="unbounded">
<xs:element name="sample">
<xs:complexType>
<xs:all>
<xs:element name="date" type="xs:date"/>
<xs:element name="price" type="xs:float"/>
</xs:all>
</xs:complexType>
</xs:element>
</xs:sequence>
<xs:attribute name="ticker" type="xs:string" use="required"/>
<xs:attribute name="exchange" type="xs:string" use="required"/>
</xs:complexType>
</xs:element>
</xs:schema>
"#;

static VALID_NOTE_XML: &'static str = r#"<?xml version="1.0"?>
<note>
<to>Tove</to>
<from>Jani</from>
Expand All @@ -30,7 +51,7 @@ static XML: &'static str = r#"<?xml version="1.0"?>
</note>
"#;

static INVALID_XML: &'static str = r#"<?xml version="1.0"?>
static INVALID_NOTE_XML: &'static str = r#"<?xml version="1.0"?>
<note>
<bad>Tove</bad>
<another>Jani</another>
Expand All @@ -39,13 +60,30 @@ static INVALID_XML: &'static str = r#"<?xml version="1.0"?>
</note>
"#;

static INVALID_STOCK_XML: &'static str = r#"<?xml version="1.0"?>
<stock junkAttribute="foo">
<sample>
<date>2014-01-01</date>
<price>NOT A NUMBER</price>
</sample>
<sample>
<date>2014-01-02</date>
<price>540.98</price>
</sample>
<sample>
<date>NOT A DATE</date>
<price>543.93</price>
</sample>
</stock
"#;

#[test]
fn schema_from_string() {
let xml = Parser::default()
.parse_string(XML)
.parse_string(VALID_NOTE_XML)
.expect("Expected to be able to parse XML Document from string");

let mut xsdparser = SchemaParserContext::from_buffer(SCHEMA);
let mut xsdparser = SchemaParserContext::from_buffer(NOTE_SCHEMA);
let xsd = SchemaValidationContext::from_parser(&mut xsdparser);

if let Err(errors) = xsd {
Expand Down Expand Up @@ -73,10 +111,10 @@ fn schema_from_string() {
#[test]
fn schema_from_string_generates_errors() {
let xml = Parser::default()
.parse_string(INVALID_XML)
.parse_string(INVALID_NOTE_XML)
.expect("Expected to be able to parse XML Document from string");

let mut xsdparser = SchemaParserContext::from_buffer(SCHEMA);
let mut xsdparser = SchemaParserContext::from_buffer(NOTE_SCHEMA);
let xsd = SchemaValidationContext::from_parser(&mut xsdparser);

if let Err(errors) = xsd {
Expand All @@ -99,3 +137,36 @@ fn schema_from_string_generates_errors() {
}
}
}

#[test]
fn schema_from_string_reports_unique_errors() {
let xml = Parser::default()
.parse_string(INVALID_STOCK_XML)
.expect("Expected to be able to parse XML Document from string");

let mut xsdparser = SchemaParserContext::from_buffer(STOCK_SCHEMA);
let xsd = SchemaValidationContext::from_parser(&mut xsdparser);

if let Err(errors) = xsd {
for err in &errors {
println!("{}", err.message());
}

panic!("Failed to parse schema");
}

let mut xsdvalidator = xsd.unwrap();
if let Err(errors) = xsdvalidator.validate_document(&xml) {
assert_eq!(errors.len(), 5);
let expected_errors = vec![
"Element 'stock', attribute 'junkAttribute': The attribute 'junkAttribute' is not allowed.\n",
"Element 'stock': The attribute 'ticker' is required but missing.\n",
"Element 'stock': The attribute 'exchange' is required but missing.\n",
"Element 'price': 'NOT A NUMBER' is not a valid value of the atomic type 'xs:float'.\n",
"Element 'date': 'NOT A DATE' is not a valid value of the atomic type 'xs:date'.\n"
];
for err_msg in expected_errors {
assert!(errors.iter().any(|err| err.message() == err_msg), "Expected error message {} was not found", err_msg);
}
}
}

0 comments on commit 414d2ba

Please sign in to comment.