From 81af6f8efc37c6da8f7b291bab795ea5cf9dc122 Mon Sep 17 00:00:00 2001 From: masumi ryugo <280057467+masumi-ryugo@users.noreply.github.com> Date: Mon, 4 May 2026 11:31:32 +0900 Subject: [PATCH] fix(query-grammar): handle set_field(None) on Exists without panic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `UserInputLeaf::set_field` panicked with "Exist query without a field isn't allowed" when called with `None` on a `UserInputLeaf::Exists`. This was reachable straight from the strict `parse_query(&str)` API, which builds an `Exists` leaf via `alt((range, set, exists, regex, term_or_phrase))` even when no leading `field:` prefix was parsed (`opt(field_name)`), then routes through `leaf.set_field(field_name)` with `field_name == None`. A 4-byte cargo-fuzz repro (`(*'\n`) drove this in single-digit minutes from an empty corpus on `tantivy-query-grammar/main`: thread '' panicked at query-grammar/src/user_input_ast.rs:51:30: Exist query without a field isn't allowed … 7: query_grammar::literal at query-grammar/src/query_grammar.rs:374 Switch the call to preserve the existing field when `None` is passed, matching the no-op intent in `set_default_field` for the same leaf shape. The strict parser still produces the same AST it would have produced with a non-`None` field; the only behavior change is that malformed user-supplied query strings no longer crash the process. Two regression tests in `tests`: - `test_set_field_none_on_exists_does_not_panic` — direct `set_field(None)` call on an `Exists` leaf. - `test_parse_query_fuzz_repro_does_not_panic` — end-to-end via `parse_query("(*'\\n")` on the fuzz repro byte string. The lenient `parse_query_lenient` path was unaffected because it goes through a different conversion that already tolerates the missing field. Co-Authored-By: Claude Opus 4.7 (1M context) --- query-grammar/src/user_input_ast.rs | 39 +++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/query-grammar/src/user_input_ast.rs b/query-grammar/src/user_input_ast.rs index b607e56cd6..2fc1dc08d2 100644 --- a/query-grammar/src/user_input_ast.rs +++ b/query-grammar/src/user_input_ast.rs @@ -47,8 +47,15 @@ impl UserInputLeaf { upper, }, UserInputLeaf::Set { field: _, elements } => UserInputLeaf::Set { field, elements }, - UserInputLeaf::Exists { field: _ } => UserInputLeaf::Exists { - field: field.expect("Exist query without a field isn't allowed"), + // `Exists` requires a field name on the AST, but the caller can + // legitimately pass `None` here when no leading field prefix was + // parsed (the grammar accepts `exists`-shaped sub-productions + // without a preceding `field:`). Preserving the existing field + // matches the no-op intent in `set_default_field` for the same + // leaf and turns what was a release-build panic into a clean + // pass-through. + UserInputLeaf::Exists { field: existing } => UserInputLeaf::Exists { + field: field.unwrap_or(existing), }, UserInputLeaf::Regex { field: _, pattern } => UserInputLeaf::Regex { field, pattern }, } @@ -453,4 +460,32 @@ mod tests { r#"{"type":"bool","clauses":[["must",{"type":"all"}],["should",{"type":"literal","field_name":"title","phrase":"hello","delimiter":"none","slop":0,"prefix":false}]]}"# ); } + + /// Regression test: a 4-byte fuzz repro (`(*'\n`) drove the strict + /// `parse_query` parser into `set_field(None)` on a `UserInputLeaf::Exists` + /// that had been constructed without a leading `field:` prefix, and the + /// `field.expect(...)` panicked with "Exist query without a field isn't + /// allowed". After the fix `set_field(None)` on an `Exists` leaf + /// preserves the existing field, matching the no-op intent in + /// `set_default_field` for the same leaf. + #[test] + fn test_set_field_none_on_exists_does_not_panic() { + let leaf = UserInputLeaf::Exists { + field: "title".to_string(), + }; + let result = leaf.set_field(None); + assert!(matches!( + result, + UserInputLeaf::Exists { ref field } if field == "title" + )); + } + + /// End-to-end regression test driving the exact 4-byte cargo-fuzz repro + /// through the public `parse_query` entry point. Before this patch the + /// call panicked inside `UserInputLeaf::set_field`; after the patch it + /// either succeeds or returns an error, but does not panic. + #[test] + fn test_parse_query_fuzz_repro_does_not_panic() { + let _ = crate::parse_query("(*'\n"); + } }