Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(error-handling): change panics into recoverable results #121

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion examples/schema_example.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,13 @@ fn main() {
.parse_file("tests/resources/schema.xml")
.expect("Expected to be able to parse XML Document from file");

let mut xsdparser = SchemaParserContext::from_file("tests/resources/schema.xsd");
let mut xsdparser =
SchemaParserContext::from_file("tests/resources/schema.xsd").unwrap_or_else(|err| {
println!("{}", err.message.as_ref().unwrap());

panic!("Failed to create parsing context");
});

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

if let Err(errors) = xsd {
Expand Down
54 changes: 49 additions & 5 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ pub struct StructuredError {

impl StructuredError {
/// Copies the error information stored at `error_ptr` into a new `StructuredError`
///
///
/// # Safety
/// This function must be given a pointer to a valid `xmlError` struct. Typically, you
/// will acquire such a pointer by implementing one of a number of callbacks
/// defined in libXml which are provided an `xmlError` as an argument.
///
///
/// This function copies data from the memory `error_ptr` but does not deallocate
/// the error. Depending on the context in which this function is used, you may
/// need to take additional steps to avoid a memory leak.
Expand Down Expand Up @@ -94,12 +94,56 @@ impl StructuredError {
}
}

/// Creates a custom `StructuredError`to report unexpected 0-byte when attempting to convert `Vec<u8>` to `CString`
pub fn cstring_error(position: usize) -> Self {
StructuredError {
message: Some(
format!("Path contained an unexpected null-terminator at position {position}. String must not contain any 0-bytes."),
),
level: XmlErrorLevel::Fatal,
filename: None,
line: None,
col: None,
domain: 0,
code: -1,
}
}

/// Creates a custom `StructuredError`to report an invalid pointer-reference
pub fn null_ptr() -> Self {
StructuredError {
message: Some("Could not create context due to an unexpected null-reference.".into()),
level: XmlErrorLevel::Fatal,
filename: None,
line: None,
col: None,
domain: 0,
code: -1,
}
}

/// Wraps the `libxml2` internal error (exit-code `-1`) in a `StructuredError`
pub fn internal() -> Self {
StructuredError {
message: Some("Failed to validate file due to internal error".into()),
level: XmlErrorLevel::Fatal,
filename: None,
line: None,
col: None,
domain: 0,
code: -1,
}
}

/// Human-readable informative error message.
///
///
/// This function is a hold-over from the original bindings to libxml's error
/// reporting mechanism. Instead of calling this method, you can access the
/// reporting mechanism. Instead of calling this method, you can access the
/// StructuredError `message` field directly.
#[deprecated(since="0.3.3", note="Please use the `message` field directly instead.")]
#[deprecated(
since = "0.3.3",
note = "Please use the `message` field directly instead."
)]
pub fn message(&self) -> &str {
self.message.as_deref().unwrap_or("")
}
Expand Down
21 changes: 11 additions & 10 deletions src/schemas/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,43 +18,44 @@ pub struct SchemaParserContext {

impl SchemaParserContext {
/// Create a schema parsing context from a Document object
pub fn from_document(doc: &Document) -> Self {
pub fn from_document(doc: &Document) -> Result<Self, StructuredError> {
let parser = unsafe { bindings::xmlSchemaNewDocParserCtxt(doc.doc_ptr()) };

if parser.is_null() {
panic!("Failed to create schema parser context from XmlDocument"); // TODO error handling
return Err(StructuredError::null_ptr());
}

Self::from_raw(parser)
Ok(Self::from_raw(parser))
}

/// Create a schema parsing context from a buffer in memory
pub fn from_buffer<Bytes: AsRef<[u8]>>(buff: Bytes) -> Self {
pub fn from_buffer<Bytes: AsRef<[u8]>>(buff: Bytes) -> Result<Self, StructuredError> {
let buff_bytes = buff.as_ref();
let buff_ptr = buff_bytes.as_ptr() as *const c_char;
let buff_len = buff_bytes.len() as i32;

let parser = unsafe { bindings::xmlSchemaNewMemParserCtxt(buff_ptr, buff_len) };

if parser.is_null() {
panic!("Failed to create schema parser context from buffer"); // TODO error handling
return Err(StructuredError::null_ptr());
}

Self::from_raw(parser)
Ok(Self::from_raw(parser))
}

/// Create a schema parsing context from an URL
pub fn from_file(path: &str) -> Self {
let path = CString::new(path).unwrap(); // TODO error handling for \0 containing strings
pub fn from_file(path: &str) -> Result<Self, StructuredError> {
let path =
CString::new(path).map_err(|err| StructuredError::cstring_error(err.nul_position()))?;
let path_ptr = path.as_bytes_with_nul().as_ptr() as *const c_char;

let parser = unsafe { bindings::xmlSchemaNewParserCtxt(path_ptr) };

if parser.is_null() {
panic!("Failed to create schema parser context from path"); // TODO error handling
return Err(StructuredError::null_ptr());
}

Self::from_raw(parser)
Ok(Self::from_raw(parser))
}

/// Drains error log from errors that might have accumulated while parsing schema
Expand Down
11 changes: 6 additions & 5 deletions src/schemas/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl SchemaValidationContext {
let ctx = unsafe { bindings::xmlSchemaNewValidCtxt(s.as_ptr()) };

if ctx.is_null() {
panic!("Failed to create validation context from XML schema") // TODO error handling
return Err(vec![StructuredError::null_ptr()]);
}

Ok(Self::from_raw(ctx, s))
Expand All @@ -48,21 +48,22 @@ impl SchemaValidationContext {
let rc = unsafe { bindings::xmlSchemaValidateDoc(self.ctxt, doc.doc_ptr()) };

match rc {
-1 => panic!("Failed to validate document due to internal error"), // TODO error handling
-1 => Err(vec![StructuredError::internal()]),
0 => Ok(()),
_ => Err(self.drain_errors()),
}
}

/// Validates a given file from path for its compliance with the loaded XSD schema definition
pub fn validate_file(&mut self, path: &str) -> Result<(), Vec<StructuredError>> {
let path = CString::new(path).unwrap(); // TODO error handling for \0 containing strings
let path =
CString::new(path).map_err(|err| vec![StructuredError::cstring_error(err.nul_position())])?;
let path_ptr = path.as_bytes_with_nul().as_ptr() as *const c_char;

let rc = unsafe { bindings::xmlSchemaValidateFile(self.ctxt, path_ptr, 0) };

match rc {
-1 => panic!("Failed to validate file due to internal error"), // TODO error handling
-1 => Err(vec![StructuredError::internal()]),
0 => Ok(()),
_ => Err(self.drain_errors()),
}
Expand All @@ -73,7 +74,7 @@ impl SchemaValidationContext {
let rc = unsafe { bindings::xmlSchemaValidateOneElement(self.ctxt, node.node_ptr()) };

match rc {
-1 => panic!("Failed to validate element due to internal error"), // TODO error handling
-1 => Err(vec![StructuredError::internal()]),
0 => Ok(()),
_ => Err(self.drain_errors()),
}
Expand Down
59 changes: 54 additions & 5 deletions tests/schema_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,12 @@ fn schema_from_string() {
.parse_string(VALID_NOTE_XML)
.expect("Expected to be able to parse XML Document from string");

let mut xsdparser = SchemaParserContext::from_buffer(NOTE_SCHEMA);
let mut xsdparser = SchemaParserContext::from_buffer(NOTE_SCHEMA).unwrap_or_else(|err| {
println!("{}", err.message.as_ref().unwrap());

panic!("Failed to create parsing context");
});

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

if let Err(errors) = xsd {
Expand Down Expand Up @@ -114,7 +119,12 @@ fn schema_from_string_generates_errors() {
.parse_string(INVALID_NOTE_XML)
.expect("Expected to be able to parse XML Document from string");

let mut xsdparser = SchemaParserContext::from_buffer(NOTE_SCHEMA);
let mut xsdparser = SchemaParserContext::from_buffer(NOTE_SCHEMA).unwrap_or_else(|err| {
println!("{}", err.message.as_ref().unwrap());

panic!("Failed to create parsing context");
});

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

if let Err(errors) = xsd {
Expand Down Expand Up @@ -143,8 +153,13 @@ 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 mut xsdparser = SchemaParserContext::from_buffer(STOCK_SCHEMA).unwrap_or_else(|err| {
println!("{}", err.message.as_ref().unwrap());

panic!("Failed to create parsing context");
});

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

if let Err(errors) = xsd {
Expand All @@ -167,8 +182,42 @@ fn schema_from_string_reports_unique_errors() {
"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.as_ref().unwrap() == err_msg), "Expected error message {} was not found", err_msg);
assert!(
errors
.iter()
.any(|err| err.message.as_ref().unwrap() == err_msg),
"Expected error message {} was not found",
err_msg
);
}
}
}
}

#[test]
fn zero_bytes_in_path_are_reported_as_error() {
let mut xsdparser = SchemaParserContext::from_buffer(STOCK_SCHEMA).unwrap_or_else(|err| {
println!("{}", err.message.as_ref().unwrap());

panic!("Failed to create parsing context");
});

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

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

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

let mut xsdvalidator = xsd.unwrap();
let result = xsdvalidator.validate_file("\0");
assert!(result.is_err());
let _ = result.map_err(|err| {
let expected = "Path contained an unexpected null-terminator at position 0. String must not contain any 0-bytes.";
let actual = err.first().unwrap().message.as_ref().unwrap();
assert_eq!(expected, actual)
});
}
Loading