stop panicking in case of internal error

This commit is contained in:
Tamo 2021-11-04 16:20:53 +01:00
parent 3e5550c910
commit 7328ffb034
No known key found for this signature in database
GPG Key ID: 20CD8020AFA88D69
4 changed files with 26 additions and 14 deletions

View File

@ -1,13 +1,18 @@
#![no_main] #![no_main]
use filter_parser::FilterCondition; use filter_parser::{ErrorKind, FilterCondition};
use libfuzzer_sys::fuzz_target; use libfuzzer_sys::fuzz_target;
fuzz_target!(|data: &[u8]| { fuzz_target!(|data: &[u8]| {
if let Ok(s) = std::str::from_utf8(data) { if let Ok(s) = std::str::from_utf8(data) {
// When we are fuzzing the parser we can get stack overflow really easily. // When we are fuzzing the parser we can get a stack overflow very easily.
// But since this doesn't happens with a normal build we are just going to limit the fuzzer to 500 characters. // But since this doesn't happens with a normal build we are just going to limit the fuzzer to 500 characters.
if s.len() < 500 { if s.len() < 500 {
let _ = FilterCondition::parse(s); match FilterCondition::parse(s) {
Err(e) if matches!(e.kind(), ErrorKind::InternalError(_)) => {
panic!("Found an internal error: `{:?}`", e)
}
_ => (),
}
} }
} }
}); });

View File

@ -63,9 +63,14 @@ pub enum ErrorKind<'a> {
} }
impl<'a> Error<'a> { impl<'a> Error<'a> {
pub fn kind(context: Span<'a>, kind: ErrorKind<'a>) -> Self { pub fn kind(&self) -> &ErrorKind<'a> {
&self.kind
}
pub fn new_from_kind(context: Span<'a>, kind: ErrorKind<'a>) -> Self {
Self { context, kind } Self { context, kind }
} }
pub fn char(self) -> char { pub fn char(self) -> char {
match self.kind { match self.kind {
ErrorKind::Char(c) => c, ErrorKind::Char(c) => c,

View File

@ -162,12 +162,12 @@ fn parse_geo_radius(input: Span) -> IResult<FilterCondition> {
// if we were able to parse `_geoRadius` and can't parse the rest of the input we returns a failure // if we were able to parse `_geoRadius` and can't parse the rest of the input we returns a failure
cut(delimited(char('('), separated_list1(tag(","), ws(|c| recognize_float(c))), char(')'))), cut(delimited(char('('), separated_list1(tag(","), ws(|c| recognize_float(c))), char(')'))),
)(input) )(input)
.map_err(|e| e.map(|_| Error::kind(input, ErrorKind::Geo))); .map_err(|e| e.map(|_| Error::new_from_kind(input, ErrorKind::Geo)));
let (input, args) = parsed?; let (input, args) = parsed?;
if args.len() != 3 { if args.len() != 3 {
return Err(nom::Err::Failure(Error::kind(input, ErrorKind::Geo))); return Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::Geo)));
} }
let res = FilterCondition::GeoLowerThan { let res = FilterCondition::GeoLowerThan {
@ -186,9 +186,9 @@ fn parse_geo_point(input: Span) -> IResult<FilterCondition> {
// if we were able to parse `_geoPoint` we are going to return a Failure whatever happens next. // if we were able to parse `_geoPoint` we are going to return a Failure whatever happens next.
cut(delimited(char('('), separated_list1(tag(","), ws(|c| recognize_float(c))), char(')'))), cut(delimited(char('('), separated_list1(tag(","), ws(|c| recognize_float(c))), char(')'))),
))(input) ))(input)
.map_err(|e| e.map(|_| Error::kind(input, ErrorKind::ReservedGeo("_geoPoint"))))?; .map_err(|e| e.map(|_| Error::new_from_kind(input, ErrorKind::ReservedGeo("_geoPoint"))))?;
// if we succeeded we still returns a Failure because geoPoints are not allowed // if we succeeded we still returns a Failure because geoPoints are not allowed
Err(nom::Err::Failure(Error::kind(input, ErrorKind::ReservedGeo("_geoPoint")))) Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::ReservedGeo("_geoPoint"))))
} }
/// primary = (WS* ~ "(" expression ")" ~ WS*) | geoRadius | condition | to /// primary = (WS* ~ "(" expression ")" ~ WS*) | geoRadius | condition | to
@ -199,7 +199,7 @@ fn parse_primary(input: Span) -> IResult<FilterCondition> {
ws(char('(')), ws(char('(')),
cut(parse_expression), cut(parse_expression),
cut_with_err(ws(char(')')), |c| { cut_with_err(ws(char(')')), |c| {
Error::kind(input, ErrorKind::MissingClosingDelimiter(c.char())) Error::new_from_kind(input, ErrorKind::MissingClosingDelimiter(c.char()))
}), }),
), ),
|c| parse_geo_radius(c), |c| parse_geo_radius(c),
@ -209,7 +209,7 @@ fn parse_primary(input: Span) -> IResult<FilterCondition> {
|c| parse_geo_point(c), |c| parse_geo_point(c),
))(input) ))(input)
// if the inner parsers did not match enough information to return an accurate error // if the inner parsers did not match enough information to return an accurate error
.map_err(|e| e.map_err(|_| Error::kind(input, ErrorKind::InvalidPrimary))) .map_err(|e| e.map_err(|_| Error::new_from_kind(input, ErrorKind::InvalidPrimary)))
} }
/// expression = or /// expression = or

View File

@ -15,11 +15,11 @@ pub fn parse_value(input: Span) -> IResult<Token> {
return Err(err); return Err(err);
} }
match parse_geo_radius(input) { match parse_geo_radius(input) {
Ok(_) => return Err(nom::Err::Failure(Error::kind(input, ErrorKind::MisusedGeo))), Ok(_) => return Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::MisusedGeo))),
// if we encountered a failure it means the user badly wrote a _geoRadius filter. // if we encountered a failure it means the user badly wrote a _geoRadius filter.
// But instead of showing him how to fix his syntax we are going to tell him he should not use this filter as a value. // But instead of showing him how to fix his syntax we are going to tell him he should not use this filter as a value.
Err(e) if e.is_failure() => { Err(e) if e.is_failure() => {
return Err(nom::Err::Failure(Error::kind(input, ErrorKind::MisusedGeo))) return Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::MisusedGeo)))
} }
_ => (), _ => (),
} }
@ -45,9 +45,11 @@ pub fn parse_value(input: Span) -> IResult<Token> {
)(input) )(input)
.map(|(s, t)| (s, t.into())) .map(|(s, t)| (s, t.into()))
// if we found nothing in the alt it means the user did not input any value // if we found nothing in the alt it means the user did not input any value
.map_err(|e| e.map_err(|_| Error::kind(input, ErrorKind::ExpectedValue))) .map_err(|e| e.map_err(|_| Error::new_from_kind(input, ErrorKind::ExpectedValue)))
// if we found encountered a failure it means the user really tried to input a value, but had an unmatched quote // if we found encountered a failure it means the user really tried to input a value, but had an unmatched quote
.map_err(|e| e.map_fail(|c| Error::kind(input, ErrorKind::MissingClosingDelimiter(c.char())))) .map_err(|e| {
e.map_fail(|c| Error::new_from_kind(input, ErrorKind::MissingClosingDelimiter(c.char())))
})
} }
fn is_key_component(c: char) -> bool { fn is_key_component(c: char) -> bool {