Skip to content

Commit 4b9a172

Browse files
def-claude
andcommitted
repr: Remove unnecessary parse_uuid guard against a nonexistent uuid panic
`strconv::parse_uuid` (#36984) rejected inputs shorter than 32 chars or non-ASCII before handing them to the `uuid` crate, on the premise that `Uuid::parse_str` panics (rather than erroring) while building its parse error for some malformed inputs by slicing on a non-char boundary. That premise is false for the version we lock (uuid 1.19.0) and for every release back to 0.8.2: `Uuid::parse_str` returns a clean `Err` for all such inputs (`{}`, `{\0}`, non-ASCII, etc.). `InvalidUuid::into_err` rejects non-UTF8 up front, strips the `{}`/`urn:uuid:` wrappers on raw bytes (always standalone ASCII, so char-safe), and its one subtraction can't underflow. So the guard only suppressed the `uuid` crate's own (more detailed) error message for short/non-ASCII input, and the `value_decode_text` regression test asserted an invariant the crate already upholds. No saved cargo-fuzz artifact reaches a uuid panic. Restore the original parser and drop the vacuous test. Mirrors removing the equivalent persist `parse_id`/`ShardId::from_str` guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 00d8a22 commit 4b9a172

2 files changed

Lines changed: 6 additions & 29 deletions

File tree

src/repr/src/strconv.rs

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -731,15 +731,7 @@ where
731731
}
732732

733733
pub fn parse_uuid(s: &str) -> Result<Uuid, ParseError> {
734-
let trimmed = s.trim();
735-
// The `uuid` crate panics while constructing a parse error for some short,
736-
// brace-wrapped inputs (e.g. `{}`, `{\0}`) because it mis-slices the input.
737-
// Reject anything that can't be a UUID before handing it to the crate. The
738-
// shortest valid form is 32 hex digits and every form is ASCII.
739-
if trimmed.len() < 32 || !trimmed.is_ascii() {
740-
return Err(ParseError::invalid_input_syntax("uuid", s));
741-
}
742-
trimmed
734+
s.trim()
743735
.parse()
744736
.map_err(|e| ParseError::invalid_input_syntax("uuid", s).with_details(e))
745737
}
@@ -2206,19 +2198,4 @@ mod tests {
22062198
let s = format!("{}{}", "x".repeat(62), "א");
22072199
assert_eq!("x".repeat(62), parse_pg_legacy_name(&s));
22082200
}
2209-
2210-
#[mz_ore::test]
2211-
fn test_parse_uuid_rejects_non_uuid_input() {
2212-
// The `uuid` crate panics while constructing a parse error for some
2213-
// short, brace-wrapped inputs (e.g. `{}`, `{\0}`). `parse_uuid` must
2214-
// reject anything that can't be a UUID as an error, never panic.
2215-
// Regression for the value_decode_text cargo-fuzz finding.
2216-
for s in ["", "{}", "{\0}", "{", "}", "xyz", "{ }", "\u{0}\u{0}\u{0}"] {
2217-
assert!(parse_uuid(s).is_err(), "parse_uuid({s:?}) should be Err");
2218-
}
2219-
// Valid UUIDs (and their accepted forms) still parse.
2220-
assert!(parse_uuid("00000000-0000-0000-0000-000000000000").is_ok());
2221-
assert!(parse_uuid("{00000000-0000-0000-0000-000000000000}").is_ok());
2222-
assert!(parse_uuid("00000000000000000000000000000000").is_ok());
2223-
}
22242201
}

src/storage-types/src/sources/casts.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -818,13 +818,13 @@ mod tests {
818818

819819
#[mz_ore::test]
820820
fn error_uuid() {
821-
// `parse_uuid` rejects anything too short to be a UUID before the
822-
// `uuid` crate runs (the crate panics building an error for some
823-
// short inputs), so the error carries no crate-specific detail.
824-
// This matches Postgres, whose message for `'bad'::uuid` has none.
825821
assert_eq!(
826822
eval_cast_err(CastFunc::CastStringToUuid, "bad"),
827-
parse_err("uuid", "bad"),
823+
parse_err_with_details(
824+
"uuid",
825+
"bad",
826+
"invalid length: expected length 32 for simple format, found 3"
827+
),
828828
);
829829
}
830830

0 commit comments

Comments
 (0)