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

Add recursive data cost to fuzzer + speed up deserialize_any identifier check #506

Merged
merged 2 commits into from
Sep 17, 2023
Merged
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: 2 additions & 6 deletions fuzz/fuzz_targets/bench/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1389,8 +1389,6 @@ impl<'a> SerdeDataType<'a> {
&mut self,
u: &mut Unstructured<'u>,
) -> arbitrary::Result<SerdeDataValue<'u>> {
let u_len = u.len();

let mut name_length: usize = 0;

let value = match self {
Expand Down Expand Up @@ -1558,10 +1556,8 @@ impl<'a> SerdeDataType<'a> {
let _ = u.arbitrary::<bool>()?;
}

if u.len() == u_len {
// Enforce that producing a value is never free
let _ = u.arbitrary::<bool>()?;
}
// Enforce that producing a value or adding a level is never free
let _ = u.arbitrary::<bool>()?;

Ok(value)
}
Expand Down
6 changes: 2 additions & 4 deletions src/de/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,10 +320,8 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
return visitor.visit_f64(std::f64::NAN);
}

// `identifier` does not change state if it fails
let ident = self.parser.identifier().ok();

if let Some(ident) = ident {
// `skip_identifier` does not change state if it fails
if let Some(ident) = self.parser.skip_identifier() {
self.parser.skip_ws()?;

return self.handle_any_struct(visitor, Some(ident));
Expand Down
22 changes: 12 additions & 10 deletions src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ impl<'a> Parser<'a> {

parser.skip_ws()?;

if parser.skip_identifier() {
if parser.skip_identifier().is_some() {
parser.skip_ws()?;

match parser.peek_char() {
Expand Down Expand Up @@ -817,7 +817,7 @@ impl<'a> Parser<'a> {
Ok(value)
}

pub fn skip_identifier(&mut self) -> bool {
pub fn skip_identifier(&mut self) -> Option<&'a str> {
#[allow(clippy::nonminimal_bool)]
if self.check_str("b\"") // byte string
|| self.check_str("b'") // byte literal
Expand All @@ -828,30 +828,32 @@ impl<'a> Parser<'a> {
|| self.check_str("r##") // raw string
|| false
{
return false;
return None;
}

if self.check_str("r#") {
// maybe a raw identifier
let len = self.next_chars_while_from_len(2, is_ident_raw_char);
if len > 0 {
let ident = &self.src()[2..2 + len];
self.advance_bytes(2 + len);
return true;
return Some(ident);
}
return false;
return None;
}

if let Some(c) = self.peek_char() {
// maybe a normal identifier
if is_ident_first_char(c) {
self.advance_bytes(
c.len_utf8() + self.next_chars_while_from_len(c.len_utf8(), is_xid_continue),
);
return true;
let len =
c.len_utf8() + self.next_chars_while_from_len(c.len_utf8(), is_xid_continue);
let ident = &self.src()[..len];
self.advance_bytes(len);
return Some(ident);
}
}

false
None
}

pub fn identifier(&mut self) -> Result<&'a str> {
Expand Down
19 changes: 19 additions & 0 deletions tests/438_rusty_byte_strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,3 +463,22 @@ fn byte_literal() {
})
);
}

#[test]
fn invalid_identifier() {
#[allow(dead_code)]
#[derive(Debug, Deserialize)] // GRCOV_EXCL_LINE
struct Test {
a: i32,
}

for id in ["b\"", "b'", "br#", "br\"", "r\"", "r#\"", "r##"] {
assert_eq!(
ron::from_str::<Test>(&format!("({}: 42)", id)).unwrap_err(),
SpannedError {
code: Error::ExpectedIdentifier,
position: Position { line: 1, col: 2 },
}
);
}
}