Sourced from insta's releases.
1.48.0
Release Notes
- Add
strip_ansi_escape_codessetting which removes ANSI escape sequences (color codes, cursor movement, etc.) from snapshot content before comparison. Requires thefiltersfeature. #899 (@pierluigilenoci)- Add opt-in support for YAML literal blocks for multiline strings in snapshot metadata fields such as
descriptionandexpression. SetINSTA_YAML_BLOCK_STYLE=1to enable. #851 (@ivov)- Setting
CI=truenormally makescargo insta testbehave as though--checkwas passed. Explicit snapshot handling options such as--acceptnow take precedence over this environment variable, allowing users to override this behavior if they want to. #924- Fix
cargo insta test --profilebeing forwarded to nextest as the nextest profile instead of the cargo build profile; it now translates to--cargo-profilefor the nextest runner. Add--nextest-profileto select the nextest profile. #910- Fix
cargo insta pending-snapshotsprinting unusable\\?\-prefixed paths on Windows. The--snapshotfilter now also accepts partial paths: any trailing path suffix of the snapshot file matches, so a bare--snapshot my_test.snapworks. #904- Accepting a binary snapshot no longer fails with
os error 2when its data file is missing (e.g. gitignored and not committed). #914Install cargo-insta 1.48.0
Install prebuilt binaries via shell script
curl --proto '=https' --tlsv1.2 -LsSf https://github.com/mitsuhiko/insta/releases/download/1.48.0/cargo-insta-installer.sh | shInstall prebuilt binaries via powershell script
powershell -ExecutionPolicy Bypass -c "irm https://github.com/mitsuhiko/insta/releases/download/1.48.0/cargo-insta-installer.ps1 | iex"Download cargo-insta 1.48.0
File Platform Checksum cargo-insta-aarch64-apple-darwin.tar.xz Apple Silicon macOS checksum cargo-insta-x86_64-apple-darwin.tar.xz Intel macOS checksum cargo-insta-x86_64-pc-windows-msvc.zip x64 Windows checksum cargo-insta-x86_64-unknown-linux-gnu.tar.xz x64 Linux checksum cargo-insta-x86_64-unknown-linux-musl.tar.xz x64 MUSL Linux checksum
Sourced from insta's changelog.
1.48.0
- Add
strip_ansi_escape_codessetting which removes ANSI escape sequences (color codes, cursor movement, etc.) from snapshot content before comparison. Requires thefiltersfeature. #899 (@pierluigilenoci)- Add opt-in support for YAML literal blocks for multiline strings in snapshot metadata fields such as
descriptionandexpression. SetINSTA_YAML_BLOCK_STYLE=1to enable. #851 (@ivov)- Setting
CI=truenormally makescargo insta testbehave as though--checkwas passed. Explicit snapshot handling options such as--acceptnow take precedence over this environment variable, allowing users to override this behavior if they want to. #924- Fix
cargo insta test --profilebeing forwarded to nextest as the nextest profile instead of the cargo build profile; it now translates to--cargo-profilefor the nextest runner. Add--nextest-profileto select the nextest profile. #910- Fix
cargo insta pending-snapshotsprinting unusable\\?\-prefixed paths on Windows. The--snapshotfilter now also accepts partial paths: any trailing path suffix of the snapshot file matches, so a bare--snapshot my_test.snapworks. #904- Accepting a binary snapshot no longer fails with
os error 2when its data file is missing (e.g. gitignored and not committed). #914
7f23d2e
Release 1.48.0 (#925)ee9cae1
Allow CI=true to be overridden by an explicitly passed
--accept CLI flag ...043cf82
fix: translate --profile to --cargo-profile for nextest (#913)9c77f13
test: cover deep-wildcard redaction through arrays (#915)362f432
Fix --snapshot filter on Windows; allow partial paths (#904)a436836
fix: tolerate a missing binary snapshot data file (#914)bf5fcdf
fix: regenerate Cargo.lock and guard it with --locked in CI (#912)a761a9c
feat: Support YAML literal blocks for multiline strings (#851)f9633f3
ci: pin check-minver to nightly-2026-04-25 (#905)c7b98b8
feat: add strip_ansi_escape_codes setting (#899)` blocks
- - Links to known flaky test issues (look for GitHub issue links like `database-issues/#NNNN`)
- - Main branch history showing if this test passes on main (flaky test indicator)
-
-Parse the error annotations to get a quick overview of all failures before fetching any logs.
-
-## Step 4: Fetch logs when needed
-
-Only fetch full logs when annotations don't provide enough detail. Triage in this order:
-
-1. **clippy** — compilation/lint errors that often explain everything
-2. **lint-and-rustfmt** — formatting and lint-check failures
-3. **cargo-test** — unit/integration test failures
-4. **fast-sql-logic-tests** — SLT failures
-5. **testdrive** — integration test failures (often cascading)
-6. **Everything else** (checks-parallel, cluster-tests, dbt, etc.)
-
-To fetch a job's log:
-```bash
-bk job log -p -b --no-timestamps --no-pager 2>/dev/null | tail -100
-```
-
-For large logs, first grep for errors to find the relevant section:
-```bash
-bk job log -p -b --no-timestamps --no-pager 2>/dev/null | grep -B2 -A5 'error\|FAIL\|panicked'
-```
-
-Fetch multiple job logs in parallel when they are independent (e.g., clippy + lint at the same time).
+Failing rows link to
+`https://buildkite.com/materialize//builds/#`,
+the identifiers used throughout this skill.
+
+### Listing a build's failed jobs directly
+
+Buildkite mirrors each job's result into GitHub as a
+`buildkite//` commit status (never-dispatched jobs post
+nothing), but `gh pr checks` reads the PR's current head commit only. A build
+therefore shows up there only if it ran on exactly that commit. For everything
+else (scheduled nightlies on main, release-qualification builds, a PR nightly
+from before the latest push), list the genuinely failed jobs via the API:
+`list_jobs` as described in the MCP section, or with `bk` the listing recipe
+in `bk-fallback.md`. Either way, also grab the build's commit and branch:
+Step 5's known-vs-new checks need the commit, and neighboring-build lookups
+need the branch.
+
+Job-state semantics matter here:
+- The failure states are `failed` and `timed_out`, the same set as
+ `BUILDKITE_RELEVANT_FAILED_BUILD_STEP_STATES` in
+ `misc/python/materialize/buildkite_insights/buildkite_api/buildkite_constants.py`.
+- `broken` is not a failure. Buildkite marks a job `broken` when its
+ configuration prevents it from running, for example a branch filter or an
+ `if:` condition that evaluates false, so the job never dispatches. Every
+ build contains broken jobs (a routine main nightly has dozens), so counting
+ them massively over-counts failures.
+- Jobs with `soft_failed: true` are allowed to fail without making the build
+ red; no current CI step sets it, so filtering it is defensive.
+- `waiting_failed` is not a failure either: the job's dependency (usually a
+ build step) failed, so it never started. Its presence is informative,
+ though. A PR build full of `waiting_failed` jobs ran no tests at all, so
+ report that fixing the visible failures is necessary but no proof the
+ build then goes green.
+- Retried jobs (automatic or manual) appear only as their latest attempt, so
+ a job that failed and then passed on retry counts as passed. That is the
+ right verdict for build triage; fetch the earlier attempts only when an
+ in-depth investigation needs them. A manual retry that failed again is
+ itself a signal: someone already tried to shake the failure off and it
+ reproduced.
+
+To find a PR's builds that are not on its current head in the first place (an
+older nightly, a pre-push run), get the branch with
+`gh pr view --json headRefName` and filter Buildkite builds by
+branch; the MCP's `list_builds` takes a `branch:` filter, and fork branches
+are named `:` on Buildkite.
+
+## Step 2: Check annotations first
+
+**Before diving into logs**, fetch the build annotations. They contain pre-extracted error messages, stack traces, and links to known flaky test issues — this saves significant time compared to grepping through raw logs. With the MCP they arrive in `get_build_failure_summary` (or `list_annotations`); with `bk`, use the annotations recipe in `bk-fallback.md`.
+
+Besides the error itself, an error annotation (`style` `"error"`) carries
+known-issue links (Linear keys like `CPU-170`, or legacy
+`database-issues/#NNNN`) and the job's main-branch history, a flaky-test
+indicator.
+
+Things to know when reconciling annotations against jobs:
+
+- Error annotations persist from failed attempts even when a retry later
+ passed, so a build can have more error annotations than failed jobs. A
+ `bin/ci-annotate-errors` annotation's `context` field is `-error`,
+ and that job id can belong to a retried attempt that the default job
+ listing does not contain. (Other producers use other contexts, e.g.
+ `images-not-public` from the build step.)
+- Only jobs running through the mzcompose/cloudtest plugins get annotations
+ and `bin/ci-failures` rows (both come from `bin/ci-annotate-errors`). Plain
+ shell steps (lint, Security advisories, and similar checks) produce neither,
+ so a real failure there has no annotation at all. Go straight to its job
+ log.
+- Info-style annotations (" succeeded with known error logs") mean the
+ job passed and every logged error matched an open known issue; they are
+ not failures. When a job went red via "Test succeeded, but unknown errors
+ found in logs" (see Step 3), the unknown and potential-regression entries
+ in its annotation are what turned it red. The reverse also exists: a
+ failed job turns green (" would have failed with known error logs")
+ when all its logged errors match issues marked `ci-ignore-failure: true`.
+
+## Step 3: Fetch logs when needed
+
+Only fetch full logs when annotations don't provide enough detail. On PR test
+builds, read compile and lint job logs first (clippy, lint-and-rustfmt): they
+often explain every downstream failure.
+
+Fetch and search logs with the MCP's `search_logs`/`read_logs`/`tail_logs`,
+or with `bk` use the job-log recipes in `bk-fallback.md`. The log tail can be a
+red herring: some jobs print long non-error output after the actual error
+(cargo deny's dependency tree runs hundreds of lines past the advisory), so
+when the tail shows no error, search or grep for the error instead of
+tailing more.
+
+NOTE: for mzcompose-based jobs (testdrive, SQLsmith/SQLancer, platform checks,
+...) the job console log holds only the harness's output. The services' own
+output (environmentd/clusterd panics and errors) goes to `services.log` and
+similar files inside the job's log artifacts. A panic that is absent from
+the job's console log can still be the failure, so never conclude "this
+error did not happen in this run" from it alone. Check the annotations or
+`bin/ci-failures` instead, both are fed by `bin/ci-annotate-errors`, which
+scans the uploaded log artifacts (`services.log`, `run.log`, junit XML, ...)
+at the end of each job. The same scan can fail a job whose own workflow
+passed: the console log then reads `Test succeeded, but unknown errors found
+in logs, marking as failed`, and the cause is whatever the scan found in the
+artifacts, not a test failure. Alternatively, download the log artifacts and
+grep them directly.
### Artifacts
Jobs upload artifacts (junit XML, service logs, coredumps, ...). Use the
-`bk artifacts` subcommands:
-
-```bash
-# All artifacts of a build (each entry has id, job_id, path)
-bk artifacts list -p --no-pager 2>/dev/null
-# Artifacts of a single job
-bk artifacts list -p --job-uuid --no-pager 2>/dev/null
-# Download one artifact into the current directory
-bk artifacts download --build -p
-```
-
-Do not use `bk api` for artifacts. The plural
-`/pipelines//builds//artifacts` endpoint works but
-silently returns only the first 30 entries, and its `.../download` endpoint
-fails to parse.
+MCP's `list_artifacts_for_build`/`list_artifacts_for_job`/`get_artifact`,
+or with `bk` use the artifacts recipes in `bk-fallback.md`.
### Shard contents
@@ -136,47 +204,84 @@ shows it:
```bash
# Mapping of every sharded job to what it ran, with a link per job whose
-# `#` fragment is the `` for `bk job log` (omit links with --no-url)
+# `#` fragment is the `` for fetching job logs (omit links with --no-url)
bin/ci-shards https://buildkite.com/materialize//builds/
-# What a single job ran (job link as copied from the Buildkite UI)
-bin/ci-shards 'https://buildkite.com/materialize//builds/#'
-
# Which job(s) ran a specific item (test file, scenario, or workflow,
# `workflow_foo_bar` function names are matched as `foo-bar`)
bin/ci-shards https://buildkite.com/materialize//builds/ numeric.td
```
-## Step 5: Categorize failures
+## Step 4: Categorize failures
Use these Materialize-specific patterns to diagnose:
### Clippy errors
-Code lint issues in changed files. Common ones: `as_conversions`, `needless_borrow`, `clone_on_ref_ptr`. Fix the code, not the lint config.
+Code lint issues in changed files. Fix the code, not the lint config.
### `check-test-flags` lint failure
A new configuration flag was introduced but not registered in the required places:
- `misc/python/materialize/parallel_workload/action.py` (FlipFlagsAction)
- `misc/python/materialize/mzcompose/__init__.py` (get_variable_system_parameters / get_minimal_system_parameters / UNINTERESTING_SYSTEM_PARAMETERS)
-### Cargo test failures
-Read the panic message or assertion diff. Common patterns:
-- `unwrap_err() on Ok` → test expected an error but the code now succeeds
-- `assertion left == right failed` → behavioral change in output
-
### Testdrive cascades
After one test crashes environmentd, all subsequent tests in that shard fail with `Name or service not known` or `connection closed`. **Only the first failure in a shard matters** — everything after it is a cascade. Look for the first `error:` or `FAIL` in the log.
Testdrive shards with the same number (e.g., `testdrive-10` and `testdrive-with-alloydb-10`) run the same tests — if both fail, it's likely to be the same root cause.
### SLT failures
-Check whether it's wrong output (behavioral change) vs. connection error (crash/timeout). Wrong output means the query semantics changed.
-
-## Step 6: Summarize
-
-Group failures by **root cause**, not by job name. Typically many failing jobs share just 1-2 root causes. Present the summary as:
-
-1. **Root cause A** — description, which jobs it affects, what to fix
-2. **Root cause B** — description, which jobs it affects, what to fix
-
-Distinguish between issues that are clearly caused by the PR's changes vs. pre-existing flaky tests. The annotations often link to known flaky test issues (GitHub `database-issues` links) — use these to identify pre-existing flakes vs. regressions introduced by the PR.
+Check whether it's wrong output (behavioral change) vs. connection error (crash/timeout). Wrong output means the query semantics changed. An `InconsistentViewOutcome` failure comes from CI's `--auto-index-selects` mode; the mz-test skill explains the mechanism and the exemption lists.
+
+### Timeouts
+A `timed_out` job hit its step budget (`timeout_in_minutes` in
+`ci//pipeline.template.yml`). The annotation records only the fact
+("test timed out"), never the cause. Find the cause in the console log: the
+last completed unit of work, plus anything marked as still running when the
+job was canceled (nextest prints `SLOW [>1200.000s]` lines naming the stuck
+tests). If no such marker appears, compare wall-clock against the job's last
+passing run to tell a gradually outgrown budget from a hung test.
+
+### Rarer classes
+
+If the failure matches one of these, read `rare-failures.md` in this
+skill's directory for its triage pattern: `New regression against
+` (feature benchmark), a cargo-fuzz crash, a Miri failure, a
+limits-test failure, or `ImagesNotPublicError`.
+
+## Step 5: Summarize
+
+Group failures by **root cause**, not by job name — typically many failing
+jobs share just 1-2 root causes. Per root cause, report a description, the
+affected jobs, and what to fix.
+
+Distinguish between issues that are clearly caused by the change under test vs. pre-existing flaky tests. The annotations often link to known flaky test issues — use these to identify pre-existing flakes vs. new regressions.
+
+More ways to establish known vs new:
+
+- Check whether main already fixed it. Any build can run code that is behind
+ main: a nightly investigated the morning after, but also a fresh PR based
+ on an older main. From an up-to-date main checkout run
+ `git log ..HEAD -- ` or
+ `git log -S ''`. A fix can also sit in a not-yet-merged PR,
+ invisible to git log: look at recently opened PRs for one that already
+ addresses the failure (a PR's file list shows which of the build's root
+ causes it covers). When citing a later build as evidence
+ of a fix, confirm the specific job's state there is `passed`: a build can
+ be green because the job was `broken` and never ran.
+- Known-issue tracking lives in Linear. Annotations and `bin/ci-failures`
+ cite issue keys like `CPU-170` or `SS-361`, which resolve to
+ `https://linear.app/materializeinc/issue/`. Legacy references point at
+ GitHub `MaterializeInc/database-issues` instead.
+- Annotations and `bin/ci-failures` classify a failure three ways; for the
+ latter two, check the signature's history with `bin/ci-failures` before
+ treating the failure as new. A known-issue link means it matched an open
+ tracked issue. `"UNKNOWN ERROR"` means it matched no tracked issue, not
+ that the failure is new: a long streak under that label is a chronic
+ untracked flake worth filing. An issue reference tagged
+ `(POTENTIAL REGRESSION)` means it matched a tracked issue that is now
+ closed: a known signature with stale tracking, not by itself evidence of
+ a new regression; suggest to the user that the closed issue may need
+ reopening. The issue matching is a content heuristic and often wrong, so
+ verify that the matched issue actually describes the problem: search the
+ tracker for the error text, since the right issue may already exist open,
+ or the problem may need a totally new one.
diff --git a/.agents/skills/mz-debug-ci/bk-fallback.md b/.agents/skills/mz-debug-ci/bk-fallback.md
new file mode 100644
index 0000000000000..9756f627c39e9
--- /dev/null
+++ b/.agents/skills/mz-debug-ci/bk-fallback.md
@@ -0,0 +1,67 @@
+# bk fallback for mz-debug-ci
+
+Command recipes for sessions without the Buildkite MCP. The semantics (job
+states, annotation contents, triage rules) live in SKILL.md; only the `bk`
+mechanics live here.
+
+`bk` writes warnings to stderr, including a benign `BUILDKITE_API_TOKEN is
+overriding the credential stored for this organization` on healthy setups.
+Pipe `bk` output with `2>/dev/null`, never `2>&1`, or the noise corrupts the
+JSON.
+
+## Listing a build's failed jobs
+
+```bash
+bk api "/pipelines//builds/" --no-pager 2>/dev/null \
+ | jq -r '.jobs[] | select(.state=="failed" or .state=="timed_out") | select(.soft_failed != true) | [.state, .id, .name] | @tsv'
+```
+
+The same response carries the build's `.commit` and `.branch`. Add
+`?include_retried_jobs=true` to see the earlier attempts of retried jobs
+(exposes `.retried` and `.retry_source`).
+
+## Annotations
+
+```bash
+bk api /pipelines//builds//annotations --no-pager 2>/dev/null \
+ | jq -r '.[] | select(.style=="error") | "=== \(.context)\n\(.body_html)"' \
+ | sed -e 's/<[^>]*>//g' | grep -v '^$'
+```
+
+## Job logs
+
+```bash
+bk job log -p -b --no-timestamps --no-pager 2>/dev/null | tail -100
+```
+
+For large logs, grep for the error instead of tailing. Buildkite logs are
+full of ANSI escapes (colors, plus cursor-movement and OSC sequences from
+docker pulls) and embedded carriage returns that leave doubled blank lines,
+halving grep's `-A`/`-B` reach. Normalize first:
+
+```bash
+bk job log ... 2>/dev/null | tr '\r' '\n' \
+ | sed -e 's/\x1b\[[0-9;]*[a-zA-Z]//g' -e 's/\x1b\][^\x07]*\x07//g' | grep -v '^$' \
+ | grep -B2 -A5 'error\|FAIL\|panicked'
+```
+
+## Artifacts
+
+```bash
+# All artifacts of a build (each entry has id, job_id, path)
+bk artifacts list -p --no-pager 2>/dev/null
+# Artifacts of a single job
+bk artifacts list -p --job-uuid --no-pager 2>/dev/null
+# Download one artifact into the current directory
+bk artifacts download --build -p
+```
+
+NOTE: if `bk` rejects one of these flags as unknown, the installed `bk`
+predates the April 2026 artifacts rework. Check
+`bk artifacts --help` for the local spelling, and recommend to
+the user that they upgrade `bk`.
+
+Do not use `bk api` for artifacts. The plural
+`/pipelines//builds//artifacts` endpoint works but
+silently returns only the first 30 entries, and its `.../download` endpoint
+fails to parse.
diff --git a/.agents/skills/mz-debug-ci/rare-failures.md b/.agents/skills/mz-debug-ci/rare-failures.md
new file mode 100644
index 0000000000000..4c025116aca95
--- /dev/null
+++ b/.agents/skills/mz-debug-ci/rare-failures.md
@@ -0,0 +1,51 @@
+# Rare failure classes for mz-debug-ci
+
+Triage patterns for failure classes that come up rarely. SKILL.md's Step 4
+points here by keyword.
+
+## Feature benchmark regressions
+
+A `New regression against ` annotation shows one comparison table,
+but the harness re-runs every flagged scenario and annotates only the last
+run: grep the console log for `Run [0-9] with scenarios` and `!!YES!!` to
+see whether the scenario survived every re-run. Even surviving all re-runs
+does not prove a code cause, since agent-level speed offsets are stable
+within a job, and these scenarios trip on noise regularly, so check the
+base rate with `bin/ci-failures ''`. The decisive check is
+reproducing locally: check out the exact commit the build ran and run the
+reproducer command from the annotation. mzbuild pulls the images CI already
+published for that commit, so nothing needs to build. Pass `--release` for
+nightly and release-qualification builds (they build with `CI_LTO=1`); PR
+test builds and flag-less local runs both use the optimized profile, so
+no flag there.
+
+## cargo-fuzz crashes
+
+The fuzz target's own panic text is not in the console log; it is in
+`.log` inside the `fuzz-logs.tar.zst` build artifact, and the
+annotation names the file. For known-vs-new use
+`bin/ci-failures ''`; cargo-fuzz runs only in release
+qualification, so that is its entire history.
+
+## Limits test failures
+
+For red `Product limits` / `Instance size limits` jobs, the mz-limits-test
+skill explains the framework, its typical failure modes, and
+single-scenario local reruns.
+
+## Miri failures
+
+`:rust: Miri test (full)` fails as either `timed_out` (a test too slow under
+Miri's interpreter) or `failed` with `unsupported operation` (code Miri
+cannot execute, e.g. a foreign function). Neither is a product bug, and the
+standard reaction is not to make the code Miri-compatible: add
+`#[cfg_attr(miri, ignore)]` with a short reason to the offending test, as
+about a thousand existing tests do.
+
+## `ImagesNotPublicError`
+
+A PR adding a new publishable mzbuild image fails `:rust: Build x86_64`
+until the image's DockerHub/GHCR repos are made public, a process step
+described by the `images-not-public` annotation. The check runs only on the
+x86_64 build, so aarch64 passing while x86_64 fails is expected, not an
+arch bug.
diff --git a/.agents/skills/mz-test/SKILL.md b/.agents/skills/mz-test/SKILL.md
index dbaf40b616f34..126a70d0d9e8a 100644
--- a/.agents/skills/mz-test/SKILL.md
+++ b/.agents/skills/mz-test/SKILL.md
@@ -198,6 +198,9 @@ If a flake cannot be reproduced locally, suggest the user trigger CI runs manual
Removing the "Target Branch" generates a random identifier, allowing parallel runs.
10-20 runs to reproduce a flake is fine.
+To check whether a failure is already known and chronic in CI, see the
+mz-debug-ci skill's `bin/ci-failures` search.
+
## Additional logging for flaky tests
Add logging through `log_filter` in the test's `mzcompose.py`:
From 34a619c51ee2f2e6e1dcfb8563d50bc908d48c2b Mon Sep 17 00:00:00 2001
From: Dennis Felsing
Date: Wed, 19 Aug 2026 12:06:47 +0200
Subject: [PATCH 5/5] tests: sharpen the cargo-fuzz oracles across the
workspace (#37979)
The recurring theme is oracles that could pass while the behaviour they
name is broken, generators that stall before the decode they exist to
drive, and coverage the module docs claimed but never reached.
Issues these sharpened oracles found, fixed separately:
* MaterializeInc/materialize#37971, persist
* MaterializeInc/materialize#37972, avro
* MaterializeInc/materialize#37973, sql-parser
* MaterializeInc/materialize#37974, expr
* MaterializeInc/materialize#37975, repr, proto decode
* MaterializeInc/materialize#37976, repr, regex
* MaterializeInc/materialize#37977, repr, strconv
* MaterializeInc/materialize#37978, pgrepr
* MaterializeInc/materialize#37993, sql
Formatting and tooling:
* `bin/fmt` skipped the fuzz crates entirely. `cargo metadata` only
reports the workspace it is pointed at, and the `src/*/fuzz` crates
attach to `test/cargo-fuzz`, which the root workspace does not include.
Run it over both manifests, keyed by edition, since `gen` is an
identifier in 2021 but a reserved keyword in 2024 and rustfmt cannot
parse a file at the wrong edition.
* Pin `style_edition = "2024"` in rustfmt.toml. Formatting style is
repo-wide and independent of the edition a crate parses as, but
overriding `edition` per package would otherwise drag the 2021
import-sorting style along with it.
Runner and profiles:
* Release qualification runs `--profile all`, not `--profile fruitful`.
A target nobody runs is a target whose oracle can quietly stop asserting
anything, and the 24h budget is the only place the low-yield targets get
exercised at all. The profile docs said the opposite.
* Pair the two durable-state decoders (rollup = full snapshot, state
diff = incremental) in `fruitful`. Both are read from blob/consensus on
every state load, so a decode panic there makes the shard unloadable.
* Add `strconv_parse_interval` to `fruitful`. Every other hand-written
strconv text parser is in the "untrusted text/bytes parsers" bucket, so
what that file's own doc calls "the most intricate parser in strconv"
was the one omission, and did not run under the 24h
release-qualification profile at all.
* Record that ASan does not cover the cc-built C dependencies. The
justification for defaulting `--sanitizer=none` claimed our targets
cannot find memory-corruption bugs. Some can: `ProtoNumeric` decode
hands untrusted bytes to libdecnumber's `decPackedToNumber`, which
writes past the coefficient it is given. Keep the default, because ASan
does not help there either. cargo-fuzz instruments through RUSTFLAGS
while the C is compiled by the `cc` crate without them, so the FFI
writes stay invisible unless CFLAGS carry `-fsanitize=address` too. Say
so, so the next reader does not read the old wording as "no target can
hit memory corruption".
mz-avro `schema_resolve`:
* Require the resolutions documented as infallible instead of guarding
them. Identity and writer->reader resolution succeed by construction, so
`if let Ok(..)` meant a promotion that regressed at a non-union position
came back as a top-level `Err` and switched the decode oracle off for
exactly the inputs that found it. Deleting `ResolveIntDouble` from
`reader.rs` now fails within seconds; before it left the target green.
* Draw the encoded union branch from the input. Decoding consults only
the resolved permutation entry for the branch on the wire, so the fixed
first-promotable choice left every other entry's deferred `Err`
unobserved, the exact failure mode this target exists for.
* Reach the other two union arms: the reader rendering now sometimes
collapses a union to the encoded branch (`ResolveUnionConcrete`) and
sometimes wraps a concrete node in one (`ResolveConcreteUnion`), whose
no-match error path has a fixed out-of-bounds history. Unions also carry
named variants now, for the name-keyed side of variant matching.
* Drop trailing reader enum symbols so the `default` becomes
load-bearing, and require each decode to consume every byte the writer
wrote.
mz-avro `reader_decode`:
* Straddle `safe_len` deliberately. All three "corrupt framing"
magnitudes sat above `MAX_ALLOCATION_BYTES`, so every one of them
stopped at that guard: the short-read case was dead and duplicated the
negative-size case, and the accepted-but-oversized count band, where the
bug this target is credited with finding still lived, was never
generated.
* Stop generating schemas the parser rejects outright (`fixed` size 0,
`["null","null"]`, nested unions), which discarded \~6% of inputs at the
header with no block decoding at all.
mz-avro `avro_schema_parse`:
* Reach `MAX_SCHEMA_DEPTH`. It was unreachable through
`Schema::from_str`: serde_json's own 128-container limit fires one level
first, so the target's headline claim of driving the guard past its
limit was never true. The deep case now goes through `Schema::parse`,
and the text case is bounded to nesting serde_json accepts. Adds the
unit test the depth guard never had, against a hand-built `Value`, plus
one pinning why the text entry point cannot reach it.
* Generate two named types sharing a fullname, the only shape that
reaches `alloc_name`'s duplicate rejection.
* Correct the doc: two of the "must reject" shapes are accepted today
(`parse_record` has no duplicate-field-name check, and `parse_bytes`
swallows a decimal `scale > precision`), and alias collisions mean
nothing until `resolve_schemas`, which this target never calls.
* Seeds are prefixed with the byte that selects `reader_decode`'s
raw-bytes branch. `reader_decode` reads its input as an `Unstructured`
recipe, so the seeded container file was being consumed as generator
choices and never reached the magic-header check that
`prepare-corpus.sh` exists to skip past.
Catalog serde round-trip:
* Route the oracle through the durable encoding. The on-disk form is not
JSON text: `StateUpdateKindJson` packs the serde value into a `Jsonb`,
i.e. an `mz_repr::Row`, and reads it back through `to_serde_json` +
`from_value`. That leg is the one that can lose information, since every
number becomes a `Datum::Numeric` and object keys are deduplicated and
reordered by the `Row` map encoding, and `to_vec`/`from_slice` never
touches it. The text form is still asserted alongside it.
* Add `StateUpdateKind`, the durable envelope every catalog write goes
through and the only member of this family whose round trip is not
trivially total: it is `#[serde(tag = "kind")]`, so it deserializes
through serde's content-buffering path. Every other reachable field
bottoms out in integers, bools, strings, `Vec`, `Option`, or fieldless
enums, for which the derived `Serialize`/`Deserialize` are inverses by
construction.
* Drop the `json == json2` assertion. Serialization is a pure function
of the fields and every reachable type derives structural `PartialEq`,
so it was implied by the value equality above it. It earns its keep in
the proto siblings this was modelled on, where `from_rust` +
`encode_to_vec` can differ for equal values, but not here.
* Both arms now dispatch over one type list in the same order, so a
corpus entry's type byte survives a one-bit change to the arm byte. Seed
the corpus and add a dictionary: the raw arm feeds the input straight to
`from_slice::`, which returns nothing until the input is a complete
object with the right field names and variant tags, so the nested types
were unreachable.
`build_regex`:
* The target capped patterns at 4096 chars on the premise that "the
`regex` crate is size-limited, so an oversized pattern returns an error
rather than OOMing". That premise is wrong. `size_limit` bounds only the
compiled NFA. The memory is spent earlier, translating the pattern's AST
into `regex-syntax`'s HIR, which `size_limit` never sees, and cost there
scales with the pattern's source length. Measured with regex 1.12.4 and
the exact `RegexBuilder` configuration `build_regex` uses, `\p{L}`
repeated under `i`, peak RSS is \~7.4 KB per pattern byte against \~0.35
KB for a plain literal: 4 095 bytes -> 47 MB, 524 285 -> 3 903 MB, 1 048
575 -> 7 792 MB, all returning `CompiledTooBig`. The cap sat 87x under
the runner's `-rss_limit_mb=4096` and was dead besides, since the runner
passes no `-max_len` and libFuzzer's default is 4096 bytes.
* Add a rare `gen_long_pattern` arm that synthesizes a long pattern by
repeating one small unit, so the length comes from a handful of input
bytes rather than from `-max_len`, and straddles the 1 MiB guard. Drop
the pattern cap, keep the text cap. The arm now reaches 4043 MB of peak
RSS, and libFuzzer reports an 11-byte input that spends 59 s building a
513 845-byte pattern.
* Correct three doc claims: the module premise above, `gen_near_limit`'s
claim that its output can hit `PatternTooLarge` (those patterns are <=
\~45 bytes, so only `CompiledTooBig` is reachable), and its example,
which put the innermost quantifier inside the group. `REPLACEMENT`'s
`$99` was documented as an out-of-range capture index but parses as the
capture *name* `99y`, since an unbraced `$name` takes the longest run of
`[0-9A-Za-z_]`, which took the name-lookup path and silently swallowed
the trailing literal. Brace it as `${99}`.
`EvalError` proto round-trip:
* The round-trip assertion alone certified a decoded `EvalError` that
aborted the process the moment anything looked at it: `OutOfDomain(None,
None, _)` survives the wire intact, and `Display` panicked on exactly
that state. Assert that the decoded value also renders, which is what
`DataflowErrorSer::Display` does with untrusted bytes on the index peek
path. Verified falsifiable both ways: a 15-byte mode B input crafted to
decode as `OutOfDomain(None, None, "x")` crashes the target against the
pre-fix `Display` and passes after it, and 1.3M executions are clean on
the fixed tree.
* Handle a `new_tree` rejection by returning rather than panicking,
matching the sibling round-trip targets, so a generator limit cannot be
reported as a product crash. Correct the module doc: `EvalError` carries
no `NumericMaxScale`, and it is mode B, not mode A, that reaches the
rejection branches of `char::from_proto` and `usize::from_proto`, since
mode A always starts from an already-valid Rust value.
Transform equivalence oracles:
* Add `hits_non_strict_error_fold`, which skips plans matching the open
bug [CPU-137](https://linear.app/materializeinc/issue/CPU-137) rather
than rediscovering it on every run. `Or::eval` returns `true` the moment
it sees a true operand and drops any error it collected, `And::eval`
does the same for `false`, and `error_if_null` evaluates its message
operand only when the first operand is NULL, yet `reduce`'s generic
variadic fold replaces the whole call with an operand's literal error
and `undistribute_and_or` can recombine an erroring operand across the
short-circuit boundary. Deliberately conservative: it asks whether an
operand *could* error, because `reduce` folds a column-free fallible
operand to a literal error first and absorbs it after. Measured at 3.6%
of `gen_rel(depth = 4)` plans skipped.
`interchange::avro::Decoder` over a fuzzer-generated reader schema:
* Gate every "don't build a valid body" branch on remaining input.
`Unstructured::int_in_range` does not fail once the data is exhausted,
it returns the low end of the range forever, and schema generation
consumes input greedily, so the ungated `== 0` branches were taken by
every exhausted input. Measured over 400k executions with the arms
instrumented: empty bodies fell from 36.5% to 4.3% of schema-directed
runs, the clean-body oracle armed on 5.8% before and 13.6% after, and
the same budget found 1035 new coverage units against 608, at a 7%
throughput cost.
* Make the corruption arms corrupt. `truncate(keep)` drew `keep` from
`0..=b.len()`, so a `keep == b.len()` draw was a no-op, and the byte
flip used `^= arbitrary::()`, so a zero mask was one too.
* Stop generating schemas mz-avro rejects, which discarded the whole
input: `fixed` size 0 (`parse_fixed` requires a positive size) and
`["null",["null",T]]` (`UnionSchema::new` rejects a union directly
inside a union). Schema rejection went from 3.4% of schema-directed runs
to zero.
* Bound a `decimal`-over-`fixed`'s precision by the chosen byte run.
Precision and size were drawn independently, and `parse_fixed` silently
demotes an over-precision decimal to a plain `fixed` with a `warn!`
rather than an error, so 10.2% of generated `DecimalFixed` types never
reached the decimal path while the harness still believed they had.
* Assert a clean body is consumed in full. A length-accounting bug that
under-consumes while still packing a well-formed `Row` passed every
other oracle.
* Correct two coverage claims. The `assert_success` oracle cannot reach
the deferred union-promotion class it cited
(MaterializeInc/materialize#37087): with `WriterSchemaProvider::None`
the resolver returns the reader schema verbatim, so no
`SchemaPiece::Resolve*` piece is ever built, and that class is covered
by `avro/fuzz::schema_resolve` instead. And the target never runs
`validate_schema_1`/`validate_schema_2`/`get_union_columns`, which are
reachable only from `schema_to_relationdesc`, so what it exercises is
the runtime `AvroFlatDecoder::union_branch`, not the column-name
invention.
`mz_repr::adt::interval` proto round-trip:
* Replace the `interval_proto_roundtrip` fuzz target with a proptest
next to the type. `Interval` <-> `ProtoInterval` is a field-for-field
copy of three integers, so there is no untrusted-input surface for a
fuzzer to explore: the property only bites once the two structs drift,
which a proptest over `any::()` catches just as well and runs
in CI on every commit.
`strconv` time and timestamp round-trips:
* `strconv_parse_time` skipped every parsed value whose nanosecond field
was not an exact multiple of 1000, then asserted exact equality across
`format_time`/`parse_time`. Post-filter that pair is a bijection, so
neither the assertion nor the re-parse-failure return could ever fire
and the effective property was only "`parse_time` must not panic". The
skipped region was also the only part of the space where the renderer
does anything interesting, so `format_time` never saw a sub-microsecond
fraction or a leap second at all. Render unconditionally and assert
bounded drift instead: the renderer writes microseconds rounding half
away from zero, so a faithful rendering moves the value by at most half
a microsecond. Measure the drift as nanoseconds from midnight rather
than field by field, because a fraction that rounds up legitimately
carries into the seconds field and on past a minute or hour boundary.
The one looser case is a carry out of the last second of the day, where
there is no second to carry into and the fraction saturates at
`.999999`. Falsifiable: dropping the renderer back to truncation is
reported in under a minute (`00:08:00.000000800` renders as `00:08:00`).
* Both timestamp targets fixed the rounding precision at `None`, i.e.
microseconds, which makes `round_to_precision`'s rounding branch dead:
the quantum is a single microsecond, so every value is already on a
boundary and only the seventh-digit nudge runs. The cast they mirror
carries an arbitrary `Option` from the column's type
modifier, and a precision below 6 is where rounding up off the end of
`HIGH_DATE` is easy to reach, a single `.5` at precision 0 rather than
seven exact digits. Take the precision from the input.
* Add a second oracle for the consumer that does not round.
`mz_pgrepr::Value::decode_text`, behind COPY and text-format bind
parameters, stores the parsed nanoseconds as they are, so for it the
renderer is itself the rounding step, and rendering plus re-parsing must
land exactly on the microsecond-rounded value. The carve-out is where
rounding up leaves chrono's range: the renderer has no error channel and
saturates the fraction instead, so there is nothing for the two to agree
on.
* Drop both escape hatches. A parsed `:60` now round-trips, so the
leap-second carve-out's comment no longer holds. The re-parse-failure
return's stated reason, a year of more than four digits, was simply
wrong: `262142-12-31 23:59:59` parses and round-trips byte-identically,
and over 395,498 successful parses the guard fired zero times. A
renderer that emits text the parser rejects is a bug to report, not
tolerate, so both are now `expect`.
* Add `strconv_parse_timestamp.dict` and
`strconv_parse_timestamptz.dict`. Neither end of the accepted range is
reachable by mutation alone: `HIGH_DATE` is the single date
`262142-12-31`, so hitting it means mutating a 6-digit ASCII integer
onto one exact value while a late time-of-day and a westward offset are
already in place, and the range check offers no coverage gradient to
climb, just one saturated edge for "accepted" and one for "rejected".
`src/repr/fuzz` shipped no dictionary at all, and its corpus directory
is gitignored, so there were no seeds either. With the boundary dates,
the round-up fractions and the sub-minute offsets spelled out as tokens,
an empty-corpus run reproduces the timestamptz rounding overflow in
under three minutes, on `(precision 2, "262142-12-31
?23:59:59.9999995")`.
* Verified by A/B: against the pre-fix renderer the unrounded oracle
fails on `2020-01-01 00:00:00.9999999` with `00:00:01` vs
`00:00:00.100`, and on a leap second because the rendering no longer
parses. Against the fixes, three minutes each of
`strconv_parse_timestamp` (7.1M execs), `strconv_parse_timestamptz`
(7.2M) and `strconv_parse_time` (3.6M) are clean.
`strconv_parse_interval`:
* Assert the round trip the module doc names. The oracle returned
silently when the rendering failed to re-parse, so only the "if it
re-parses, values agree" half was checked and any rendering the parser
rejects was a silent success. The justification given for that carve-out
does not exist: the hours field is the only unbounded one and is derived
as `(micros / 1_000_000).abs() / 3600`, so multiplying it back out
during the re-parse cannot exceed `|micros|` and cannot overflow.
`i64::MAX` micros renders as `2562047788:00:54.775807` and re-parses
exactly, as does `i64::MIN`. A direct sweep of the `(months, days,
micros)` space, 300,776 triples over both extremes, an edge grid and a
pseudo-random fan-out, produced zero re-parse failures and zero
mismatches, so re-parse is total and the branch was dead.
* What the tolerance cost, measured by mutating `Display`'s months
spelling to one `parse_interval` rejects: the tolerant oracle ran
3,570,705 executions and reported success, while the strict one reports
the 4-byte input `1mon` immediately. Against the real renderer 5.2M
executions are clean.
* Render through `strconv::format_interval` rather than `Display`
directly, since that is the entry point `::text` and the pgwire encoders
call. The two agree today, `format_interval` being a `write!` of
`Display`, so this only keeps the target pointed at the right function
if that stops being true. This matters for a type whose rendering is
load-bearing and has already changed once: the unit spelling moved to
`mon`/`mons` in v26.35
([SQL-463](https://linear.app/materializeinc/issue/SQL-463)), filed as a
wrong-result bug because a client's parser stopped at the old spelling.
`strconv_parse_date`:
* Assert the `Nestable` verdict, not just the round trip. `format_date`
returns a `Nestable` alongside the rendering and the target dropped it,
leaving half the contract unchecked, and that half was wrong: the ` BC`
suffix carries whitespace, which every element escaper quotes for, yet
`Nestable::Yes` was returned unconditionally. A wrong `Yes` is invisible
to a round-trip oracle, because the wrongly-unquoted rendering still
re-parses, so it needs an assertion of its own. Checked against the real
escapers through `element_needs_escaping` rather than restating their
rules in the target, where a copy would go stale:
[SQL-466](https://linear.app/materializeinc/issue/SQL-466) already added
a rule to that predicate.
* Make the re-parse a hard failure. The tolerance was borrowed from the
timestamp targets and neither reason applies to DATE. `format_date`
writes no time component, so the leap-second gap cannot appear in its
output, and a year of more than four digits parses, since
`fill_pdt_date` reads a 6-digit leading number followed by a dash as a
full year. Sweeping the whole `Date` range, exhaustively at both ends
and across the BC/AD transition, leaves exactly four reachable forms, a
4, 5 or 6 digit Common Era year and a 4 digit BC year, and 100,962
sampled dates produced zero re-parse failures. The branch was dead.
* Verified by A/B: with `format_date`'s verdict put back to an
unconditional `Nestable::Yes` the target reports `8==8=8= BC` within
seconds, and against the fix 7.9M executions are clean.
`sql_server_table_desc_proto_roundtrip`:
* Assert the constraint-string arm's oracle. The module doc and an
inline comment both stated it, "garbage strings must be rejected, valid
ones must parse", and neither half was checked: `Err` was discarded by
an early `return`, and on `Ok` nothing looked at which spelling produced
it. The arm's whole input space is `CONSTRAINT_TYPES.len() * 4` fixed
cases, so without that assertion it contributed nothing beyond "does not
panic", and it was blind to all three ways the parser can regress. A
normalization or catch-all arm would start accepting `primary key`; a
valid spelling ceasing to parse would make every input take the early
return, leaving an inert arm indistinguishable from a passing one; and
swapping the two variants is preserved by a round trip. The variant is
not cosmetic, it drives `is_primary` on the `TableConstraint::Unique`
that purification writes into the generated subsource DDL, so misparsing
a non-unique upstream constraint as `PrimaryKey` declares a key that
does not hold. Verified by mutating the product parser three ways:
trim/uppercase normalization, `"PRIMARY KEY"` no longer parsing, and the
two variants swapped. Each is now reported within seconds, and the
unmutated tree is clean.
* Give every arm its bytes from offset 1. Only the proptest arm consumes
the 32-byte seed, yet all four skipped past it, so the other three read
their selector bytes from offset 33 and saw nothing but defaults until
libFuzzer grew inputs that far. That also meant the raw-bytes arm, the
one meant to consume a real encoded descriptor, decapitated any genuine
blob added to the corpus, which `--corpus-sync` accumulates across runs.
The same 3-minute budget now reaches 3594 edges and 18382 features
against 3507 and 17215 before.
* Unlock two `parse_data_type` branches that were unreachable by
construction, not merely rare. `precision` and `scale` were drawn `%
39`, capping both at 38, so the `precision > 39` rejection and the
`NumericMaxScale::try_from` failure could never fire, while the header
claimed the type list exercises every branch of the mapping. `% 45`
reaches both, and `SqlServerColumnDesc::new` folds either rejection into
an `Unsupported` decode type that still round-trips.
* Pin `max_length` 4 and 8. `real`/`float`/`double precision` select
`F32`/`F64` by byte width, not by name or precision, so the two decode
types the header documents were reachable only by luck through the two
random length branches. The inline comments claimed precision selected
them, which was never the rule.
* Drop the per-column leaf round-trip loop. The table's
`into_proto`/`from_proto` delegate per element and equality is
structural, so the table-level assertion already implies it. The loop
just doubled the arm's conversion work.
* Give the constraint arm's descriptor the columns its constraint names,
instead of pairing a constraint over `c0..cn` with an empty column list.
Harmless against today's oracle, but a constraint/column consistency
check in `from_proto` would have been reported as a crash on the arm's
own inputs.
Transform oracle helpers:
* Stop swallowing `TransformError`. `optimize` discarded the whole enum
with `.ok()`, documented as skipping the case where "the `Typecheck`
pass rejects the plan". That case cannot happen.
`Typecheck::actually_perform_transform` returns `Ok(())` on every path
and routes a type error through `type_error!(true, ..)` ->
`soft_panic_or_log!`, as does `Fixpoint` non-convergence, and soft
assertions default to `cfg!(debug_assertions)`, which cargo-fuzz
enables. Confirmed by handing `optimize` a `Filter` with an `int4`
predicate in the built fuzz binary: it panics at `typecheck.rs:1672`, it
does not return `None`. So the comment sent anyone extending the
generators looking for a silent skip that does not exist, while a
rejected plan actually lands as a `crash-*` artifact pointing into
`Optimizer::optimize`, which CI reports as an optimizer bug.
* What could still reach `.ok()` was the opposite of benign: every
remaining `TransformError` over these plans is an optimizer invariant
violation, a `Let`/`Get` on an unbound local id, a `Let` whose type
changed under it, an ANF rebinding that lost an identifier, all of which
reach users as `internal error`. `NormalizeLets` runs on every input and
`RelationCSE` in `fixpoint_logical_02`, so the harness ran through this
code constantly while being structurally unable to report anything in
it. Measured false-pass width by making
`NormalizeLets::actually_perform_transform` return `Internal("injected
invariant violation")` on every input: the old oracle completed
**367,428 executions and reported success**. It now panics on the first
one. `fold_to_multiset`'s `.ok()?` and `collapse`'s two `is_err() ->
StuckFixpoint` arms folded the same class into a skip and get the same
treatment; `StuckFixpoint` still covers genuine non-convergence.
* Record why the `EvalError` skip in `fold_to_multiset` is required
rather than a blind spot. `predicate_pushdown` knowingly pushes a
predicate that can error into a join input and manufactures an error the
unoptimized plan never raises (database-issues#6258), so one side
folding to an error while the other yields rows is accepted behaviour,
measured at roughly 1 in 1,500 executions. Without the reference the
next reader is likely to "fix" it into an assertion.
* Document `gen_rel`'s arity contract. The doc spelled out the
non-negativity contract for the caller-supplied `leaf` but not that it
must return at least one column. A zero-arity leaf makes the `Project`
arm's `int_in_range(1..=arity)` an empty range and underflows `arity -
1`, and worse than either panic, `join_scalars` drops an arity-0
single-row input *after* `join` computed the equivalences' global column
offsets over the full input list, so the equivalences and the returned
schema would silently point at the wrong columns. Both in-tree leaves
return 1-3 columns, so this is a contract note, not a fix.
Transform generators and the three transform targets:
* Build some nodes as raw variants instead of through
`MirRelationExpr`'s smart constructors, which normalize away exactly the
shapes the structural fusions collapse: `map` extends an existing
`Map`'s scalar list, `negate` cancels an enclosing `Negate`, `union`
flattens its operands, `project` composes into an existing `Project`,
`filter` merges/sorts/dedups predicates. `MapFusion`, `NegateFusion` and
`UnionFusion` therefore could not fire on *any* input, so their two
assertions compared a plan against a clone of itself. Measured over
25,714 plans before: 0, 0 and 0 firings against FilterFusion's 16,526
(which survives only through its `canonicalize_predicates` call) and
ProjectFusion's 6,046 (through identity-projection removal). After: 939,
2,934 and 10,504, with the other three unchanged or slightly up.
* Rebuild `wrap_preserving` from raw variants. Two of its four arms were
unconditional no-ops, `filter` dropping a literal-true predicate and
`negate().negate()` cancelling to the identity, which silently reduced
the structural-rewrite oracle to `fold(rel) == fold(rel)`. It now
changes the plan on every input (40,001 of 40,001) against 81.6% before.
* Apply each fusion in the traversal order its real driver uses.
`fusion::union` and `ProjectionExtraction` call `visit_mut_post`; only
the `Filter`/`Project`/`Map`/`Negate` fusions are pre-order. The comment
claimed all six matched their drivers.
* Report the input plan, not the folded one, in `FoldConstants`' shape
assertion. Every other call site passes the original; this one had moved
it.
* Plan with production's `OptimizerFeatures`, not `Default::default()`,
which is all-`false` and is what no deployment runs. 14 of the 19
boolean optimizer flags default to `true` in production, including the
three `EquivalencePropagation` reads.
`enable_eq_classes_withholding_errors` is the pointed one: it exists to
stop equivalence propagation suppressing errors, and `gen_scalar`
deliberately seeds error literals, so the one feature built for this
generator's input class was the one turned off. `fuzz_features` names
only the flags whose production default is `true` and takes the rest
from `Default`, so it cannot break this crate's slow sanitizer build.
The tripwire for a newly added flag stays where it is cheap: `mz_sql`'s
`optimizer_features_no_enable_for_item_parsing` already destructures
`OptimizerFeatures` exhaustively, and now says to enable the flag in
`fuzz_features` too when it ships on, per the repo convention that a new
flag is ON in the test configuration.
* Cap the oracles' constant folding at `FOLD_ROW_LIMIT` rows.
`FoldConstants`' join arm materializes the full cross product before
applying equivalences and `limit: None` disables its only size check, so
the harness could ask for a `Vec<(Row, Diff)>` bounded only by the
product of every leaf. Declining to fold is already a benign skip on
both oracles, and the cap sits well above the largest product these
generators were measured to reach.
* Optimize before deciding the baseline is comparable, in
`optimizer_symbolic_equiv`. A plan whose result is an evaluation error
folds to an `Err` constant, which `collapse` reports as a skip, and
returning there meant never calling the optimizer on it: `gen_scalar`
spends one of three literal-leaf choices on `EvalError::DivisionByZero`,
so a deliberately generated class of input never reached the code under
test at all, not even for panic coverage.
* Share one `collapse` between the two equivalence targets.
`full_optimizer_equiv` folded the optimized side with a single
`FoldConstants` pass, which cannot fold through a `Let`, while its
sibling already iterated `FoldConstants` + `NormalizeLets` to a fixpoint
"to collapse any `Let`s the optimizer's CSE introduced". `RelationCSE`
gets perfect candidates from the `Union` arm cloning `inner` into both
branches, and the skip landed precisely on the executions where the
post-collapse stages had run on real structure.
* Correct two doc claims. Neither target skips on an optimizer error any
more, so the `Typecheck`-rejection rationale is gone from both. And of
the five transforms `full_optimizer_equiv` claimed as its unique
coverage, only `NonNullRequirements` and `RedundantJoin` run before step
2's trailing constant folding; `Demand`, `SemijoinIdempotence` and
`ReduceElision` sit in `fixpoint_logical_01`/`_02` and see a bare
`Constant`. Nothing in the suite covers those three against exact key
facts, which the doc now says.
* Record that `Collapse::StuckFixpoint` conflates "folding could not
evaluate it" with "it folded to an `Err`", that only the second is a
determined result, and why the tempting `Ok -> Err` assertion is not
available today (`predicate_pushdown` manufactures such errors,
database-issues#6258).
Carving out [CPU-137](https://linear.app/materializeinc/issue/CPU-137),
once, in one place:
* Give the shape a single definition,
`MirScalarExpr::could_hit_nonstrict_error_fold`, in `mz_expr` next to
the fold it describes. Four oracles now compare evaluation across
`reduce` and each needs to skip it; two had already drifted, since the
transform helper counted `ErrorIfNull` as non-strict and
`mir_scalar_reduce`'s copy did not.
* Apply it to `mfp_optimize`, which had no carve-out at all. `optimize`
reduces every map expression and predicate, and that target generates
`.and()`/`.or()` over a fallible vocabulary, so all three of its
assertion arms were exposed: a folded error makes a row the raw MFP
passed error out, the literal is typed non-nullable so a
nullability-dependent rewrite above it can change the projected value,
and a predicate folded that way drops the row. Its existing tolerance
covers none of those, because it only forgives an error on a row the raw
MFP *filtered*.
* In `mfp_optimize` the carve-out is also load-bearing for a second
mechanism, the one release qualification 1330, 1332, 1334 and 1335
failed on: memoization hoists a fallible subexpression shared under an
`AND`/`OR` into a mapped column that `SafeMfpPlan::evaluate_inner` runs
eagerly, surfacing an error the lazy operator absorbs.
MaterializeInc/materialize#37974 originally changed `optimize` to
prevent that, and after
[review]()
it no longer does: `AND`/`OR` evaluation order is undefined
([STG-54](https://linear.app/materializeinc/issue/STG-54)), so the
surfaced error is a legal outcome of the same plan, and the oracle
rather than the product has to tolerate it.
* Non-temporal mode therefore takes `mir_scalar_reduce`'s row-precise
`absorbs_and_or_operand_error` rather than the coarse whole-MFP skip, so
only the rows that actually absorb an operand error stop asserting. The
coarse predicate fires on 2.2% of MFPs (30,576 of 1,400,000), and those
are exactly the `AND`/`OR`-over-fallible shapes where optimize's
memoization is busiest, so dropping them entirely would blind the value
and filter arms where a real miscompile is most likely to sit. Temporal
mode keeps the coarse skip: its reference side substitutes a literal for
`mz_now()` and re-runs `optimize` per probed time, so "absorbs on this
row" has no single answer there. Verified in both directions against the
unfixed product: a hand-crafted input generating `((#2 * #2) <= (#2 *
#2)) IS NULL OR #4` with an overflowing `#2` crashes the current main
oracle with exactly the release-qualification assertion and passes the
row-precise one, and a twin input hiding the shared fallible part in an
untaken `If` branch still crashes it when `eager_children`'s `If` guard
is deliberately removed.
* `mir_scalar_reduce` keeps its row-precise variant rather than adopting
the coarse predicate, because the shape is common in its generator and
skipping every expression carrying it would cost most of the
comparisons. `mfp_optimize`'s non-temporal mode adopts the same
row-precise variant. The relationship between the coarse and row-precise
forms is now written down at both ends.
`mir_scalar_reduce`:
* Skip a row that absorbs a non-strict AND/OR operand error outright,
instead of merely allowing it to reduce to an error. The old carve-out
was gated on the reduced side also being an error, justified as "an
absorbed error cannot resurface as a value". The fuzzer falsified that
after 2.2M executions: `Or([Literal(Err(DivisionByZero)), Column(3)])`
evaluates to `Ok(True)`, because `Or::eval` returns true on its first
true operand and drops the error it collected, while reduce replaces the
whole call with the literal error. From that point the reduced
expression is folding a value the original never computed, and the
rewrites downstream do not all propagate it: the reported case turned
`CastBoolToString(IsNull(If(..)))` from `Ok("true")` into a literal
`Ok("false")`.
* That is [CPU-137](https://linear.app/materializeinc/issue/CPU-137),
which PR MaterializeInc/materialize#37299 did not fix, and the wrong
value is downstream of it rather than a second defect, verified by
reducing the `Or` in isolation. So the whole row goes, matching how the
transform targets skip the same shape via `hits_non_strict_error_fold`.
The skip stays per-row rather than per-expression, so every
non-absorbing row of an affected expression is still compared.
* Checked that this does not blunt the oracle: swapping `reduce_if`'s
literal-condition branches is still reported within seconds, and 4.8M
executions are clean against the unmutated tree.
Workspace-wide sweep of the remaining targets:
* The same three corrections applied across the rest of the suite:
generators that reach the decode instead of stalling at an early reject,
oracles that assert the property they name rather than a weaker one it
implies, and module docs pruned of coverage that was never reached.
Touches the targets in `repr` (row/arrow/codec round-trips, `strconv`
parsers, `numeric_arith`, `range_ops`, the type-modifier proto
round-trips), `expr` (`jsonb_get`, `jsonb_path`, `cast_string`,
`like_pattern_compile` and `like_pattern_escape`, `agg_decompose`,
`mir_scalar_reduce`, `timezone_convert`, `mfp_optimize`), `pgwire`
(`codec_decode`, plus corpus dictionary and seeding), `pgcopy`,
`pgrepr`, `pgtz`, `storage` (the four upsert targets), `storage-types`,
`sql-parser`, `mysql-util`, `postgres-util`, `sql-server-util`,
`persist-client` and `interchange`.
---------
Co-authored-by: Claude Opus 5 (1M context)
---
.../pipeline.template.yml | 5 +-
misc/python/materialize/cli/fmt.py | 61 +-
rustfmt.toml | 5 +
src/avro/fuzz/Cargo.toml | 4 +
src/avro/fuzz/corpus.dict | 14 +-
.../fuzz/fuzz_targets/avro_schema_parse.rs | 110 +++-
src/avro/fuzz/fuzz_targets/reader_decode.rs | 95 ++-
src/avro/fuzz/fuzz_targets/schema_resolve.rs | 289 +++++---
src/avro/fuzz/prepare-corpus.sh | 11 +-
src/avro/src/schema.rs | 43 ++
src/catalog-protos/fuzz/Cargo.toml | 4 +
.../fuzz/catalog_objects_serde_roundtrip.dict | 100 +++
.../catalog_objects_serde_roundtrip.rs | 107 +--
src/catalog-protos/fuzz/prepare-corpus.sh | 67 ++
src/expr/fuzz/fuzz_targets/agg_decompose.rs | 177 +++--
src/expr/fuzz/fuzz_targets/build_regex.rs | 93 ++-
src/expr/fuzz/fuzz_targets/cast_string.rs | 2 +-
.../eval_error_proto_roundtrip.rs | 53 +-
src/expr/fuzz/fuzz_targets/jsonb_get.rs | 99 ++-
src/expr/fuzz/fuzz_targets/jsonb_path.rs | 27 +-
.../fuzz/fuzz_targets/like_pattern_compile.rs | 66 +-
.../fuzz/fuzz_targets/like_pattern_escape.rs | 132 +++-
src/expr/fuzz/fuzz_targets/mfp_optimize.rs | 271 ++++++--
.../fuzz/fuzz_targets/mir_scalar_reduce.rs | 113 +++-
.../fuzz/fuzz_targets/timezone_convert.rs | 52 +-
src/expr/fuzz/like_pattern_compile.dict | 20 +
src/expr/src/scalar.rs | 38 ++
.../fuzz_targets/avro_decode_fuzzed_schema.rs | 211 ++++--
.../fuzz/fuzz_targets/json_encode.rs | 198 +++++-
.../protobuf_decode_fuzzed_schema.rs | 138 ++--
.../mysql_table_desc_proto_roundtrip.rs | 137 ++--
.../fuzz_targets/rollup_proto_roundtrip.rs | 112 +++-
.../state_diff_proto_roundtrip.rs | 101 ++-
src/pgcopy/fuzz/fuzz_targets/copy_decode.rs | 81 ++-
.../fuzz/fuzz_targets/value_decode_binary.rs | 228 +++++--
.../fuzz/fuzz_targets/value_decode_text.rs | 618 +++++++++++-------
src/pgtz/fuzz/fuzz_targets/timezone_parse.rs | 187 ++++--
src/pgwire/fuzz/corpus.dict | 5 +
src/pgwire/fuzz/fuzz_targets/codec_decode.rs | 213 +++++-
src/pgwire/fuzz/prepare-corpus.sh | 32 +-
src/pgwire/src/lib.rs | 12 +-
.../postgres_table_desc_proto_roundtrip.rs | 179 +++--
src/repr/fuzz/Cargo.toml | 7 -
.../fuzz_targets/acl_item_proto_roundtrip.rs | 14 +-
.../column_type_proto_roundtrip.rs | 108 ++-
.../fuzz_targets/interval_proto_roundtrip.rs | 76 ---
src/repr/fuzz/fuzz_targets/numeric_arith.rs | 2 +-
src/repr/fuzz/fuzz_targets/range_ops.rs | 13 +-
.../relation_desc_proto_roundtrip.rs | 174 ++++-
.../fuzz/fuzz_targets/row_arrow_roundtrip.rs | 19 +-
.../fuzz/fuzz_targets/row_codec_roundtrip.rs | 13 +-
.../fuzz/fuzz_targets/row_proto_roundtrip.rs | 3 +-
.../scalar_type_proto_roundtrip.rs | 27 +-
.../fuzz/fuzz_targets/strconv_parse_date.rs | 33 +-
.../fuzz_targets/strconv_parse_interval.rs | 35 +-
.../fuzz_targets/strconv_parse_numeric.rs | 19 +-
.../fuzz/fuzz_targets/strconv_parse_range.rs | 17 +-
.../fuzz/fuzz_targets/strconv_parse_time.rs | 56 +-
.../fuzz_targets/strconv_parse_timestamp.rs | 74 ++-
.../fuzz_targets/strconv_parse_timestamptz.rs | 72 +-
src/repr/fuzz/prepare-corpus.sh | 191 ++++++
src/repr/fuzz/strconv_parse_timestamp.dict | 51 ++
src/repr/fuzz/strconv_parse_timestamptz.dict | 57 ++
src/repr/src/adt/interval.rs | 17 +
src/repr/src/strconv.rs | 12 +
src/sql-parser/fuzz/build.rs | 12 +-
src/sql-parser/fuzz/fuzz_targets/grammar.rs | 69 +-
.../fuzz/fuzz_targets/sql_roundtrip.rs | 213 +++++-
.../sql_server_table_desc_proto_roundtrip.rs | 181 +++--
src/sql/src/session/vars/definitions.rs | 6 +
.../fuzz/fuzz_targets/csv_decode.rs | 51 +-
.../dataflow_error_proto_roundtrip.rs | 6 +-
.../source_data_proto_roundtrip.rs | 9 +-
...xport_statement_details_proto_roundtrip.rs | 63 +-
src/storage-types/src/sources.rs | 1 +
.../fuzz/fuzz_targets/upsert_consolidate.rs | 6 +-
.../fuzz/fuzz_targets/upsert_runtime.rs | 16 +-
.../fuzz_targets/upsert_state_consolidate.rs | 6 +-
.../fuzz_targets/upsert_value_roundtrip_v2.rs | 20 +-
.../fuzz/fuzz_targets/full_optimizer_equiv.rs | 72 +-
.../fuzz_targets/mir_relation_transforms.rs | 175 ++++-
.../fuzz_targets/optimizer_symbolic_equiv.rs | 109 +--
src/transform/fuzz/src/lib.rs | 224 ++++++-
test/cargo-fuzz/mzcompose.py | 33 +-
84 files changed, 5231 insertions(+), 1641 deletions(-)
create mode 100644 src/catalog-protos/fuzz/catalog_objects_serde_roundtrip.dict
create mode 100755 src/catalog-protos/fuzz/prepare-corpus.sh
create mode 100644 src/expr/fuzz/like_pattern_compile.dict
delete mode 100644 src/repr/fuzz/fuzz_targets/interval_proto_roundtrip.rs
create mode 100755 src/repr/fuzz/prepare-corpus.sh
create mode 100644 src/repr/fuzz/strconv_parse_timestamp.dict
create mode 100644 src/repr/fuzz/strconv_parse_timestamptz.dict
diff --git a/ci/release-qualification/pipeline.template.yml b/ci/release-qualification/pipeline.template.yml
index 598044ac10241..de1c956824af9 100644
--- a/ci/release-qualification/pipeline.template.yml
+++ b/ci/release-qualification/pipeline.template.yml
@@ -278,12 +278,9 @@ steps:
composition: cargo-fuzz
ci_builder: nightly
args:
- - --profile=fruitful
+ - --profile=all
- --max-seconds=86400
- --wall-budget=84600
- # Step hard-times out at 1440min (86400s). --wall-budget ends fuzzing
- # at 84600s, leaving 1800s; cap minimize at 1200s so the corpus
- # upload has ~600s of headroom before the kill.
- --minimize-timeout=1200
- --corpus-sync
artifact_paths:
diff --git a/misc/python/materialize/cli/fmt.py b/misc/python/materialize/cli/fmt.py
index 7ea912bf98a08..9a12b7f631123 100644
--- a/misc/python/materialize/cli/fmt.py
+++ b/misc/python/materialize/cli/fmt.py
@@ -18,6 +18,12 @@
from materialize import MZ_ROOT
from materialize.parallel_task import TaskSpec, run_parallel
+# Rust sources live in more than one workspace, and `cargo metadata` only ever
+# reports the one it is pointed at. The `src/*/fuzz` cargo-fuzz crates attach to
+# the `test/cargo-fuzz` workspace, which the root workspace does not include, so
+# without a second invocation here they go unformatted entirely.
+RUST_MANIFESTS = ["Cargo.toml", "test/cargo-fuzz/Cargo.toml"]
+
def main() -> int:
parser = argparse.ArgumentParser(prog="fmt")
@@ -45,28 +51,43 @@ def _rustfmt_fn(*, check: bool):
def run() -> tuple[bool, str]:
ncpus = os.cpu_count() or 8
- result = subprocess.run(
- ["cargo", "metadata", "--no-deps", "--format-version=1"],
- capture_output=True,
- text=True,
- )
- if result.returncode != 0:
- return False, result.stderr.strip()
-
- meta = json.loads(result.stdout)
kinds = {"lib", "bin", "bench", "test", "example", "proc-macro", "custom-build"}
- paths = [
- t["src_path"]
- for pkg in meta["packages"]
- for t in pkg["targets"]
- if kinds & set(t["kind"])
- ]
- if not paths:
+ # Keyed by edition: `gen` is an identifier in 2021 but a reserved keyword
+ # in 2024, so rustfmt cannot even parse a file at the wrong edition.
+ paths_by_edition: dict[str, list[str]] = {}
+ for manifest in RUST_MANIFESTS:
+ result = subprocess.run(
+ [
+ "cargo",
+ "metadata",
+ "--no-deps",
+ "--format-version=1",
+ f"--manifest-path={manifest}",
+ ],
+ capture_output=True,
+ text=True,
+ )
+ if result.returncode != 0:
+ return False, result.stderr.strip()
+
+ meta = json.loads(result.stdout)
+ for pkg in meta["packages"]:
+ for t in pkg["targets"]:
+ if kinds & set(t["kind"]):
+ paths_by_edition.setdefault(pkg["edition"], []).append(
+ t["src_path"]
+ )
+ if not paths_by_edition:
return True, ""
# Split into batches and run rustfmt in parallel.
- batch_size = math.ceil(len(paths) / ncpus)
- batches = [paths[i : i + batch_size] for i in range(0, len(paths), batch_size)]
+ batches = []
+ for edition, paths in paths_by_edition.items():
+ batch_size = math.ceil(len(paths) / ncpus)
+ batches += [
+ (edition, paths[i : i + batch_size])
+ for i in range(0, len(paths), batch_size)
+ ]
cmd_base = ["rustfmt", "--config", "error_on_line_overflow=true"]
if check:
@@ -74,11 +95,11 @@ def run() -> tuple[bool, str]:
procs = [
subprocess.Popen(
- cmd_base + batch,
+ cmd_base + [f"--edition={edition}"] + batch,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
)
- for batch in batches
+ for edition, batch in batches
]
all_output = []
diff --git a/rustfmt.toml b/rustfmt.toml
index f8dc60ca8e5d2..b6ab042396f4a 100644
--- a/rustfmt.toml
+++ b/rustfmt.toml
@@ -1,3 +1,8 @@
edition = "2024"
+# Formatting style is repo-wide, independent of the edition a crate parses as.
+# The `src/*/fuzz` crates are still edition 2021, and `bin/fmt` overrides
+# `edition` per package so they parse at all. Without this pin they would also
+# fall back to the 2021 *style*, which sorts imports differently.
+style_edition = "2024"
tab_spaces = 4
merge_derives = false
diff --git a/src/avro/fuzz/Cargo.toml b/src/avro/fuzz/Cargo.toml
index 96118872559ff..6072a0f3298ec 100644
--- a/src/avro/fuzz/Cargo.toml
+++ b/src/avro/fuzz/Cargo.toml
@@ -20,6 +20,10 @@ cargo-fuzz = true
[dependencies]
libfuzzer-sys = "0.4"
mz-avro = { path = ".." }
+# Version must match the root `Cargo.toml`'s. `avro_schema_parse` needs a
+# `serde_json::Value` to reach `Schema::parse`, and that value has to be the same
+# type `mz-avro` was compiled against.
+serde_json = "1.0.150"
[[bin]]
name = "reader_decode"
diff --git a/src/avro/fuzz/corpus.dict b/src/avro/fuzz/corpus.dict
index 4740480132fd9..dcf3f2b00d664 100644
--- a/src/avro/fuzz/corpus.dict
+++ b/src/avro/fuzz/corpus.dict
@@ -1,10 +1,16 @@
-# libFuzzer dictionary for the Avro reader_decode target.
+# libFuzzer dictionary for the Avro fuzz targets, resolved crate-wide by
+# `dict_for` in test/cargo-fuzz/mzcompose.py.
#
# An Avro object-container file starts with the 4-byte magic "Obj\x01" followed
# by a map of metadata (notably "avro.schema" and "avro.codec") and a 16-byte
-# sync marker. Without the magic the decoder bails immediately, so the most
-# valuable token by far is the magic itself; the rest let the mutator build
-# plausible headers and schemas.
+# sync marker. Without the magic the decoder bails immediately, so the magic and
+# the schema JSON tokens let the mutator build plausible headers and schemas.
+#
+# These tokens pay off for the targets that consume their input as *bytes*:
+# `avro_schema_parse`, and `reader_decode`'s raw-bytes branch. `reader_decode`'s
+# container-building branch and `schema_resolve` instead read their input as an
+# `arbitrary::Unstructured` recipe of generator choices, where a token is just
+# more choice bytes and carries no structural signal.
# Object-container magic.
"Obj\x01"
diff --git a/src/avro/fuzz/fuzz_targets/avro_schema_parse.rs b/src/avro/fuzz/fuzz_targets/avro_schema_parse.rs
index 52cbe61c7a44c..e7970d02ebc04 100644
--- a/src/avro/fuzz/fuzz_targets/avro_schema_parse.rs
+++ b/src/avro/fuzz/fuzz_targets/avro_schema_parse.rs
@@ -16,20 +16,39 @@
//!
//! `schema_resolve` only parses the narrow set of schemas it generates (no
//! logical types, no named back-references, shallow), and the decode targets
-//! cap nesting low. This one stresses the parser itself: it generates schema
-//! JSON that can nest *past* the depth limit (so the guard must fire cleanly
-//! rather than overflow the stack) and that re-references already-defined names
-//! (recursive definitions, a distinct resolution path).
+//! cap nesting low. This one stresses the parser itself: schemas that
+//! re-reference already-defined names (recursive definitions, a distinct
+//! resolution path), and nesting that straddles `MAX_SCHEMA_DEPTH`.
+//!
+//! Reaching that guard takes the *other* entry point. `Schema::from_str` runs
+//! `serde_json::from_str` first, and serde_json's own recursion limit is also
+//! 128 containers, so it rejects the schema one level before the avro guard can
+//! fire: the deepest JSON text that survives is 127 wrappers, which is avro
+//! depth 128, exactly at the limit and not over it. So the deep case goes
+//! through `Schema::parse`, which takes an already-parsed `serde_json::Value`,
+//! and the text case is bounded to nesting serde_json will accept rather than
+//! generating JSON no avro code will ever see.
//!
//! It also drives the parser's *naming* and *validation* paths:
//! * `namespace` fields and dotted/`a.b.C` names. The `FullName::from_parts`
//! split logic, including the documented edge case where a name has dots
//! *and* a `namespace` is also given (they may disagree).
-//! * `aliases` arrays on named types, sometimes deliberately colliding with
-//! another defined name or with the type's own name.
-//! * structurally-valid-but-semantically-invalid schemas the parser is meant
-//! to *reject* (not panic on): duplicate enum symbols, duplicate record
-//! field names, decimal `scale > precision`, and `fixed` `size: 0`.
+//! * a name that collides with an already-defined type's fullname, which
+//! `alloc_name` must reject.
+//! * `aliases` arrays on named types, sometimes colliding with another defined
+//! name or with the type's own name. NOTE: parsing only *collects* aliases
+//! into the `Name`, so a collision is indistinguishable from any other alias
+//! here. Aliases only acquire meaning in `resolve_schemas`, which this target
+//! does not call. They are generated to keep the attribute shape varied, not
+//! as a validation path.
+//! * semantically-invalid schemas, which must produce an error rather than a
+//! panic: duplicate enum symbols, an enum `default` outside its symbols, and
+//! `fixed` `size: 0` are all rejected. NOTE: two further shapes generated
+//! here are *accepted* today, and are generated because the parser has to
+//! survive them, not because it rejects them. `parse_record` has no
+//! duplicate-field-name check, so a duplicate silently collapses in `lookup`
+//! while both entries stay in `fields`; and `parse_bytes` catches a decimal
+//! `scale > precision` error, logs it, and falls back to plain `bytes`.
//! Parsing must never panic. It returns `Ok`/`Err`.
#![no_main]
@@ -100,11 +119,22 @@ fn gen_named_attrs(
/// error rather than panic.
fn gen_invalid(u: &mut Unstructured, counter: &mut u32) -> arbitrary::Result {
*counter += 1;
- Ok(match u.int_in_range(0u8..=4)? {
- // Enum with duplicate symbols.
- 0 => format!(
- "{{\"type\":\"enum\",\"name\":\"E{counter}\",\"symbols\":[\"A\",\"B\",\"A\"]}}"
+ Ok(match u.int_in_range(0u8..=5)? {
+ // Two named types sharing a fullname. The names generated elsewhere carry
+ // a strictly increasing counter and so can never collide, which leaves
+ // `alloc_name`'s duplicate-fullname rejection unreachable; this is the one
+ // shape that reaches it. Both twins are declared bare, with no
+ // `namespace`, so the collision does not depend on how the enclosing type
+ // was named.
+ 5 => format!(
+ "{{\"type\":\"record\",\"name\":\"Outer{counter}\",\"fields\":[\
+ {{\"name\":\"a\",\"type\":{{\"type\":\"record\",\"name\":\"Twin{counter}\",\"fields\":[]}}}},\
+ {{\"name\":\"b\",\"type\":{{\"type\":\"record\",\"name\":\"Twin{counter}\",\"fields\":[]}}}}]}}"
),
+ // Enum with duplicate symbols.
+ 0 => {
+ format!("{{\"type\":\"enum\",\"name\":\"E{counter}\",\"symbols\":[\"A\",\"B\",\"A\"]}}")
+ }
// Record with duplicate field names.
1 => format!(
"{{\"type\":\"record\",\"name\":\"R{counter}\",\"fields\":[\
@@ -215,14 +245,60 @@ fn gen_type(
})
}
+/// Enough array layers to straddle `MAX_SCHEMA_DEPTH` (128) from both sides.
+const MAX_WRAPPERS: u32 = 300;
+
fn run(mut u: Unstructured) -> arbitrary::Result<()> {
let mut counter = 0u32;
let mut defined = Vec::new();
- // Start deeper than MAX_SCHEMA_DEPTH (128) so the fuzzer can drive nesting
- // past the limit and exercise the depth guard, not just shallow schemas.
- let depth = u.int_in_range(1u32..=200)?;
+ // Bounded so the generated text always survives serde_json, which refuses
+ // the 128th nested container. `depth` counts generator levels, not
+ // containers, and a level spends up to three of them (a record is
+ // `{record}` → `[fields]` → `{field}` before recursing), with `gen_invalid`
+ // spending five at the leaf. So a schema nests at most `3 * depth + 2`
+ // containers, and 40 keeps the worst case at 122. Deep nesting is the
+ // wrapped path's job below, which is not subject to this limit at all.
+ let depth = u.int_in_range(1u32..=40)?;
let schema = gen_type(&mut u, &mut counter, &mut defined, depth)?;
- let _ = Schema::from_str(&schema);
+
+ // Half the inputs go through the text entry point unchanged. The other half
+ // get wrapped to straddle `MAX_SCHEMA_DEPTH`, which is only reachable behind
+ // `Schema::parse`. Splitting rather than doing both keeps the cost at one JSON
+ // parse plus one schema parse per input, the same as parsing the text alone.
+ let wrappers = if u.int_in_range(0u8..=1)? == 0 {
+ 0
+ } else {
+ u.int_in_range(1u32..=MAX_WRAPPERS)?
+ };
+ if wrappers == 0 {
+ let _ = Schema::from_str(&schema);
+ return Ok(());
+ }
+
+ // Every generated schema is well-formed JSON by construction (all
+ // interpolated names are `R`/`E`/`F`/`AnAlias`/`Twin`), so a
+ // parse failure here is a generator bug, and swallowing it would silently
+ // skip the depth-guard half of this target.
+ let mut value: serde_json::Value = serde_json::from_str(&schema)
+ .unwrap_or_else(|e| panic!("generated schema {schema} must be valid JSON: {e}"));
+
+ // `Schema::parse` takes an already-parsed value and so is not subject to
+ // serde_json's limit, which is what makes the guard reachable at all. The
+ // wrapper count falls on either side of it, exercising the accepted depths,
+ // the rejection, and the `self.depth` decrement on the guard's error path.
+ // Built by moving `value` into each new layer. `serde_json::json!` would
+ // deep-clone it instead (its leaf case goes through `to_value`), making the
+ // wrapping quadratic in the layer count.
+ for _ in 0..wrappers {
+ let mut layer = serde_json::Map::new();
+ layer.insert(
+ "type".to_string(),
+ serde_json::Value::String("array".to_string()),
+ );
+ layer.insert("items".to_string(), value);
+ value = serde_json::Value::Object(layer);
+ }
+ let _ = Schema::parse(&value);
Ok(())
}
diff --git a/src/avro/fuzz/fuzz_targets/reader_decode.rs b/src/avro/fuzz/fuzz_targets/reader_decode.rs
index f64f3b34101de..5c308517db63c 100644
--- a/src/avro/fuzz/fuzz_targets/reader_decode.rs
+++ b/src/avro/fuzz/fuzz_targets/reader_decode.rs
@@ -32,12 +32,22 @@
//! * an unrecognized codec string: the `UnrecognizedCodec` header-parse
//! error. (`snappy` is gated behind a Cargo feature that this fuzz crate
//! does not enable, so it is intentionally not generated.)
-//! We also corrupt the declared block framing: occasionally the object-count or
-//! byte-size prefix is replaced with a huge or wildly inconsistent value, which
-//! must be caught by the `safe_len` allocation guard rather than spinning or
-//! over-allocating. And we still hit the structural error paths, occasionally a
-//! corrupted trailing sync marker (the mismatch path) or a whole-file truncation
-//! mid-block (the short-read path). Decoding must never panic any of these ways.
+//! We also corrupt the declared block framing. `read_block_next` runs both the
+//! object-count and the byte-size prefix through the `safe_len` allocation guard
+//! before it touches the payload, so a magnitude past `MAX_ALLOCATION_BYTES`
+//! never reaches anything downstream of that guard. The lies therefore straddle
+//! it deliberately:
+//! * a count or size past the budget, which `safe_len` must reject rather than
+//! attempt a giant allocation.
+//! * a count over the objects the block actually encodes but inside the
+//! budget, which `safe_len` accepts. `bound_block_object_count` must then
+//! reject any count the payload could not hold, and one it accepts must
+//! terminate against the bytes present instead of spinning.
+//! * a size over the bytes the block actually carries but inside the budget,
+//! so `fill_buf` runs and takes the short-read path.
+//! And we still hit the structural error paths, occasionally a corrupted trailing
+//! sync marker (the mismatch path) or a whole-file truncation mid-block (another
+//! short read). Decoding must never panic any of these ways.
#![no_main]
@@ -45,6 +55,12 @@ use libfuzzer_sys::arbitrary::{self, Unstructured};
use libfuzzer_sys::fuzz_target;
use mz_avro::{Codec, Reader};
+/// The ceiling `util::safe_len` applies to every length read off the wire, from
+/// the private `mz_avro::util::MAX_ALLOCATION_BYTES`. Only used to pick the
+/// magnitude of a corrupted block prefix, so drift merely shifts which side of
+/// the guard a generated prefix lands on.
+const MAX_ALLOCATION_BYTES: i64 = 512 * 1024 * 1024;
+
/// A generated Avro type. Structured (not straight-to-JSON) so the object
/// encoder can walk the same type. An array is one item type in the schema but
/// N item values in a block.
@@ -82,7 +98,11 @@ fn gen_ty(u: &mut Unstructured, counter: &mut u32, depth: u32) -> arbitrary::Res
7 => Ty::Bytes,
8 => {
*counter += 1;
- let size = u.int_in_range(0u32..=24)?;
+ // Size 0 is rejected outright ("Fixed values require a positive size
+ // attribute"), which would fail the header parse and discard the
+ // input before a single block decodes. `avro_schema_parse` covers
+ // that rejection deliberately.
+ let size = u.int_in_range(1u32..=24)?;
Ty::Fixed(*counter, size)
}
9 => {
@@ -101,7 +121,22 @@ fn gen_ty(u: &mut Unstructured, counter: &mut u32, depth: u32) -> arbitrary::Res
}
11 => Ty::Array(Box::new(gen_ty(u, counter, depth - 1)?)),
12 => Ty::Map(Box::new(gen_ty(u, counter, depth - 1)?)),
- _ => Ty::Nullable(Box::new(gen_ty(u, counter, depth - 1)?)),
+ _ => {
+ // `Ty::Nullable` is the only union this target emits, and the parser
+ // rejects both `["null","null"]` and a union nested directly inside a
+ // union. Flatten a nested nullable and swap a bare `null` for a type
+ // that keeps the union valid, so the header still parses. Those two
+ // rejections belong to `avro_schema_parse`. Here they would only
+ // discard the input before any block decodes.
+ let mut inner = gen_ty(u, counter, depth - 1)?;
+ while let Ty::Nullable(nested) = inner {
+ inner = *nested;
+ }
+ if matches!(inner, Ty::Null) {
+ inner = Ty::Long;
+ }
+ Ty::Nullable(Box::new(inner))
+ }
})
}
@@ -115,14 +150,16 @@ fn ty_to_json(ty: &Ty, out: &mut String) {
Ty::Double => out.push_str("\"double\""),
Ty::String => out.push_str("\"string\""),
Ty::Bytes => out.push_str("\"bytes\""),
- Ty::Fixed(n, size) => {
- out.push_str(&format!("{{\"type\":\"fixed\",\"name\":\"F{n}\",\"size\":{size}}}"))
- }
+ Ty::Fixed(n, size) => out.push_str(&format!(
+ "{{\"type\":\"fixed\",\"name\":\"F{n}\",\"size\":{size}}}"
+ )),
Ty::Enum(n) => out.push_str(&format!(
"{{\"type\":\"enum\",\"name\":\"E{n}\",\"symbols\":[\"A\",\"B\",\"C\"]}}"
)),
Ty::Record(n, fields) => {
- out.push_str(&format!("{{\"type\":\"record\",\"name\":\"R{n}\",\"fields\":["));
+ out.push_str(&format!(
+ "{{\"type\":\"record\",\"name\":\"R{n}\",\"fields\":["
+ ));
for (i, f) in fields.iter().enumerate() {
if i > 0 {
out.push(',');
@@ -332,20 +369,36 @@ fn run(mut u: Unstructured) -> arbitrary::Result<()> {
}
// The block framing prefixes. Usually honest, but occasionally we lie
- // about the object count or the byte size, including absurdly large or
- // negative (→ huge `usize`) values, to exercise the `safe_len` guard
- // and the short-read handling, neither of which may panic.
+ // about the object count or the byte size. `read_block_next` runs both
+ // prefixes through `safe_len` before touching the payload, so a
+ // magnitude past `MAX_ALLOCATION_BYTES` only ever reaches that guard.
+ // The lies below therefore straddle it deliberately: over the budget to
+ // exercise the guard, and over the *encoded data* but inside the budget
+ // to exercise what the reader does with a framing it accepted.
let (count_prefix, size_prefix) = match u.int_in_range(0u8..=9)? {
// Honest framing (the common case).
0..=6 => (nobj, objs.len() as i64),
- // Huge object count with the real byte size: the reader must cap
- // the count via `safe_len` instead of trying to decode billions.
- 7 => (u.int_in_range(1i64 << 40..=i64::MAX)?, objs.len() as i64),
+ 7 => match u.int_in_range(0u8..=3)? {
+ // More objects than the block encodes, drawn across the whole
+ // range `safe_len` accepts. Two further guards live in here and
+ // both must hold without spinning: `bound_block_object_count`
+ // rejects a count the payload could not hold, and a count it
+ // accepts has to terminate against the bytes actually present.
+ 0..=2 => (
+ u.int_in_range(nobj + 1..=MAX_ALLOCATION_BYTES)?,
+ objs.len() as i64,
+ ),
+ // Past the budget: `safe_len` must reject before the block bound
+ // is ever consulted.
+ _ => (u.int_in_range(1i64 << 40..=i64::MAX)?, objs.len() as i64),
+ },
// Negative byte size (wraps to an enormous `usize`): `safe_len`
// must reject it rather than attempt a giant allocation.
8 => (nobj, -u.int_in_range(1i64..=i64::MAX)?),
- // Byte size far larger than the bytes actually present → short read.
- _ => (nobj, objs.len() as i64 + (1i64 << 30)),
+ // More bytes than the block carries, but a size `safe_len` accepts,
+ // so `fill_buf` runs and takes the short-read path. A magnitude over
+ // the budget would stop at the guard and never get there.
+ _ => (nobj, objs.len() as i64 + u.int_in_range(1i64..=1i64 << 20)?),
};
encode_long(count_prefix, &mut out);
encode_long(size_prefix, &mut out);
@@ -360,7 +413,7 @@ fn run(mut u: Unstructured) -> arbitrary::Result<()> {
}
// Occasionally truncate the whole file mid-block (short-read path).
- if !out.is_empty() && u.int_in_range(0u8..=7)? == 0 {
+ if u.int_in_range(0u8..=7)? == 0 {
let keep = u.int_in_range(0usize..=out.len())?;
out.truncate(keep);
}
diff --git a/src/avro/fuzz/fuzz_targets/schema_resolve.rs b/src/avro/fuzz/fuzz_targets/schema_resolve.rs
index e0d555ba5d537..73c61937eb0ef 100644
--- a/src/avro/fuzz/fuzz_targets/schema_resolve.rs
+++ b/src/avro/fuzz/fuzz_targets/schema_resolve.rs
@@ -30,8 +30,16 @@
//! trailing field (absent from the writer) with a JSON `default`, driving
//! the "reader field not in writer, use default" branch in `resolve_named`.
//! * union matching. Multi-variant unions whose variants the resolver must
-//! match up by type/name across writer and reader.
-//! * enums with a `default` symbol.
+//! match up across writer and reader, by type for the primitive variants
+//! and by *name* for the record/enum/fixed ones. The reader rendering also
+//! sometimes collapses a union down to the one variant the encoder
+//! expresses (`ResolveUnionConcrete`) and sometimes wraps a concrete node
+//! in a union (`ResolveConcreteUnion`), so all three union arms of
+//! `SchemaResolver::resolve` are reachable, not just union-against-union.
+//! * enums with a `default` symbol, whose reader rendering also drops
+//! trailing symbols. A dropped symbol is what makes the `default`
+//! load-bearing: it becomes an `Err` entry in the resolved enum that decode
+//! substitutes the default for.
//! We resolve writer-against-itself (identity), and both cross-directions.
//!
//! A panic is not the only failure mode, though. `resolve_schemas` can return
@@ -44,16 +52,25 @@
//! sees nothing wrong. So beyond requiring no panic, we add a *decode* oracle:
//! the reader rendering only ever widens the writer, so every node, every
//! union branch included, has a valid reader target, and decoding a
-//! writer-encoded value through the writer→reader resolved schema MUST succeed.
-//! A deferred mismatch turns that decode into an error, which this target
-//! treats as a finding.
+//! writer-encoded value through the writer→reader resolved schema MUST succeed
+//! and MUST consume every byte the writer wrote. A deferred mismatch turns that
+//! decode into an error, which this target treats as a finding.
+//!
+//! Because the reader rendering only widens, both renderings are valid schema
+//! JSON, identity resolution succeeds, and writer→reader resolution succeeds,
+//! all by construction. Those are `expect`s rather than `if let Ok(..)` guards
+//! on purpose. A promotion that regresses at a *non-union* position (a record
+//! field, an array item, a map value) surfaces as a top-level `Err` from
+//! `resolve_schemas`, not as a deferred per-variant `Err`, so a guard there
+//! would switch the decode oracle off for exactly the inputs that found a bug.
+//! Only the narrowing reader→writer direction may legitimately fail.
#![no_main]
use libfuzzer_sys::arbitrary::{self, Unstructured};
use libfuzzer_sys::fuzz_target;
use mz_avro::schema::resolve_schemas;
-use mz_avro::{from_avro_datum, Schema};
+use mz_avro::{Schema, from_avro_datum};
/// One of the primitive Avro types, ordered by promotability so the reader
/// rendering can pick a "wider" target. `int` ⊑ `long` ⊑ `float` ⊑ `double`.
@@ -69,7 +86,16 @@ enum Shape {
/// A primitive that has no promotion (rendered identically on both sides).
OtherPrim(&'static str),
/// `[..]` union with N>=1 variants.
- Union(Vec),
+ Union {
+ variants: Vec,
+ /// The variant the encoder expresses on the wire. Drawn from the fuzz
+ /// input: decoding a `ResolveUnionUnion` only ever consults the
+ /// `permutation` entry for the encoded index, so a branch fixed by the
+ /// generator would leave every other entry's deferred `Err` unobserved.
+ branch: usize,
+ /// Whether the reader rendering drops the union down to `branch` alone.
+ reader_collapse: bool,
+ },
Array(Box),
Map(Box),
Record {
@@ -82,6 +108,10 @@ enum Shape {
name: u32,
/// Whether the reader rendering gives the enum a `default` symbol.
reader_default: bool,
+ /// How many trailing symbols the reader rendering omits. Only ever
+ /// nonzero when `reader_default` is set: dropping a symbol the writer
+ /// can express is a widening exactly because the default catches it.
+ reader_dropped: u8,
},
Fixed {
name: u32,
@@ -119,17 +149,42 @@ fn gen_shape(u: &mut Unstructured, counter: &mut u32, depth: u32) -> arbitrary::
if variants.is_empty() {
variants.push(Shape::Promotable(0));
}
- Shape::Union(variants)
+ // Named variants get distinct names, and a union's duplicate check
+ // keys named variants by name, so any number of them is valid even
+ // when several are the same kind of named type. They are the only
+ // way to reach the name-keyed side of the resolver's variant
+ // matching.
+ for _ in 0..u.int_in_range(0u8..=2)? {
+ variants.push(gen_named(u, counter, depth - 1)?);
+ }
+ let branch = u.int_in_range(0..=variants.len() - 1)?;
+ Shape::Union {
+ variants,
+ branch,
+ // A starved `Unstructured` answers every `int_in_range` with the
+ // low end, so testing against the high end keeps collapsing
+ // rare rather than universal once the input runs out.
+ reader_collapse: u.int_in_range(0u8..=3)? == 3,
+ }
}
3 => Shape::Array(Box::new(gen_shape(u, counter, depth - 1)?)),
4 => Shape::Map(Box::new(gen_shape(u, counter, depth - 1)?)),
- 5 | 6 => {
- *counter += 1;
- let name = *counter;
+ _ => gen_named(u, counter, depth - 1)?,
+ })
+}
+
+/// Generate one *named* shape. Named types are what the resolver matches up by
+/// name rather than by type, both at ordinary positions and as union variants.
+fn gen_named(u: &mut Unstructured, counter: &mut u32, depth: u32) -> arbitrary::Result {
+ let kind = u.int_in_range(0u8..=3)?;
+ *counter += 1;
+ let name = *counter;
+ Ok(match kind {
+ 0 | 1 => {
let n = u.int_in_range(0u8..=3)?;
let mut fields = Vec::with_capacity(n.into());
for _ in 0..n {
- fields.push(gen_shape(u, counter, depth - 1)?);
+ fields.push(gen_shape(u, counter, depth)?);
}
Shape::Record {
name,
@@ -137,20 +192,22 @@ fn gen_shape(u: &mut Unstructured, counter: &mut u32, depth: u32) -> arbitrary::
reader_extra_default: u.int_in_range(0u8..=1)? == 0,
}
}
- 7 => {
- *counter += 1;
+ 2 => {
+ let reader_default = u.int_in_range(0u8..=1)? == 0;
Shape::Enum {
- name: *counter,
- reader_default: u.int_in_range(0u8..=1)? == 0,
- }
- }
- _ => {
- *counter += 1;
- Shape::Fixed {
- name: *counter,
- size: u.int_in_range(1u8..=16)?,
+ name,
+ reader_default,
+ reader_dropped: if reader_default {
+ u.int_in_range(0u8..=2)?
+ } else {
+ 0
+ },
}
}
+ _ => Shape::Fixed {
+ name,
+ size: u.int_in_range(1u8..=16)?,
+ },
})
}
@@ -169,7 +226,7 @@ fn render_writer(shape: &Shape, out: &mut String) {
out.push_str(p);
out.push('"');
}
- Shape::Union(variants) => {
+ Shape::Union { variants, .. } => {
out.push('[');
for (i, v) in variants.iter().enumerate() {
if i > 0 {
@@ -218,13 +275,33 @@ fn render_writer(shape: &Shape, out: &mut String) {
/// Render the *reader* version of `shape`: widens each promotable primitive to
/// a (fuzz-chosen) wider type on the promotion chain, appends a defaulted
-/// `extra` record field, and gives enums a `default` symbol. These are all the
+/// `extra` record field, drops trailing enum symbols in favour of a `default`,
+/// and rewrites unions into and out of concrete types. These are all the
/// schema-evolution shapes `resolve_schemas` handles.
+///
+/// `in_union` says whether `shape` sits directly inside a union, which forbids
+/// wrapping it in another one.
fn render_reader_promoted(
u: &mut Unstructured,
shape: &Shape,
+ in_union: bool,
out: &mut String,
) -> arbitrary::Result<()> {
+ // Reading a concrete writer node as a union that contains a match for it is
+ // a widening, and the only way to reach `ResolveConcreteUnion`. Avro rejects
+ // a union directly inside a union, and `["null","null"]` is a duplicate
+ // variant, hence the two exclusions. Testing against the high end of the
+ // range keeps wrapping rare once the input is exhausted rather than
+ // wrapping every single node.
+ if !in_union
+ && !matches!(shape, Shape::Union { .. } | Shape::OtherPrim("null"))
+ && u.int_in_range(0u8..=3)? == 3
+ {
+ out.push_str("[\"null\",");
+ render_reader_promoted(u, shape, true, out)?;
+ out.push(']');
+ return Ok(());
+ }
match shape {
Shape::Promotable(idx) => {
// Choose a target at or after `idx` on the chain, a valid
@@ -239,18 +316,35 @@ fn render_reader_promoted(
out.push_str(p);
out.push('"');
}
- Shape::Union(variants) => {
+ Shape::Union {
+ variants,
+ branch,
+ reader_collapse,
+ } if *reader_collapse => {
+ // Drop the union down to the single variant the encoder expresses,
+ // so a union writer resolves against a concrete reader
+ // (`ResolveUnionConcrete`). That variant is rendered in its *writer*
+ // form on purpose. Resolution records the one writer variant that
+ // matches the reader's concrete type, and decoding then rejects any
+ // other encoded index. Since a union's variants are unique per type
+ // and per name, an exact rendering is what guarantees the match lands
+ // on `branch`. A widened one can select a different variant, turning
+ // a correct decode into a wrong-index error: `["int","long"]` with
+ // the `long` branch encoded, read as `"float"`, matches `int`, which
+ // comes first and promotes to `float` just as well.
+ render_writer(&variants[*branch], out);
+ }
+ Shape::Union { variants, .. } => {
// Widen the promotable variants while keeping their types distinct.
// Widening each independently could collapse two variants onto the
// same type (`[int, long]` becoming `[long, long]`), which Avro
- // rejects as a duplicate union type. That makes the whole reader
- // schema unparseable, so the writer->reader decode oracle below is
- // silently skipped for this input. We emit the non-promotable
- // variants (only `null`, per `gen_shape`) first, then the
- // promotables in ascending source order with strictly increasing
- // targets. Reserving one chain slot for every later variant keeps a
- // valid assignment reachable, since PROMO_CHAIN has one slot per
- // possible source index.
+ // rejects as a duplicate union type, and an unparseable reader
+ // rendering fails the parse assertion in `run`. We emit the
+ // non-promotable variants (`null` and the named ones) first, then
+ // the promotables in ascending source order with strictly
+ // increasing targets. Reserving one chain slot for every later
+ // variant keeps a valid assignment reachable, since PROMO_CHAIN has
+ // one slot per possible source index.
out.push('[');
let mut written = 0;
for v in variants
@@ -260,7 +354,7 @@ fn render_reader_promoted(
if written > 0 {
out.push(',');
}
- render_reader_promoted(u, v, out)?;
+ render_reader_promoted(u, v, true, out)?;
written += 1;
}
let mut promo: Vec = variants
@@ -290,12 +384,12 @@ fn render_reader_promoted(
}
Shape::Array(item) => {
out.push_str("{\"type\":\"array\",\"items\":");
- render_reader_promoted(u, item, out)?;
+ render_reader_promoted(u, item, false, out)?;
out.push('}');
}
Shape::Map(values) => {
out.push_str("{\"type\":\"map\",\"values\":");
- render_reader_promoted(u, values, out)?;
+ render_reader_promoted(u, values, false, out)?;
out.push('}');
}
Shape::Record {
@@ -311,7 +405,7 @@ fn render_reader_promoted(
out.push(',');
}
out.push_str(&format!("{{\"name\":\"f{i}\",\"type\":"));
- render_reader_promoted(u, f, out)?;
+ render_reader_promoted(u, f, false, out)?;
out.push('}');
}
if *reader_extra_default {
@@ -325,9 +419,18 @@ fn render_reader_promoted(
Shape::Enum {
name,
reader_default,
+ reader_dropped,
} => {
+ // Symbols are dropped from the tail, which keeps `A` around as the
+ // default. A writer symbol the reader lacks becomes an `Err` entry
+ // in the resolved enum, and decoding it substitutes the default.
+ let symbols = match *reader_dropped {
+ 0 => "\"A\",\"B\",\"C\"",
+ 1 => "\"A\",\"B\"",
+ _ => "\"A\"",
+ };
out.push_str(&format!(
- "{{\"type\":\"enum\",\"name\":\"N{name}\",\"symbols\":[\"A\",\"B\",\"C\"]"
+ "{{\"type\":\"enum\",\"name\":\"N{name}\",\"symbols\":[{symbols}]"
));
if *reader_default {
out.push_str(",\"default\":\"A\"");
@@ -365,11 +468,11 @@ fn encode_blob(bytes: &[u8], out: &mut Vec) {
/// Avro-binary-encode one value matching the *writer* rendering of `shape`
/// (i.e. `render_writer`'s wire format), so it can be decoded back through a
-/// resolved schema. For unions we deliberately pick a *promotable* branch:
-/// that is the branch whose resolved decode walks the numeric-promotion path,
-/// the exact spot where #37087 deferred a union match failure into the resolved
-/// schema and re-raised it here at decode time. Every generated union has at
-/// least one promotable variant, so the search never falls back.
+/// resolved schema. A union is encoded at its `Shape::Union::branch`, which the
+/// generator drew from the fuzz input: decoding consults only the resolved
+/// permutation entry for the encoded index, so a deferred `Err` parked in any
+/// other entry, the failure mode of #37087, is invisible unless the branch
+/// varies.
fn encode_writer_value(
u: &mut Unstructured,
shape: &Shape,
@@ -396,13 +499,11 @@ fn encode_writer_value(
encode_blob(&b, out);
}
},
- Shape::Union(variants) => {
- let branch = variants
- .iter()
- .position(|v| matches!(v, Shape::Promotable(_)))
- .unwrap_or(0);
- encode_long(branch as i64, out);
- encode_writer_value(u, &variants[branch], out)?;
+ Shape::Union {
+ variants, branch, ..
+ } => {
+ encode_long(*branch as i64, out);
+ encode_writer_value(u, &variants[*branch], out)?;
}
Shape::Array(item) => {
let n = u.int_in_range(0i64..=3)?;
@@ -446,6 +547,24 @@ fn encode_writer_value(
Ok(())
}
+/// Decode `value` through `resolved`, requiring both that the decode succeeds
+/// and that it consumes every byte the writer wrote. Leftover bytes mean the
+/// resolved schema drove the decoder over less than the writer's value, for
+/// instance a promotion of the wrong width or a field the resolver dropped
+/// without skipping its bytes, which the tail of a record would otherwise hide.
+fn decode_fully(resolved: &Schema, value: &[u8], what: &str) {
+ let mut cursor = value;
+ match from_avro_datum(resolved, &mut cursor) {
+ Ok(_) => assert!(
+ cursor.is_empty(),
+ "decoding the writer's bytes through the {what} left {} of {} bytes unconsumed",
+ cursor.len(),
+ value.len(),
+ ),
+ Err(e) => panic!("decoding the writer's bytes through the {what} failed: {e}"),
+ }
+}
+
fn run(mut u: Unstructured) -> arbitrary::Result<()> {
let mut counter = 0u32;
// The top level of an OCF/registry schema is virtually always a record.
@@ -465,11 +584,17 @@ fn run(mut u: Unstructured) -> arbitrary::Result<()> {
let mut writer_json = String::new();
render_writer(&shape, &mut writer_json);
let mut reader_json = String::new();
- render_reader_promoted(&mut u, &shape, &mut reader_json)?;
+ render_reader_promoted(&mut u, &shape, false, &mut reader_json)?;
- let Ok(writer) = writer_json.parse::() else {
- return Ok(());
- };
+ // Both renderings are valid schema JSON by construction, so a parse failure
+ // is a generator bug. Reporting it beats skipping the input, which would
+ // turn the decode oracle below off without a trace.
+ let writer = writer_json
+ .parse::()
+ .unwrap_or_else(|e| panic!("writer rendering {writer_json} must parse: {e}"));
+ let reader = reader_json
+ .parse::()
+ .unwrap_or_else(|e| panic!("reader rendering {reader_json} must parse: {e}"));
// Encode one value against the writer rendering. We decode it back through
// the resolved schemas below. The writer only ever widens into the reader,
@@ -481,28 +606,38 @@ fn run(mut u: Unstructured) -> arbitrary::Result<()> {
let mut writer_value = Vec::new();
encode_writer_value(&mut u, &shape, &mut writer_value)?;
- // Identity resolution must succeed and decode the writer's own bytes.
- if let Ok(resolved) = resolve_schemas(&writer, &writer) {
- from_avro_datum(&resolved, &mut &writer_value[..])
- .expect("decode through identity-resolved schema must succeed");
- }
- if let Ok(reader) = reader_json.parse::() {
- // Writer→reader is the *widening* direction: it hits the promotion /
- // default / union-match branches, must resolve, and the resolved schema
- // must decode the writer's bytes. A deferred promotion mismatch turns
- // this `expect` into the fuzzer's signal.
- if let Ok(resolved) = resolve_schemas(&writer, &reader) {
- from_avro_datum(&resolved, &mut &writer_value[..]).expect(
- "decode through writer→reader resolved schema must succeed; a failure means \
- resolution deferred a fixable mismatch into the resolved schema (see #37087)",
- );
- }
- // The reverse (reader→writer) *narrows*, so its resolution may
- // legitimately fail or defer a genuine mismatch. We only require that
- // neither direction panics.
- let _ = resolve_schemas(&reader, &writer);
- let _ = resolve_schemas(&reader, &reader);
- }
+ // Identity resolution is the control: a schema always resolves against
+ // itself, and the resolved form always decodes what the writer wrote.
+ let resolved = resolve_schemas(&writer, &writer)
+ .unwrap_or_else(|e| panic!("identity resolution of {writer_json} must succeed: {e}"));
+ decode_fully(
+ &resolved,
+ &writer_value,
+ &format!("identity resolution of {writer_json}"),
+ );
+
+ // Writer→reader is the *widening* direction: it hits the promotion /
+ // default / union-match branches, must resolve, and the resolved schema must
+ // decode the writer's bytes. A promotion the resolver rejects outright fails
+ // the `resolve_schemas` assertion, and one it accepts but defers into the
+ // resolved schema fails the decode.
+ let resolved = resolve_schemas(&writer, &reader).unwrap_or_else(|e| {
+ panic!("the reader rendering only widens, so resolving {writer_json} against {reader_json} must succeed: {e}")
+ });
+ decode_fully(
+ &resolved,
+ &writer_value,
+ &format!("resolution of {writer_json} against {reader_json}"),
+ );
+
+ // The reverse (reader→writer) *narrows*, so its resolution may legitimately
+ // fail or defer a genuine mismatch. We only require that it does not panic.
+ // It is also the direction that reaches the no-match error paths of the
+ // asymmetric union arms: a node the reader wrapped and widened, say an `int`
+ // read as `["null","double"]`, has no match in reverse, because `double`
+ // does not narrow back to `int`.
+ let _ = resolve_schemas(&reader, &writer);
+ let _ = resolve_schemas(&reader, &reader);
Ok(())
}
diff --git a/src/avro/fuzz/prepare-corpus.sh b/src/avro/fuzz/prepare-corpus.sh
index cfa2c6df73569..e2edcac2bef16 100755
--- a/src/avro/fuzz/prepare-corpus.sh
+++ b/src/avro/fuzz/prepare-corpus.sh
@@ -13,6 +13,14 @@
# container files so the fuzzer doesn't waste cycles bouncing off the
# magic-header check. libFuzzer mutates these into deeper structural
# variants while still hitting real decoder code paths.
+#
+# `reader_decode` does not read its input as a container file. It reads it as an
+# `arbitrary::Unstructured` recipe, and only the branch selected by the first
+# byte feeds the remaining bytes to `Reader` verbatim. A container file dropped
+# in as-is is therefore consumed as generator choices and never reaches the
+# magic-header check at all, so each seed is prefixed with the byte that selects
+# that raw branch. `int_in_range(0..=3)` takes one byte from the front and
+# returns it modulo 4, so a leading NUL selects branch 0.
set -euo pipefail
@@ -20,7 +28,8 @@ cd "$(dirname "$0")"
mkdir -p corpus/reader_decode
find corpus/reader_decode -maxdepth 1 -name 'seed_*.avro' -delete
-cp ../benches/quickstop-null.avro corpus/reader_decode/seed_01_quickstop_null.avro
+{ printf '\0'; cat ../benches/quickstop-null.avro; } \
+ > corpus/reader_decode/seed_01_quickstop_null.avro
echo "Seeded:"
for d in corpus/*/; do
diff --git a/src/avro/src/schema.rs b/src/avro/src/schema.rs
index ff1a7c42cdecb..68c51006a5715 100644
--- a/src/avro/src/schema.rs
+++ b/src/avro/src/schema.rs
@@ -2503,6 +2503,49 @@ mod tests {
use super::*;
+ /// Wrap `"int"` in `n` layers of `{"type":"array","items":…}`.
+ fn nest_arrays(n: usize) -> Value {
+ let mut value = Value::String("int".to_string());
+ for _ in 0..n {
+ let mut layer = Map::new();
+ layer.insert("type".to_string(), Value::String("array".to_string()));
+ layer.insert("items".to_string(), value);
+ value = Value::Object(layer);
+ }
+ value
+ }
+
+ #[mz_ore::test]
+ fn parse_rejects_nesting_past_the_depth_limit() {
+ // `MAX_SCHEMA_DEPTH` counts `parse_inner` levels, and the innermost leaf
+ // costs a level without being a JSON container, so `n` wrappers reach
+ // depth `n + 1`.
+ assert_ok!(Schema::parse(&nest_arrays(MAX_SCHEMA_DEPTH - 1)));
+ let err = Schema::parse(&nest_arrays(MAX_SCHEMA_DEPTH))
+ .expect_err("nesting past the limit must be rejected");
+ assert!(
+ err.to_string()
+ .contains(&format!("nesting depth exceeds limit {MAX_SCHEMA_DEPTH}")),
+ "unexpected error: {err}"
+ );
+ }
+
+ #[mz_ore::test]
+ fn from_str_cannot_reach_the_depth_limit() {
+ // Why the test above builds a `Value` rather than parsing text: `from_str`
+ // runs serde_json first, whose own recursion limit is also 128
+ // containers, so it rejects the schema one level before the guard could
+ // fire. A depth-guard test written against text would be asserting
+ // serde_json's error, not ours. If this ever fails, the guard became
+ // reachable from text and deserves a test there too.
+ let text = serde_json::to_string(&nest_arrays(MAX_SCHEMA_DEPTH)).unwrap();
+ let err = Schema::from_str(&text).expect_err("serde_json must reject this depth");
+ assert!(
+ err.to_string().contains("recursion limit exceeded"),
+ "unexpected error: {err}"
+ );
+ }
+
fn check_schema(schema: &str, expected: SchemaPiece) {
let schema = Schema::from_str(schema).unwrap();
assert_eq!(&expected, schema.top_node().inner);
diff --git a/src/catalog-protos/fuzz/Cargo.toml b/src/catalog-protos/fuzz/Cargo.toml
index 09c93c65098da..4b8cb63def46d 100644
--- a/src/catalog-protos/fuzz/Cargo.toml
+++ b/src/catalog-protos/fuzz/Cargo.toml
@@ -16,6 +16,10 @@ cargo-fuzz = true
[dependencies]
libfuzzer-sys = "0.4"
mz-catalog-protos = { path = "..", features = ["proptest"] }
+# For `Jsonb`, so the oracle can assert the round trip the durable catalog
+# actually performs (`StateUpdateKindJson` packs the serde value into a `Row`)
+# rather than only the JSON text form, which never touches that stage.
+mz-repr = { path = "../../repr" }
proptest = "1.11.0"
serde = "1.0.219"
serde_json = "1.0.150"
diff --git a/src/catalog-protos/fuzz/catalog_objects_serde_roundtrip.dict b/src/catalog-protos/fuzz/catalog_objects_serde_roundtrip.dict
new file mode 100644
index 0000000000000..f9c640a620148
--- /dev/null
+++ b/src/catalog-protos/fuzz/catalog_objects_serde_roundtrip.dict
@@ -0,0 +1,100 @@
+# libFuzzer dictionary for the catalog_objects_serde_roundtrip target.
+#
+# The raw-bytes arm feeds the input to `serde_json::from_slice::`, which needs
+# a complete JSON object with the exact field names and variant tags before it
+# returns anything at all. The nested types (`ClusterValue`, `ItemValue`,
+# `RoleValue`, `NetworkPolicyValue`, `ClusterReplicaValue`) are out of reach for
+# byte mutation without them, so that arm would spend the whole run bailing on a
+# failed decode. Only the short types (`ConfigValue`, `SettingValue`, `RoleId`)
+# are reachable unaided.
+#
+# Structure.
+"{}"
+"[]"
+":"
+","
+"null"
+"true"
+"false"
+
+# `StateUpdateKind` is `#[serde(tag = "kind")]`, so its discriminant is a field.
+"\"kind\""
+"\"key\""
+"\"value\""
+
+# `StateUpdateKind` variants.
+"\"AuditLog\""
+"\"Cluster\""
+"\"ClusterIntrospectionSourceIndex\""
+"\"ClusterReplica\""
+"\"Comment\""
+"\"Config\""
+"\"Database\""
+"\"DefaultPrivileges\""
+"\"FenceToken\""
+"\"GidMapping\""
+"\"IdAlloc\""
+"\"Item\""
+"\"NetworkPolicy\""
+"\"Role\""
+"\"RoleAuth\""
+"\"Schema\""
+"\"ServerConfiguration\""
+"\"Setting\""
+"\"SourceReferences\""
+"\"StorageCollectionMetadata\""
+"\"SystemPrivileges\""
+"\"TxnWalShard\""
+"\"UnfinalizedShard\""
+
+# Field names reachable from the fuzzed types.
+"\"acl_mode\""
+"\"attributes\""
+"\"auto_scaling_strategy\""
+"\"availability_zones\""
+"\"catalog_id\""
+"\"cluster_id\""
+"\"config\""
+"\"definition\""
+"\"entries\""
+"\"extra_versions\""
+"\"fingerprint\""
+"\"global_id\""
+"\"grantee\""
+"\"grantor\""
+"\"inherit\""
+"\"location\""
+"\"logging\""
+"\"membership\""
+"\"name\""
+"\"oid\""
+"\"owner_id\""
+"\"privileges\""
+"\"replication_factor\""
+"\"rules\""
+"\"schedule\""
+"\"schema_id\""
+"\"size\""
+"\"superuser\""
+"\"variant\""
+"\"vars\""
+"\"version\""
+"\"workload_class\""
+"\"create_sql\""
+
+# Variant tags of the nested enums: `RoleId`/`GlobalId`, `ClusterVariant`,
+# `ReplicaLocation`, `CatalogItem`, `RoleVar`, `ClusterSchedule`.
+"\"System\""
+"\"User\""
+"\"Public\""
+"\"Predefined\""
+"\"Transient\""
+"\"Explain\""
+"\"IntrospectionSourceIndex\""
+"\"Managed\""
+"\"Unmanaged\""
+"\"Manual\""
+"\"Refresh\""
+"\"Flat\""
+"\"SqlSet\""
+"\"V1\""
diff --git a/src/catalog-protos/fuzz/fuzz_targets/catalog_objects_serde_roundtrip.rs b/src/catalog-protos/fuzz/fuzz_targets/catalog_objects_serde_roundtrip.rs
index d2a9171f43fc9..e5559ffde3131 100644
--- a/src/catalog-protos/fuzz/fuzz_targets/catalog_objects_serde_roundtrip.rs
+++ b/src/catalog-protos/fuzz/fuzz_targets/catalog_objects_serde_roundtrip.rs
@@ -7,21 +7,34 @@
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0.
-//! Fuzz target: catalog object serde JSON round-trip is idempotent. The
-//! catalog state is durable on-disk data, so a serde edge case that loses
-//! information through a JSON round trip is a catalog-corruption risk.
+//! Fuzz target: catalog object serde round-trips are lossless. The catalog
+//! state is durable on-disk data, so an encoding edge case that loses
+//! information is a catalog-corruption risk.
+//!
+//! The durable form is not JSON *text*. `StateUpdateKindJson` packs the serde
+//! value into a `Jsonb`, i.e. an `mz_repr::Row`, and reads it back out through
+//! `to_serde_json` + `from_value`. That JSONB leg is the one that can lose
+//! information: every number becomes a `Datum::Numeric` and object keys are
+//! deduplicated and reordered by the `Row` map encoding. So the oracle asserts
+//! that leg, not just `to_vec`/`from_slice`, which never touches it.
//!
//! Two complementary input arms (the first byte picks the arm, the next byte
-//! picks which catalog type to exercise):
+//! picks which catalog type to exercise). Both arms dispatch over the same type
+//! list in the same order, so a corpus entry's type byte means the same thing in
+//! either arm:
//!
//! * **Structured arm.** Drives the catalog type's proptest `Arbitrary`
//! (behind mz-catalog-protos' `proptest` feature) from the libFuzzer byte
-//! stream to synthesize a *valid, deeply-populated* value, then asserts the
-//! full `value -> JSON -> value -> JSON` chain is idempotent. We deliberately
-//! target the genuinely nested catalog types: `ClusterValue`
-//! (`RoleId` + `Vec` + the `ClusterConfig`/`ClusterVariant`/
-//! `ManagedCluster`/`ClusterSchedule` tree), `ItemValue` (the `CatalogItem`
-//! enum + `GlobalId` enum + `Vec`), `RoleValue` (the
+//! stream to synthesize a *valid, deeply-populated* value, then asserts both
+//! round trips. The list leads with `StateUpdateKind`, the durable envelope
+//! every catalog write goes through and the one member of this family whose
+//! round trip is not trivially total: it is `#[serde(tag = "kind")]`, so it
+//! deserializes through serde's content-buffering path rather than straight
+//! from the input. The rest are the genuinely nested values reached through
+//! it: `ClusterValue` (`RoleId` + `Vec` + the
+//! `ClusterConfig`/`ClusterVariant`/`ManagedCluster`/`ClusterSchedule` tree),
+//! `ItemValue` (the `CatalogItem` enum + `GlobalId` enum +
+//! `Vec`), `RoleValue` (the
//! `RoleAttributes`/`RoleMembership`/`RoleVars`/`RoleVar` tree),
//! `NetworkPolicyValue` (`Vec`), and `ClusterReplicaValue`
//! (the `ReplicaConfig`/`ReplicaLocation` enum). Random JSON bytes almost
@@ -29,15 +42,18 @@
//! serde branches actually get covered.
//! * **Raw-bytes arm.** Deserializes arbitrary bytes straight into the type,
//! exercising the deserializer against malformed/adversarial JSON input,
-//! then re-serializes the recovered value.
+//! then round-trips the recovered value. The nested types need a complete
+//! object with the right field names and variant tags to get past the decode,
+//! which is what `catalog_objects_serde_roundtrip.dict` supplies.
#![no_main]
use libfuzzer_sys::fuzz_target;
use mz_catalog_protos::objects::{
ClusterConfig, ClusterReplicaValue, ClusterValue, ConfigValue, GidMappingValue, ItemValue,
- MzAclItem, NetworkPolicyValue, RoleId, RoleValue, SettingValue,
+ MzAclItem, NetworkPolicyValue, RoleId, RoleValue, SettingValue, StateUpdateKind,
};
+use mz_repr::adt::jsonb::Jsonb;
use proptest::strategy::{Strategy, ValueTree};
use proptest::test_runner::{Config, RngAlgorithm, TestRng, TestRunner};
@@ -69,17 +85,28 @@ where
assert_idempotent(tree.current());
}
-/// `value -> JSON -> value` must be the identity, and re-serializing must
-/// produce byte-identical JSON.
+/// `value -> encoded -> value` must be the identity, through both the JSON text
+/// form and the JSONB form the durable catalog actually stores.
fn assert_idempotent(orig: T)
where
T: serde::de::DeserializeOwned + serde::Serialize + PartialEq + std::fmt::Debug,
{
+ // JSON text: what a `to_vec`/`from_slice` consumer sees.
let json = serde_json::to_vec(&orig).expect("serialize of valid value must succeed");
let round: T = serde_json::from_slice(&json).expect("re-decode must round-trip");
assert_eq!(orig, round, "serde roundtrip changed value");
- let json2 = serde_json::to_vec(&round).expect("re-serialize must succeed");
- assert_eq!(json, json2, "serde re-serialize was not idempotent");
+
+ // The durable path, `StateUpdateKindJson::from_serde` / `try_to_serde`: pack
+ // the serde value into a `Row` and read it back. Numbers survive as
+ // `Datum::Numeric` only because every integer in these types fits in
+ // `Numeric`'s 39 digits (`u64::MAX` is 20) and `to_standard_notation_string`
+ // never emits exponent notation, which is a property worth asserting rather
+ // than assuming.
+ let value = serde_json::to_value(&orig).expect("serialize to a value must succeed");
+ let jsonb = Jsonb::from_serde_json(value).expect("catalog value must pack as jsonb");
+ let via_jsonb: T = serde_json::from_value(jsonb.as_ref().to_serde_json())
+ .expect("jsonb must round-trip back into the catalog type");
+ assert_eq!(orig, via_jsonb, "jsonb roundtrip changed value");
}
/// Decode adversarial JSON bytes straight into `T`, then assert the recovered
@@ -94,6 +121,28 @@ where
assert_idempotent(orig);
}
+/// Dispatch `$arm` over the catalog types. One list for both arms, so the type
+/// byte selects the same type either way and a corpus entry stays meaningful
+/// across a one-bit change to the arm byte.
+macro_rules! dispatch {
+ ($which:expr, $arm:ident, $rest:expr) => {
+ match $which % 12 {
+ 0 => $arm::($rest),
+ 1 => $arm::($rest),
+ 2 => $arm::($rest),
+ 3 => $arm::($rest),
+ 4 => $arm::($rest),
+ 5 => $arm::($rest),
+ 6 => $arm::($rest),
+ 7 => $arm::($rest),
+ 8 => $arm::($rest),
+ 9 => $arm::($rest),
+ 10 => $arm::($rest),
+ _ => $arm::($rest),
+ }
+ };
+}
+
fuzz_target!(|data: &[u8]| {
let Some((&mode, rest)) = data.split_first() else {
return;
@@ -104,31 +153,9 @@ fuzz_target!(|data: &[u8]| {
if mode & 1 == 0 {
// Structured arm: synthesize a valid, deeply-nested value.
- match which % 10 {
- 0 => structured_roundtrip::(rest),
- 1 => structured_roundtrip::(rest),
- 2 => structured_roundtrip::(rest),
- 3 => structured_roundtrip::(rest),
- 4 => structured_roundtrip::(rest),
- 5 => structured_roundtrip::(rest),
- 6 => structured_roundtrip::(rest),
- 7 => structured_roundtrip::(rest),
- 8 => structured_roundtrip::(rest),
- _ => structured_roundtrip::(rest),
- }
+ dispatch!(which, structured_roundtrip, rest)
} else {
// Raw-bytes arm: decode adversarial JSON, then round-trip.
- match which % 10 {
- 0 => raw_roundtrip::(rest),
- 1 => raw_roundtrip::(rest),
- 2 => raw_roundtrip::(rest),
- 3 => raw_roundtrip::(rest),
- 4 => raw_roundtrip::(rest),
- 5 => raw_roundtrip::(rest),
- 6 => raw_roundtrip::(rest),
- 7 => raw_roundtrip::(rest),
- 8 => raw_roundtrip::(rest),
- _ => raw_roundtrip::(rest),
- }
+ dispatch!(which, raw_roundtrip, rest)
}
});
diff --git a/src/catalog-protos/fuzz/prepare-corpus.sh b/src/catalog-protos/fuzz/prepare-corpus.sh
new file mode 100755
index 0000000000000..f3c7ffce90b99
--- /dev/null
+++ b/src/catalog-protos/fuzz/prepare-corpus.sh
@@ -0,0 +1,67 @@
+#!/usr/bin/env bash
+
+# Copyright Materialize, Inc. and contributors. All rights reserved.
+#
+# Use of this software is governed by the Business Source License
+# included in the LICENSE file at the root of this repository.
+#
+# As of the Change Date specified in that file, in accordance with
+# the Business Source License, use of this software will be governed
+# by the Apache License, Version 2.0.
+#
+# prepare-corpus.sh: seed the catalog_objects_serde_roundtrip corpus with one
+# valid document per catalog type for the raw-bytes arm.
+#
+# That arm feeds the input straight to `serde_json::from_slice::`, which
+# returns nothing at all until the input is a complete object with the right
+# field names and variant tags. For the nested types that is far out of reach of
+# byte mutation starting from nothing: a run seeded only by libFuzzer itself
+# keeps a corpus of 2-4 byte entries, because growing an input yields no new
+# coverage until it parses, so there is no gradient to climb. The dictionary
+# supplies the tokens, and these seeds supply the shape to insert them into.
+#
+# Each seed carries the two-byte prefix the target reads first: an odd `mode`
+# byte to select the raw arm, then the type index. The JSON is deliberately
+# minimal rather than a dump of a realistic value, so libFuzzer has short inputs
+# to mutate.
+#
+# A seed that stops parsing (a field added to one of these types, say) costs
+# only the coverage it was buying, never a false pass: the target's oracle runs
+# on whatever it manages to decode. Field sets were taken from the types
+# themselves, all twelve verified to deserialize at the time of writing.
+
+set -euo pipefail
+
+cd "$(dirname "$0")"
+
+corpus=corpus/catalog_objects_serde_roundtrip
+mkdir -p "$corpus"
+find "$corpus" -maxdepth 1 -name 'seed_*' -delete
+
+# `mode` byte 0x01 selects the raw arm; the second byte is the type index in the
+# target's `dispatch!` list.
+seed() {
+ local index="$1" name="$2" json="$3"
+ local file
+ file="$corpus/seed_$(printf '%02d' "$index")_$name"
+ printf '%b' "\\0001\\0$(printf '%03o' "$index")" > "$file"
+ printf '%s' "$json" >> "$file"
+}
+
+cluster_value='{"name":"c","owner_id":"Public","privileges":[],"config":{"workload_class":null,"variant":"Unmanaged"}}'
+
+seed 0 state_update_kind "{\"kind\":\"Cluster\",\"key\":{\"id\":{\"User\":1}},\"value\":$cluster_value}"
+seed 1 cluster_value "$cluster_value"
+seed 2 item_value '{"schema_id":{"User":1},"name":"i","definition":{"V1":{"create_sql":"SELECT 1"}},"owner_id":"Public","privileges":[],"oid":1,"global_id":{"User":1},"extra_versions":[]}'
+seed 3 role_value '{"name":"r","attributes":{"inherit":true,"superuser":null,"login":null,"auto_provision_source":null},"membership":{"map":[]},"vars":{"entries":[]},"oid":1}'
+seed 4 network_policy_value '{"name":"n","rules":[{"name":"a","address":"0.0.0.0/0","action":"Allow","direction":"Ingress"}],"owner_id":"Public","privileges":[],"oid":1}'
+seed 5 cluster_replica_value '{"cluster_id":{"User":1},"name":"r1","config":{"logging":{"log_logging":false,"interval":null},"location":{"Managed":{"size":"1","availability_zones":[],"internal":false,"billed_as":null,"pending":false}},"arrangement_compression":false},"owner_id":"Public"}'
+seed 6 cluster_config '{"workload_class":null,"variant":"Unmanaged"}'
+seed 7 gid_mapping_value '{"catalog_id":1,"global_id":1,"fingerprint":"f"}'
+seed 8 mz_acl_item '{"grantee":"Public","grantor":{"User":1},"acl_mode":{"bitflags":0}}'
+seed 9 role_id '{"User":1}'
+seed 10 config_value '{"value":1}'
+seed 11 setting_value '{"value":"s"}'
+
+echo "Seeded:"
+printf " %-40s %4d seeds\n" "$corpus/" "$(find "$corpus" -maxdepth 1 -name 'seed_*' | wc -l)"
diff --git a/src/expr/fuzz/fuzz_targets/agg_decompose.rs b/src/expr/fuzz/fuzz_targets/agg_decompose.rs
index 95507c9dee885..ddb8c539a4581 100644
--- a/src/expr/fuzz/fuzz_targets/agg_decompose.rs
+++ b/src/expr/fuzz/fuzz_targets/agg_decompose.rs
@@ -19,38 +19,86 @@
//! * **Hierarchical re-aggregation** (min/max/any/all, idempotent aggregates
//! whose output type equals their input type): `agg(whole)` must equal
//! `agg([agg(chunk0), agg(chunk1), ...])` over the non-empty chunks of an
-//! arbitrary partition. This is exactly how the bucketed hierarchical reduce
-//! computes min/max, and it must hold even with nulls present in the data.
+//! arbitrary partition, and it must hold even with nulls present in the data.
+//! For min/max this is exactly how the bucketed hierarchical reduce works.
+//! `any`/`all` are maintained accumulably in the dataflow instead
+//! (`mz_compute_types::plan::reduce::reduction_type` groups them with the sums
+//! and `Count`, and the accumulator keeps bool counts rather than partial
+//! booleans), but both are idempotent bool monoid homomorphisms, so the same
+//! law holds and pins down `eval`'s three-valued folding.
//! * **Additive decomposition**: `count(whole)` must equal the sum of the
//! per-chunk counts. Likewise `sum(whole)` must equal the sum of the
//! per-chunk sums (this is exactly how accumulable maintenance combines
//! partial sums across batches). We check the sum law for the *integer* sums
//! (`SumInt32`/`SumInt64`), where the per-chunk combination is exact.
+//! * **Expansion equivalence** (every aggregate): a row at multiplicity `k` must
+//! aggregate exactly like `k` rows at multiplicity one.
//!
//! We generate a random multiset of nullable datums in one of several type
//! groups, a random permutation of it, and a random partition into chunks, then
//! check the applicable laws for each aggregate over the chosen group.
//!
+//! Every generated row carries a random *positive* multiplicity, because
+//! `AggregateFunc::eval` consumes `(datum, diff)` pairs and dispatches on the
+//! diff three ways: `count` sums the diffs, the signed integer sums scale each
+//! value by its diff, and multiplicity-insensitive aggregates
+//! (`AggregateFunc::ignores_multiplicity`) drop it. Constant folding is the only
+//! caller that ever passes a diff other than one, because
+//! `FoldConstants::fold_reduce_constant` consolidates the constant's rows before
+//! handing them to `eval` while the dataflow's reduce renderers pass `Diff::ONE`
+//! deliberately. So a divergence in that arithmetic is a silent wrong answer
+//! reachable only through constant folding, and this is the only coverage of it.
+//!
+//! The expansion law is what actually pins that arithmetic down, and the
+//! permutation and decomposition laws cannot substitute for it. Both of those
+//! hold for *any* per-row function summed over the input, so they stay satisfied
+//! if `count` counts rows instead of summing diffs, or if the sum drops its
+//! `diff` factor: row-counting is additive too. Only comparing against the
+//! expanded form distinguishes them. What no law here can detect is a change to
+//! `ignores_multiplicity` membership for min/max, since routing those through
+//! `expand_counts` instead is genuinely equivalent at positive diffs.
+//!
+//! Retractions are deliberately out of scope. The signed sums finalize
+//! `accum == 0 && non_nulls == 0` to `Null`, which stops being additive once
+//! diffs can cancel: `[(0, -1)]` and `[(0, +1)]` each finalize to `0` while
+//! their union finalizes to `Null`. Covering retractions needs a law stated over
+//! accumulators rather than over finalized datums.
+//!
//! Groups: `int4`/`int8`/`bool` (exact integers/booleans, plain equality
//! oracle), `text` (lexicographic min/max), plus `float8` and `numeric`. The
//! float/numeric groups exercise the `OrderedFloat`/`OrderedDecimal` ordering
-//! used by min/max and feed in the tricky special values (`NaN`, `±Inf`,
-//! `-0.0`). We only apply min/max to the float/numeric groups: floating-point
-//! and bounded-decimal *sum* is not
-//! associative under rounding, so an additive/permutation law over it would be
-//! a generator artifact rather than a real product invariant. Datum equality is
-//! `OrderedFloat`-based (so `NaN == NaN`), so it is a sound oracle for the
-//! ordering aggregates even with NaN present.
+//! used by min/max. `float8` feeds in the full set of IEEE-754 corners (`NaN`,
+//! `±Inf`, `-0.0`); `numeric` feeds the specials a numeric datum can actually
+//! hold, which is `NaN` plus equal-value-different-scale pairs (see
+//! `SPECIAL_NUMERIC`). We only apply min/max to the float/numeric groups:
+//! floating-point and bounded-decimal *sum* is not associative under rounding,
+//! so an additive/permutation law over it would be a generator artifact rather
+//! than a real product invariant.
+//!
+//! Datum equality is the oracle throughout, and it is sound for the ordering
+//! aggregates because it agrees with the `Ord` that `max_datum`/`min_datum`
+//! select by: `Datum`'s derived `PartialEq` delegates to `OrderedFloat` (so
+//! `NaN == NaN`) and to `OrderedDecimal`, whose `PartialEq` is defined as
+//! `cmp(..) == Equal`. So min/max returning any one of several mutually equal
+//! candidates can never trip an assertion here.
#![no_main]
use libfuzzer_sys::arbitrary::{self, Arbitrary, Unstructured};
use libfuzzer_sys::fuzz_target;
use mz_expr::AggregateFunc;
-use mz_repr::{Datum, Diff, RowArena};
+use mz_repr::{Datum, Diff, RowArena, strconv};
const MAX_ROWS: usize = 24;
const MAX_CHUNKS: usize = 4;
+/// Upper bound on a row's multiplicity. `MAX_ROWS * MAX_DIFF * 2^63` is far
+/// inside `i128` and inside numeric's 39 digits of precision, so every partial
+/// sum below is exact: `sum_signed_int_counted`'s wrapping arithmetic never
+/// wraps, and `SumInt32`'s narrowing to `i64` never truncates.
+const MAX_DIFF: u8 = 4;
+
+/// A generated input row: a datum paired with its multiplicity.
+type Update = (Datum<'static>, Diff);
#[derive(Clone, Copy)]
enum Group {
@@ -70,8 +118,8 @@ enum Group {
/// (`"a"`/`"ab"`) where lexicographic ordering is subtle.
const POOL_STR: &[&str] = &["", "a", "ab", "abc", "b", "Z", "z", "10", "9"];
-/// Which decomposition laws apply to an aggregate (permutation invariance always
-/// applies and is checked separately).
+/// Which decomposition law applies to an aggregate. Permutation invariance and
+/// expansion equivalence apply to every aggregate and are checked separately.
enum Law {
/// Idempotent, output type == input type: `agg(whole) == agg(map(agg, parts))`.
Hierarchical,
@@ -97,6 +145,29 @@ const SPECIAL_F64: &[f64] = &[
f64::MAX,
];
+/// Numeric values worth probing. `Datum::from(i128)` only ever yields canonical,
+/// finite, exponent-0 numerics, which makes the integer majority of this group a
+/// relabelling of `Group::Int64` through a different comparison function. These
+/// add the two datum shapes that actually exercise `OrderedDecimal`'s ordering:
+/// `NaN` (whose `Ord` sorts above every finite value), and numerically equal
+/// values at different scales, which `OrderedDecimal` reduces before comparing
+/// and so calls equal despite differing bit patterns. Both min/max and the
+/// equality oracle have to agree on those.
+///
+/// Every entry must be a value a numeric *datum* can actually hold, otherwise a
+/// panic here would be a generator artifact rather than a product bug.
+/// `±Infinity` and `-0` are excluded for that reason: `strconv::parse_numeric`
+/// rejects a non-overflow infinity outright, `numeric::munge_numeric` folds `-0`
+/// to `0` and `-NaN` to `NaN`, and numeric arithmetic returns
+/// `EvalError::FloatOverflow` rather than saturating to an infinity. `NaN` is the
+/// one special that survives, via `'NaN'::numeric`.
+const SPECIAL_NUMERIC: &[&str] = &["NaN", "0", "0.000", "100", "100.00", "1E+2", "-0.5"];
+
+/// A positive multiplicity. See the module doc for why retractions are excluded.
+fn gen_diff(u: &mut Unstructured) -> arbitrary::Result {
+ Ok(Diff::from(i64::from(u.int_in_range(1..=MAX_DIFF)?)))
+}
+
fn gen_datum(u: &mut Unstructured, group: Group) -> arbitrary::Result> {
if u.ratio(1u8, 5u8)? {
return Ok(Datum::Null);
@@ -124,10 +195,17 @@ fn gen_datum(u: &mut Unstructured, group: Group) -> arbitrary::Result {
- // Integer-valued numerics keep min/max exact and easy to read. The
- // point of the group is the `OrderedDecimal` comparison path, not
- // fractional precision.
- Datum::from(i128::from(i64::arbitrary(u)?))
+ // Bias in the specials at the same ratio as the float group. The
+ // majority stays integer-valued, which keeps min/max exact and easy
+ // to read, but a fully random i64 never lands on a corner case.
+ if u.ratio(1u8, 2u8)? {
+ let i = u.int_in_range(0..=SPECIAL_NUMERIC.len() - 1)?;
+ Datum::from(
+ strconv::parse_numeric(SPECIAL_NUMERIC[i]).expect("literal numeric parses"),
+ )
+ } else {
+ Datum::from(i128::from(i64::arbitrary(u)?))
+ }
}
Group::Str => {
let i = u.int_in_range(0..=POOL_STR.len() - 1)?;
@@ -181,10 +259,7 @@ fn aggregates(group: Group) -> Vec<(AggregateFunc, Law)> {
}
/// A Fisher-Yates shuffle driven by the fuzz input.
-fn shuffle(
- u: &mut Unstructured,
- input: &[Datum<'static>],
-) -> arbitrary::Result>> {
+fn shuffle(u: &mut Unstructured, input: &[Update]) -> arbitrary::Result> {
let mut v = input.to_vec();
for i in (1..v.len()).rev() {
let j = u.int_in_range(0..=i)?;
@@ -195,15 +270,12 @@ fn shuffle(
/// Randomly assign each input to one of `1..=MAX_CHUNKS` chunks (some may be
/// empty). The chunks' concatenation is a permutation of the input multiset.
-fn partition(
- u: &mut Unstructured,
- input: &[Datum<'static>],
-) -> arbitrary::Result>>> {
+fn partition(u: &mut Unstructured, input: &[Update]) -> arbitrary::Result>> {
let k = u.int_in_range(1usize..=MAX_CHUNKS)?;
let mut chunks = vec![Vec::new(); k];
- for &d in input {
+ for &update in input {
let b = u.int_in_range(0..=k - 1)?;
- chunks[b].push(d);
+ chunks[b].push(update);
}
Ok(chunks)
}
@@ -217,9 +289,9 @@ fn as_count(d: Datum) -> i64 {
/// Decode an integer-sum result datum to an exact `i128`. `SumInt32` yields an
/// `Int64`, `SumInt64` yields an (integer-valued) `Numeric`. An empty/all-null
-/// chunk yields `Null` (returned as `None`). The values are bounded (<= 24
-/// inputs of at most i64 magnitude), so every partial sum fits an `i128`
-/// exactly and the per-chunk combination below is lossless.
+/// chunk yields `Null` (returned as `None`). The values are bounded (see
+/// `MAX_DIFF`), so every partial sum fits an `i128` exactly and the per-chunk
+/// combination below is lossless.
fn as_sum(d: Datum) -> Option {
match d {
Datum::Null => None,
@@ -244,37 +316,56 @@ fn run(u: &mut Unstructured) -> arbitrary::Result<()> {
let n = u.int_in_range(0usize..=MAX_ROWS)?;
let mut input = Vec::with_capacity(n);
for _ in 0..n {
- input.push(gen_datum(u, group)?);
+ input.push((gen_datum(u, group)?, gen_diff(u)?));
}
let permuted = shuffle(u, &input)?;
let chunks = partition(u, &input)?;
+ // NOTE: expansion is only faithful because every diff is >= 1. At diff 0 the
+ // row would vanish here while `ignores_multiplicity` aggregates still see it,
+ // so min/max would diverge for reasons that are not a product bug.
+ let expanded: Vec = input
+ .iter()
+ .flat_map(|&(d, diff)| {
+ let copies = usize::try_from(diff.into_inner()).expect("positive diff");
+ std::iter::repeat((d, Diff::ONE)).take(copies)
+ })
+ .collect();
let arena = RowArena::new();
for (agg, law) in aggregates(group) {
- // `AggregateFunc::eval` consumes `(datum, multiplicity)` pairs. Each
- // generated row has multiplicity one, so pair every datum with
- // `Diff::ONE`.
- let whole = agg.eval(input.iter().map(|&d| (d, Diff::ONE)), &arena);
+ let whole = agg.eval(input.iter().copied(), &arena);
// Permutation invariance: order must never matter.
- let shuffled = agg.eval(permuted.iter().map(|&d| (d, Diff::ONE)), &arena);
+ let shuffled = agg.eval(permuted.iter().copied(), &arena);
assert_eq!(
whole, shuffled,
"{agg:?} is not permutation-invariant\n input = {input:?}\n permuted = {permuted:?}"
);
+ // Expansion equivalence: a row at multiplicity `k` must aggregate
+ // exactly like `k` rows at multiplicity one.
+ let unit_diffs = agg.eval(expanded.iter().copied(), &arena);
+ assert_eq!(
+ whole, unit_diffs,
+ "{agg:?} does not treat a diff of k like k unit rows\n input = {input:?}\n expanded = {expanded:?}"
+ );
+
match law {
Law::Hierarchical => {
- // agg(whole) == agg(map(agg, non-empty chunks)). Empty chunks are
- // skipped: an empty chunk aggregates to null, and for any/all
- // (three-valued) a stray null would corrupt an otherwise false/true
- // result (`false OR null = null`). Min/max absorb null as their
- // identity, but skipping is correct for all of them.
+ // agg(whole) == agg(map(agg, non-empty chunks)). Skipping the
+ // empty chunks keeps the partials to the results of non-trivial
+ // work. It is a no-op for these aggregates: min/max aggregate an
+ // empty chunk to `Null` and filter nulls back out on the way in,
+ // and any/all fold from their identity (`False`/`True`), so an
+ // empty chunk contributes nothing either way.
let partials: Vec = chunks
.iter()
.filter(|c| !c.is_empty())
- .map(|c| agg.eval(c.iter().map(|&d| (d, Diff::ONE)), &arena))
+ .map(|c| agg.eval(c.iter().copied(), &arena))
.collect();
+ // The partials are values, not updates, so each enters the
+ // re-aggregation once. Every aggregate under this law ignores
+ // multiplicity regardless.
let reaggregated = agg.eval(partials.iter().map(|&d| (d, Diff::ONE)), &arena);
assert_eq!(
whole, reaggregated,
@@ -285,7 +376,7 @@ fn run(u: &mut Unstructured) -> arbitrary::Result<()> {
// count(whole) == sum(map(count, chunks))
let total: i64 = chunks
.iter()
- .map(|c| as_count(agg.eval(c.iter().map(|&d| (d, Diff::ONE)), &arena)))
+ .map(|c| as_count(agg.eval(c.iter().copied(), &arena)))
.sum();
assert_eq!(
as_count(whole),
@@ -300,7 +391,7 @@ fn run(u: &mut Unstructured) -> arbitrary::Result<()> {
// whole is also Null, so both sides are "no partials" and match.
let partials: Vec = chunks
.iter()
- .filter_map(|c| as_sum(agg.eval(c.iter().map(|&d| (d, Diff::ONE)), &arena)))
+ .filter_map(|c| as_sum(agg.eval(c.iter().copied(), &arena)))
.collect();
let combined: Option = if partials.is_empty() {
None
diff --git a/src/expr/fuzz/fuzz_targets/build_regex.rs b/src/expr/fuzz/fuzz_targets/build_regex.rs
index a752cb1d2124e..7b8ec722eef1f 100644
--- a/src/expr/fuzz/fuzz_targets/build_regex.rs
+++ b/src/expr/fuzz/fuzz_targets/build_regex.rs
@@ -10,8 +10,12 @@
//! Fuzz target: `func::build_regex` compiles an untrusted regular expression
//! (and flags) for the `regexp_*` SQL functions, and the result matches
//! untrusted text. A user controls both, so a panic compiling or matching is a
-//! real availability bug. The `regex` crate is size-limited, so an oversized
-//! pattern returns an error rather than OOMing. Single-match ops
+//! real availability bug. Two independent size limits guard that, and both are
+//! under test here: the `regex` crate's `size_limit` bounds the *compiled* NFA,
+//! and `MAX_REGEX_SIZE_BEFORE_COMPILATION` bounds `pattern.len()` before
+//! compilation, because the memory a compile spends translating the pattern is
+//! not something `size_limit` ever sees. Exceeding either must yield an error
+//! rather than an OOM or a hang. Single-match ops
//! (`is_match`/`find`/`captures`) run in linear time, but the all-matches ops
//! (`replace_all`/`split`) re-scan from every match via `find_iter` and are
//! superlinear in the text length for adversarial patterns, so the match text
@@ -32,6 +36,10 @@
//! * occasional near-size-limit patterns built from nested counted quantifiers,
//! which push the compiler toward its state-count / size limit (where it must
//! return an error rather than hang or OOM) and toward deep AST nesting.
+//! * rare very long patterns, built by repeating one small unit, which straddle
+//! `MAX_REGEX_SIZE_BEFORE_COMPILATION`. These are the only way to reach the
+//! compile-time memory cost, which scales with the *source* length and so is
+//! invisible to every other arm (see `gen_long_pattern`).
#![no_main]
@@ -145,9 +153,12 @@ fn gen_regex(
}
/// Builds a deeply nested chain of counted quantifiers whose multiplied bounds
-/// approach the regex crate's compiled-size limit, e.g. `(?:(?:a{40}){40}){40}`.
-/// `build_regex` must reject this with an error (PatternTooLarge or the regex
-/// crate's CompiledTooBig) rather than hang or OOM.
+/// approach the regex crate's compiled-size limit, e.g. `(?:(?:a){40}){40}`.
+///
+/// These patterns stay tiny in source form, a few dozen bytes, so
+/// `MAX_REGEX_SIZE_BEFORE_COMPILATION` never fires on them. The regex crate's
+/// own `size_limit` is what must reject them, with `CompiledTooBig`, rather than
+/// hanging or OOMing. `gen_long_pattern` covers the other limit.
fn gen_near_limit(u: &mut Unstructured, out: &mut String) -> arbitrary::Result<()> {
let layers = u.int_in_range(2usize..=5)?;
for _ in 0..layers {
@@ -160,20 +171,64 @@ fn gen_near_limit(u: &mut Unstructured, out: &mut String) -> arbitrary::Result<(
Ok(())
}
+/// Builds a pattern that is long *before* compilation by repeating one small
+/// unit.
+///
+/// The regex crate's `size_limit` bounds only the compiled NFA. Class-heavy
+/// patterns spend their memory earlier, in `regex-syntax`'s HIR translation,
+/// which that limit never sees, so cost scales with the pattern's *source*
+/// length. Measured worst case is ~7.4 KB of peak RSS per pattern byte
+/// (`\p{L}` repeated, under the `i` flag, where case-folding the Unicode class
+/// is the multiplier), against ~0.35 KB per byte for a plain literal. That
+/// makes `MAX_REGEX_SIZE_BEFORE_COMPILATION` the only guard on the vector, so
+/// straddle it: draw a length on both sides of 1 MiB, exercising both the
+/// `PatternTooLarge` rejection and the largest pattern that gets through.
+///
+/// The length is synthesized from a handful of input bytes rather than read out
+/// of the input, since libFuzzer's default `-max_len` of 4096 puts a pattern
+/// this long out of reach of the raw arm.
+fn gen_long_pattern(u: &mut Unstructured, out: &mut String) -> arbitrary::Result<()> {
+ let unit = *u.choose(&[
+ "\\p{L}",
+ "\\p{Greek}",
+ "\\w",
+ "\\d",
+ "\\s",
+ "[a-c]",
+ "a",
+ "(?:a)",
+ ])?;
+ let bytes = u.int_in_range(4096usize..=1_100_000)?;
+ out.reserve(bytes + unit.len());
+ // Round *up*, so a multi-byte unit can still land above the guard.
+ for _ in 0..bytes.div_ceil(unit.len()) {
+ out.push_str(unit);
+ }
+ Ok(())
+}
+
/// A replacement string mixing several capture-reference forms so the regex
/// crate's interpolation runs against whatever captures the match produced:
/// numbered (`$1`), named-braced (`${g0}`), the whole match (`$0`), a literal
-/// `$$`, and an out-of-range index (`$99`, which interpolates to empty).
-const REPLACEMENT: &str = "x$1-${g0}-$0-$$-$99y";
+/// `$$`, and an out-of-range index (`${99}`, which interpolates to empty).
+///
+/// NOTE: the braces on `${99}` are load-bearing. An unbraced `$name` takes the
+/// longest run of `[0-9A-Za-z_]`, so `$99y` would parse as the capture *name*
+/// `99y` and take the name-lookup path instead of the index one, silently
+/// swallowing the trailing literal.
+const REPLACEMENT: &str = "x$1-${g0}-$0-$$-${99}y";
fn drive(pattern: &str, flags: &str, text: &str) {
- let pattern = cap(pattern, 4096);
// The all-matches ops below (`replace_all`, `split`) drive `find_iter`,
// which re-scans from each match and is superlinear in the text length for
// adversarial patterns. Coverage instrumentation amplifies that by orders of
// magnitude, so keep the match text very short to stay within libFuzzer's
- // per-unit timeout. Pattern length compiles fast and is not the amplifier,
- // so it stays generous.
+ // per-unit timeout.
+ //
+ // The pattern is deliberately *not* capped. Its length is what drives
+ // compile-time memory, so capping it here would hide the very blowup this
+ // target is meant to reach, and `MAX_REGEX_SIZE_BEFORE_COMPILATION` is the
+ // guard under test. See `gen_long_pattern`.
let text = cap(text, 16);
let Ok(regex) = func::build_regex(pattern, flags) else {
return;
@@ -198,13 +253,17 @@ fn run(mut u: Unstructured) -> arbitrary::Result<()> {
return Ok(());
}
let mut pattern = String::new();
- // Occasionally emit a near-size-limit nested-quantifier pattern (the
- // compiler must reject it cleanly). Otherwise the structured generator.
- if u.int_in_range(0u8..=7)? == 0 {
- gen_near_limit(&mut u, &mut pattern)?;
- } else {
- let mut name_id = 0u32;
- gen_regex(&mut u, 3, &mut name_id, &mut pattern)?;
+ // Occasionally emit an adversarially sized pattern, aimed at one of the two
+ // size limits, which the compiler must reject cleanly. The long-pattern arm
+ // legitimately costs seconds and gigabytes per unit, so it stays rare.
+ // Otherwise the structured generator.
+ match u.int_in_range(0u8..=15)? {
+ 0 | 1 => gen_near_limit(&mut u, &mut pattern)?,
+ 2 => gen_long_pattern(&mut u, &mut pattern)?,
+ _ => {
+ let mut name_id = 0u32;
+ gen_regex(&mut u, 3, &mut name_id, &mut pattern)?;
+ }
}
let mut text = String::new();
for _ in 0..u.int_in_range(0usize..=24)? {
diff --git a/src/expr/fuzz/fuzz_targets/cast_string.rs b/src/expr/fuzz/fuzz_targets/cast_string.rs
index c91eb7b8810c8..810a782bb5d4c 100644
--- a/src/expr/fuzz/fuzz_targets/cast_string.rs
+++ b/src/expr/fuzz/fuzz_targets/cast_string.rs
@@ -30,7 +30,7 @@
use libfuzzer_sys::arbitrary::{self, Unstructured};
use libfuzzer_sys::fuzz_target;
-use mz_expr::{func, Eval, MirScalarExpr, UnaryFunc};
+use mz_expr::{Eval, MirScalarExpr, UnaryFunc, func};
use mz_repr::{Datum, ReprScalarType, RowArena};
const HEX: &[u8] = b"0123456789abcdefABCDEF";
diff --git a/src/expr/fuzz/fuzz_targets/eval_error_proto_roundtrip.rs b/src/expr/fuzz/fuzz_targets/eval_error_proto_roundtrip.rs
index e3ad1d4634199..68bfb7cca11f4 100644
--- a/src/expr/fuzz/fuzz_targets/eval_error_proto_roundtrip.rs
+++ b/src/expr/fuzz/fuzz_targets/eval_error_proto_roundtrip.rs
@@ -8,24 +8,30 @@
// by the Apache License, Version 2.0.
//! Fuzz target: `EvalError` must survive a proto re-encode + re-decode with the
-//! same value, exercising lossy proto conversions where the wire form decodes
-//! into a value that doesn't round-trip back to itself.
+//! same value, and the decoded value must then be renderable. This exercises
+//! lossy proto conversions where the wire form decodes into a value that doesn't
+//! round-trip back to itself, and decoded states that no Rust constructor
+//! produces and that consumers therefore mishandle.
//!
//! Two input modes share the byte stream (the first byte selects the mode):
//!
//! * Mode A (Arbitrary): drive proptest's `Arbitrary` impl for `EvalError`
//! from the libFuzzer byte stream to synthesize a *valid, deeply nested*
-//! `EvalError` (one of 80+ variants, several carrying `char`, `usize`,
-//! nested `NumericMaxScale`/`InvalidArrayError`/`DomainLimit` invariants).
-//! We then assert `from_proto(into_proto(v)) == v`. Random bytes decoded as
-//! a proto almost always yield near-empty/default messages, so this arm is
-//! what actually reaches the interesting variants and their narrowing /
-//! validation logic (`char::from_proto`, `usize`/`u32` casts, etc.).
+//! `EvalError` (one of 80+ variants, several carrying `char`, `usize`, or a
+//! nested `InvalidArrayError` / `ParseError` / `ParseHexError` /
+//! `InvalidRangeError` / `DomainLimit`). We then assert
+//! `from_proto(into_proto(v)) == v`. Random bytes decoded as a proto almost
+//! always yield near-empty/default messages, so this arm is what reaches the
+//! deeper variants at all, along with the `usize <-> u64` / `char <-> u32`
+//! casts on their payloads.
//!
//! * Mode B (raw bytes): decode the remaining bytes directly as
//! `ProtoEvalError` and, if it converts to Rust, assert it round-trips.
-//! Kept for robustness against hand-crafted / malformed wire forms that the
-//! structured generator would never produce.
+//! Mode A always starts from an already-valid Rust value, so it structurally
+//! cannot reach the *rejection* branches of `char::from_proto` /
+//! `usize::from_proto`, nor any state that no constructor builds. Only this
+//! arm can, which is why it stays even though most inputs decode to
+//! something trivial.
#![no_main]
@@ -36,16 +42,25 @@ use proptest::strategy::{Strategy, ValueTree};
use proptest::test_runner::{Config, RngAlgorithm, TestRng, TestRunner};
use prost::Message;
-/// Assert that a `EvalError` survives encode -> decode -> into_rust unchanged.
+/// Assert that a `EvalError` survives encode -> decode -> into_rust unchanged,
+/// and that the decoded value is renderable.
fn assert_roundtrip(orig: &EvalError) {
let proto = >::from_rust(orig);
let bytes = proto.encode_to_vec();
- let decoded = ProtoEvalError::decode(bytes.as_slice())
- .expect("re-encode of valid EvalError must decode");
+ let decoded =
+ ProtoEvalError::decode(bytes.as_slice()).expect("re-encode of valid EvalError must decode");
let round: EvalError = decoded
.into_rust()
.expect("re-encoded EvalError must convert back to Rust");
assert_eq!(orig, &round, "EvalError changed across proto roundtrip");
+
+ // Round-trip identity alone would certify a decoded value that aborts the
+ // process the moment anything looks at it. `DataflowErrorSer::Display`
+ // decodes untrusted bytes and Displays them on the index peek path, so a
+ // panicking `Display`/`detail`/`hint` arm is a dataflow crash loop.
+ let _ = round.to_string();
+ let _ = round.detail();
+ let _ = round.hint();
}
/// Decode `data` directly as a `ProtoEvalError`. If it converts to a Rust
@@ -78,11 +93,13 @@ fuzz_target!(|data: &[u8]| {
}
let rng = TestRng::from_seed(RngAlgorithm::ChaCha, &seed);
let mut runner = TestRunner::new_with_rng(Config::default(), rng);
- let value = ::arbitrary()
- .new_tree(&mut runner)
- .expect("valuetree")
- .current();
- assert_roundtrip(&value);
+ let Ok(tree) =
+ ::arbitrary().new_tree(&mut runner)
+ else {
+ // A generator rejection is a harness limit, not a product bug.
+ return;
+ };
+ assert_roundtrip(&tree.current());
} else {
// Mode B: raw wire bytes.
raw_bytes_arm(rest);
diff --git a/src/expr/fuzz/fuzz_targets/jsonb_get.rs b/src/expr/fuzz/fuzz_targets/jsonb_get.rs
index cebff0629b2cb..c354d6b395d68 100644
--- a/src/expr/fuzz/fuzz_targets/jsonb_get.rs
+++ b/src/expr/fuzz/fuzz_targets/jsonb_get.rs
@@ -23,6 +23,13 @@
//! generate the access key/index from that same set, so the accessors hit real
//! fields/elements (the success + traversal paths) as well as missing ones, and
//! the index includes out-of-range and extreme values for the array-bounds path.
+//!
+//! Two caps on the generated document are deliberate rather than oversights.
+//! String values draw from a fragment set that covers escapes and non-ASCII but
+//! omits the NUL escape (SQL-475), and nesting stops at depth 5 (SQL-515). Both
+//! of the excluded shapes hit bugs that are tracked already and that this target
+//! has no oracle for anyway, and a target that reproduces a known crash on every
+//! run buries whatever else it would have found.
#![no_main]
@@ -30,13 +37,33 @@ use std::str::FromStr;
use libfuzzer_sys::arbitrary::{self, Unstructured};
use libfuzzer_sys::fuzz_target;
-use mz_expr::{func, Eval, MirScalarExpr};
+use mz_expr::{Eval, MirScalarExpr, func};
use mz_repr::adt::jsonb::Jsonb;
use mz_repr::{Datum, ReprScalarType, RowArena};
/// Object keys, kept to a small set so generated access keys hit real fields.
const KEYS: &[&str] = &["a", "b", "c", "x"];
+/// Fragments of a JSON string value, spliced raw between the quotes, so escape
+/// sequences appear here already escaped. Past plain ASCII these cover what
+/// `->>` has to re-escape when `jsonb_stringify` renders the accessed element
+/// back to text: quote, backslash, a control character, and non-ASCII in both
+/// `\u` and raw UTF-8 form, including a surrogate pair.
+const STRING_PARTS: &[&str] = &[
+ "a",
+ "z",
+ "0",
+ " ",
+ "\\\"",
+ "\\\\",
+ "\\n",
+ "\\u0001",
+ "\\u00e9",
+ "é",
+ "\\ud83d\\ude00",
+ "😀",
+];
+
fn gen_json(u: &mut Unstructured, depth: u32, out: &mut String) -> arbitrary::Result<()> {
let leaf = depth == 0 || u.is_empty();
match if leaf {
@@ -54,7 +81,7 @@ fn gen_json(u: &mut Unstructured, depth: u32, out: &mut String) -> arbitrary::Re
3 => {
out.push('"');
for _ in 0..u.int_in_range(0usize..=4)? {
- out.push(*u.choose(&['a', 'z', '0', ' '])?);
+ out.push_str(u.choose(STRING_PARTS)?);
}
out.push('"');
}
@@ -87,15 +114,31 @@ fn gen_json(u: &mut Unstructured, depth: u32, out: &mut String) -> arbitrary::Re
Ok(())
}
-fn run(mut u: Unstructured) -> arbitrary::Result<()> {
- let mut json = String::new();
- gen_json(&mut u, 5, &mut json)?;
- let Ok(jsonb) = Jsonb::from_str(&json) else {
- return Ok(());
- };
- let value = jsonb.as_ref().into_datum();
- let arena = RowArena::new();
+/// Evaluates `expr`, requiring that it neither panic nor error.
+///
+/// The four accessors under test are infallible, they return `Option`, not
+/// `Result`, and both operands are `Literal(Ok(..))`, which cannot error. So the
+/// only `Err` reachable here is the `EvalError::Internal` that the eager binary
+/// dispatch raises when a literal's `Datum`/`ReprScalarType` does not match the
+/// function's declared input type. Nothing checks that match at compile time, so
+/// an `Err` means this harness built an invalid expression and every eval below
+/// short-circuits before reaching the code under test, leaving a target that
+/// exercises nothing yet still reports clean.
+fn eval_ok(expr: MirScalarExpr, arena: &RowArena) {
+ let res = expr.eval(&[], arena);
+ assert!(res.is_ok(), "harness built an invalid expression: {res:?}");
+}
+fn run(mut u: Unstructured) -> arbitrary::Result<()> {
+ // The access key and index are drawn before the document, and the order
+ // matters. `gen_json` scales its appetite with the input it is handed and
+ // drains the buffer on anything up to libFuzzer's default `-max_len=4096`,
+ // and `Unstructured` does not error once exhausted, it returns the low end of
+ // every range. Drawing these afterwards therefore pins them to `""` and `0`
+ // for the vast majority of executions, which unfuzzes half the input space.
+ // `gen_json` degrades gracefully on the remainder, it emits `null` when
+ // starved.
+ //
// Key: usually a real field name (hit), sometimes a miss / arbitrary string.
let key_buf;
let key: &str = if u.int_in_range(0u8..=2)? == 0 {
@@ -116,27 +159,39 @@ fn run(mut u: Unstructured) -> arbitrary::Result<()> {
i64::from(u.int_in_range(-3i32..=8)?)
};
+ let mut json = String::new();
+ gen_json(&mut u, 5, &mut json)?;
+ let Ok(jsonb) = Jsonb::from_str(&json) else {
+ return Ok(());
+ };
+ let value = jsonb.as_ref().into_datum();
+ let arena = RowArena::new();
+
let key_expr = || MirScalarExpr::literal_ok(Datum::String(key), ReprScalarType::String);
let index_expr = || MirScalarExpr::literal_ok(Datum::Int64(index), ReprScalarType::Int64);
let jsonb_expr = || MirScalarExpr::literal_ok(value, ReprScalarType::Jsonb);
// jsonb -> '' (object field access, returns jsonb)
- let _ = jsonb_expr()
- .call_binary(key_expr(), func::JsonbGetString)
- .eval(&[], &arena);
+ eval_ok(
+ jsonb_expr().call_binary(key_expr(), func::JsonbGetString),
+ &arena,
+ );
// jsonb ->> '' (object field access, returns text via jsonb_stringify)
- let _ = jsonb_expr()
- .call_binary(key_expr(), func::JsonbGetStringStringify)
- .eval(&[], &arena);
+ eval_ok(
+ jsonb_expr().call_binary(key_expr(), func::JsonbGetStringStringify),
+ &arena,
+ );
// jsonb -> (array element access, returns jsonb)
- let _ = jsonb_expr()
- .call_binary(index_expr(), func::JsonbGetInt64)
- .eval(&[], &arena);
+ eval_ok(
+ jsonb_expr().call_binary(index_expr(), func::JsonbGetInt64),
+ &arena,
+ );
// jsonb ->> (array element access, returns text via jsonb_stringify)
- let _ = jsonb_expr()
- .call_binary(index_expr(), func::JsonbGetInt64Stringify)
- .eval(&[], &arena);
+ eval_ok(
+ jsonb_expr().call_binary(index_expr(), func::JsonbGetInt64Stringify),
+ &arena,
+ );
Ok(())
}
diff --git a/src/expr/fuzz/fuzz_targets/jsonb_path.rs b/src/expr/fuzz/fuzz_targets/jsonb_path.rs
index 9750217628710..76c95f6180fbf 100644
--- a/src/expr/fuzz/fuzz_targets/jsonb_path.rs
+++ b/src/expr/fuzz/fuzz_targets/jsonb_path.rs
@@ -38,7 +38,7 @@ use std::str::FromStr;
use libfuzzer_sys::arbitrary::{self, Unstructured};
use libfuzzer_sys::fuzz_target;
-use mz_expr::{func, Eval, MirScalarExpr};
+use mz_expr::{Eval, MirScalarExpr, func};
use mz_repr::adt::array::{ArrayDimension, InvalidArrayError};
use mz_repr::adt::jsonb::Jsonb;
use mz_repr::{Datum, ReprScalarType, RowArena};
@@ -119,14 +119,15 @@ fn gen_json(u: &mut Unstructured, depth: u32, out: &mut String) -> arbitrary::Re
}
fn run(mut u: Unstructured) -> arbitrary::Result<()> {
- let mut json = String::new();
- gen_json(&mut u, 5, &mut json)?;
- let Ok(jsonb) = Jsonb::from_str(&json) else {
- return Ok(());
- };
- let value = jsonb.as_ref().into_datum();
- let arena = RowArena::new();
-
+ // The path is drawn before the document, and the order matters. `gen_json`
+ // scales its appetite with the input it is handed and drains the buffer on
+ // anything up to libFuzzer's default `-max_len=4096`, and `Unstructured` does
+ // not error once exhausted, it returns the low end of every range. Drawing
+ // the path afterwards therefore pins `n` to 0 for the vast majority of
+ // executions, and `jsonb #> '{}'` returns the document unchanged, so the walk
+ // this target exists for would never run. `gen_json` degrades gracefully on
+ // the remainder, it emits `null` when starved.
+ //
// Path components: object keys (hits), array indices ("0".."3"), and an
// occasional miss, so the walk descends real structure and also dead-ends.
let n = u.int_in_range(0usize..=5)?;
@@ -139,6 +140,14 @@ fn run(mut u: Unstructured) -> arbitrary::Result<()> {
});
}
+ let mut json = String::new();
+ gen_json(&mut u, 5, &mut json)?;
+ let Ok(jsonb) = Jsonb::from_str(&json) else {
+ return Ok(());
+ };
+ let value = jsonb.as_ref().into_datum();
+ let arena = RowArena::new();
+
let dims = if path.is_empty() {
Vec::new()
} else {
diff --git a/src/expr/fuzz/fuzz_targets/like_pattern_compile.rs b/src/expr/fuzz/fuzz_targets/like_pattern_compile.rs
index 119c00fe5e15f..7cfe239eb5ce5 100644
--- a/src/expr/fuzz/fuzz_targets/like_pattern_compile.rs
+++ b/src/expr/fuzz/fuzz_targets/like_pattern_compile.rs
@@ -14,18 +14,29 @@
//!
//! 1. Compiling an arbitrary pattern and matching arbitrary text never panics.
//! 2. A pattern built by escaping every wildcard/escape character in `text` is
-//! a pure literal, and `LIKE` is anchored, so it must match exactly `text`.
+//! a pure literal, and `LIKE` is anchored, so it must match exactly `text`:
+//! `text` matches, and `text` with a character appended does not.
//!
//! A pattern/text pair of random Unicode rarely contains the wildcard/escape
//! metacharacters or the literal overlap that drives the interesting code, so it
//! plateaus on shallow shapes. Instead we mostly draw both the pattern and the
//! match text from a tiny shared alphabet of literals plus the LIKE
-//! metacharacters `%`, `_`, and `\`. Generating *multiple* `%` is deliberate: at
-//! two-or-more `%` (`many_subpatterns > 1`) `compile` abandons the backtracking
-//! string matcher and routes to the linear-time regex engine, a boundary the
-//! string-only inputs never reach. Drawing the text from the same alphabet means
-//! it actually matches the wildcards, exercising the `%`-suffix-search /
-//! backtracking and the regex path on real (not always-empty) matches.
+//! metacharacters `%`, `_`, and `\`. Generating *several* `%` interleaved with
+//! literals is deliberate, because that is what crosses the routing boundary
+//! between the two matcher implementations: `compile` abandons the backtracking
+//! string matcher for the linear-time regex engine once two or more `%`
+//! subpatterns carry a *non-empty* suffix (so `%a%b`, but not `%%%%`, which
+//! collapses into one subpattern, and not `%a%`, whose trailing `%` has an empty
+//! suffix), or once the subpattern count exceeds `MAX_SUBPATTERNS`, which the
+//! `%`-heavy weighting reaches most often in practice.
+//!
+//! Drawing the text from the same alphabet means it actually matches the
+//! wildcards, exercising the `%`-suffix-search / backtracking and the regex path
+//! on real (not always-empty) matches. The alphabet carries multi-byte
+//! characters so byte offsets and character counts diverge: the `%` backtracking
+//! in `is_match_subpatterns` mixes character counting with byte indexing and
+//! walks back over char boundaries by byte, and with a single-byte alphabet that
+//! walk degenerates to one iteration and can never expose an off-by-one.
#![no_main]
@@ -34,11 +45,14 @@ use libfuzzer_sys::fuzz_target;
use mz_expr::like_pattern;
/// Literals shared between pattern and text, so wildcards match real characters.
-const LITERALS: &[char] = &['a', 'b', 'c'];
+/// `é` (2 bytes) and `漢` (3 bytes) make byte offsets and character counts
+/// diverge, so the char-boundary walk in `is_match_subpatterns` runs for more
+/// than one iteration.
+const LITERALS: &[char] = &['a', 'b', 'c', 'é', '漢'];
/// Builds a LIKE pattern over the literal alphabet plus the `%`/`_`/`\`
-/// metacharacters, weighted toward emitting several `%` so the >1-`%` regex
-/// routing boundary is crossed.
+/// metacharacters, weighted toward emitting several `%` so patterns land on both
+/// sides of the string-matcher/regex routing boundary.
fn gen_pattern(u: &mut Unstructured) -> arbitrary::Result {
let mut p = String::new();
let n = u.int_in_range(0usize..=16)?;
@@ -53,7 +67,7 @@ fn gen_pattern(u: &mut Unstructured) -> arbitrary::Result {
// reachable when this is the last iteration.
p.push('\\');
if u.int_in_range(0u8..=3)? != 0 {
- p.push(*u.choose(&['%', '_', '\\', 'a'])?);
+ p.push(*u.choose(&['%', '_', '\\', 'a', 'é'])?);
}
}
_ => p.push(*u.choose(LITERALS)?),
@@ -96,11 +110,31 @@ fn run(mut u: Unstructured) -> arbitrary::Result<()> {
}
literal.push(c);
}
- if let Ok(matcher) = like_pattern::compile(&literal, case_insensitive) {
- assert!(
- matcher.is_match(&text),
- "literal LIKE pattern {literal:?} must match its source text {text:?}"
- );
+ match like_pattern::compile(&literal, case_insensitive) {
+ Ok(matcher) => {
+ assert!(
+ matcher.is_match(&text),
+ "literal LIKE pattern {literal:?} must match its source text {text:?}"
+ );
+ // And nothing else: the pattern is anchored at both ends and every
+ // one of its characters consumes exactly one text character, under
+ // `ILIKE` too, since simple case folding maps a character to other
+ // single characters. So appending anything must break the match. This
+ // is the direction that catches over-matching regressions, such as a
+ // lost anchor in the regex or an escaped metacharacter that falls
+ // back to wildcard semantics.
+ let longer = format!("{text}a");
+ assert!(
+ !matcher.is_match(&longer),
+ "literal LIKE pattern {literal:?} must not match {longer:?}"
+ );
+ }
+ // Every escape sequence in `literal` is terminated by construction, so
+ // the documented pattern-length limit is the only legitimate rejection.
+ Err(e) => assert!(
+ literal.len() > 8 << 10,
+ "valid literal pattern {literal:?} failed to compile: {e}"
+ ),
}
Ok(())
}
diff --git a/src/expr/fuzz/fuzz_targets/like_pattern_escape.rs b/src/expr/fuzz/fuzz_targets/like_pattern_escape.rs
index 81ffb17964753..00b87491e13db 100644
--- a/src/expr/fuzz/fuzz_targets/like_pattern_escape.rs
+++ b/src/expr/fuzz/fuzz_targets/like_pattern_escape.rs
@@ -10,10 +10,27 @@
//! Fuzz target: `like_pattern::normalize_pattern` rewrites a `LIKE ... ESCAPE c`
//! pattern (custom escape character) into the default-escape form before
//! compilation. The user controls both the pattern and the escape character, so
-//! the custom-escape rewrite, a char-by-char parser with escape state, must
-//! not panic on any input, and its output must still compile and match. This is
-//! the `ESCAPE`-clause path that `like_pattern_compile` (default escape only)
-//! never exercises.
+//! the custom-escape rewrite, a char-by-char parser with escape state, must not
+//! panic on any input. This is the `ESCAPE`-clause path that
+//! `like_pattern_compile` (default escape only) never exercises.
+//!
+//! `normalize_pattern` has no slicing, no byte indexing and no fallible
+//! arithmetic, so a panic-only harness over it can never fail. The bug class it
+//! can actually carry is a miscompilation in the escape state machine, a
+//! dropped, doubled or mis-paired escape. Two oracles pin that down:
+//!
+//! 1. Every output of `normalize_pattern` emits the default escape `\` only as
+//! part of a completed two-character escape, so it is well-formed
+//! default-escape form and must compile. The size limit is the only
+//! legitimate rejection. See the exception for the identity passthrough at
+//! the check itself.
+//! 2. Escaping every metacharacter *and* the escape character itself with the
+//! custom escape character yields a pure literal pattern. `LIKE` is
+//! anchored, so after normalization it must match exactly its own source
+//! text: `text` matches, and `text` with a character appended does not. A
+//! lost `\` turns an escaped `%`/`_` back into a wildcard and the appended
+//! direction fires. A spurious or mis-paired `\` breaks the match and the
+//! first direction fires.
//!
//! With a fully arbitrary escape char and a fully arbitrary pattern, the escape
//! char almost never coincides with a character actually in the pattern, so the
@@ -29,45 +46,116 @@
use libfuzzer_sys::arbitrary::{self, Unstructured};
use libfuzzer_sys::fuzz_target;
+use mz_expr::EvalError;
use mz_expr::like_pattern::{self, EscapeBehavior};
-/// Shared alphabet for the pattern and the escape char, so the escape char
+/// Shared alphabet for the pattern and the match text, so the escape char
/// frequently matches characters in the pattern and the consume/unterminated
/// branches fire. Includes the LIKE metacharacters and a few literals.
const ALPHA: &[char] = &['%', '_', '\\', 'a', 'b', 'c'];
-fn gen_pattern(u: &mut Unstructured) -> arbitrary::Result {
- let mut p = String::new();
+/// Alphabet for the escape char: `ALPHA` minus the default escape `\`.
+/// `EscapeBehavior::Char('\\')` is an identity passthrough that never enters the
+/// custom-escape state machine, so drawing it here would spend executions on a
+/// two-line branch. `\` stays in `ALPHA` because a `\` in the *pattern* is what
+/// drives the doubling branch of the rewrite.
+const ESCAPE_ALPHA: &[char] = &['%', '_', 'a', 'b', 'c'];
+
+fn gen_over_alpha(u: &mut Unstructured) -> arbitrary::Result {
+ let mut s = String::new();
for _ in 0..u.int_in_range(0usize..=20)? {
- p.push(*u.choose(ALPHA)?);
+ s.push(*u.choose(ALPHA)?);
}
- Ok(p)
+ Ok(s)
}
fn run(mut u: Unstructured) -> arbitrary::Result<()> {
let case_insensitive = u.arbitrary()?;
- let (pattern, escape_char) = if u.int_in_range(0u8..=3)? == 0 {
- // Some fully-arbitrary inputs keep the raw-Unicode reject coverage.
- (u.arbitrary::()?, u.arbitrary::()?)
+ let (pattern, escape_char, text) = if u.int_in_range(0u8..=3)? == 0 {
+ // Some fully-arbitrary inputs keep the raw-Unicode reject coverage, and
+ // give `is_match` text long enough to expose the super-linear
+ // backtracking in the string matcher.
+ (
+ u.arbitrary::()?,
+ u.arbitrary::()?,
+ u.arbitrary::()?,
+ )
} else {
- (gen_pattern(&mut u)?, *u.choose(ALPHA)?)
+ // Text from the same alphabet so the compiled matcher can match.
+ (
+ gen_over_alpha(&mut u)?,
+ *u.choose(ESCAPE_ALPHA)?,
+ gen_over_alpha(&mut u)?,
+ )
};
- // Text from the same literal-ish alphabet so the compiled matcher can match.
- let mut text = String::new();
- for _ in 0..u.int_in_range(0usize..=20)? {
- text.push(*u.choose(ALPHA)?);
- }
for behavior in [EscapeBehavior::Char(escape_char), EscapeBehavior::Disabled] {
let Ok(normalized) = like_pattern::normalize_pattern(&pattern, behavior) else {
continue;
};
- // The rewritten pattern is in default-escape form and must compile and
- // match arbitrary text without panicking.
- if let Ok(matcher) = like_pattern::compile(&normalized, case_insensitive) {
- let _ = matcher.is_match(&text);
+ // `normalize_pattern` emits `\` only as part of a completed escape pair,
+ // so its output is well-formed default-escape form and the documented
+ // size limit is the only legitimate rejection. Anything else, notably
+ // the `EvalError::Internal` that `build_regex` raises on a regex it
+ // failed to build, means the rewrite produced a pattern the compiler
+ // cannot read. `EscapeBehavior::Char('\\')` is an identity passthrough,
+ // so there a lone trailing `\` from the input pattern reaches `compile`
+ // unchanged and rejecting it is correct.
+ let passthrough = matches!(behavior, EscapeBehavior::Char('\\'));
+ match like_pattern::compile(&normalized, case_insensitive) {
+ Ok(matcher) => {
+ let _ = matcher.is_match(&text);
+ }
+ Err(EvalError::LikePatternTooLong) => {}
+ Err(EvalError::UnterminatedLikeEscapeSequence) if passthrough => {}
+ Err(e) => panic!(
+ "normalize_pattern({pattern:?}, {behavior:?}) = {normalized:?} \
+ failed to compile: {e:?}"
+ ),
+ }
+ }
+
+ // Escaping every metacharacter in `text` with `escape_char`, plus
+ // `escape_char` itself, yields a pattern that normalizes to a pure literal.
+ // The construction holds for every `escape_char`, including when it is
+ // itself `%`, `_` or `\`: `escape_char` is only ever emitted as the
+ // immediate prefix of the character it escapes, so `normalize_pattern`'s
+ // left-to-right pairing lines up exactly with it and no unescaped wildcard
+ // survives. It also can never end in a dangling escape, which is why
+ // normalization here must succeed.
+ let mut literal = String::with_capacity(2 * text.len());
+ for c in text.chars() {
+ if matches!(c, '%' | '_' | '\\') || c == escape_char {
+ literal.push(escape_char);
+ }
+ literal.push(c);
+ }
+ let normalized = like_pattern::normalize_pattern(&literal, EscapeBehavior::Char(escape_char))
+ .expect("every escape in `literal` is terminated by construction");
+ match like_pattern::compile(&normalized, case_insensitive) {
+ Ok(matcher) => {
+ assert!(
+ matcher.is_match(&text),
+ "literal pattern {literal:?} (escape {escape_char:?}) normalized to \
+ {normalized:?} must match its source text {text:?}"
+ );
+ // And nothing else: the pattern is anchored at both ends and every
+ // one of its characters consumes exactly one text character, under
+ // `ILIKE` too, since simple case folding maps a character to other
+ // single characters. So appending anything must break the match.
+ let longer = format!("{text}a");
+ assert!(
+ !matcher.is_match(&longer),
+ "literal pattern {literal:?} (escape {escape_char:?}) normalized to \
+ {normalized:?} must not match {longer:?}"
+ );
}
+ Err(EvalError::LikePatternTooLong) => {}
+ Err(e) => panic!(
+ "literal pattern {literal:?} (escape {escape_char:?}) normalized to \
+ {normalized:?} failed to compile: {e:?}"
+ ),
}
Ok(())
}
diff --git a/src/expr/fuzz/fuzz_targets/mfp_optimize.rs b/src/expr/fuzz/fuzz_targets/mfp_optimize.rs
index 1294d8397671e..e0c65a6e37b3f 100644
--- a/src/expr/fuzz/fuzz_targets/mfp_optimize.rs
+++ b/src/expr/fuzz/fuzz_targets/mfp_optimize.rs
@@ -33,13 +33,18 @@
//! twice-optimized plan, which can repair a first-pass miscompile that
//! production `into_plan`-on-raw callers still execute. `optimize` only
//! iterates while the expression size strictly decreases, so a fresh second
-//! call is not guaranteed to be a no-op. The oracle is one-directional,
-//! mirroring the contract optimize actually owes: optimize is allowed to
-//! *drop* an error or a row that the raw MFP would reject, because it removes
-//! unused map expressions and reorders predicates. But for every input row the
-//! raw MFP passes through cleanly with output `out`, the optimized plan must
-//! also pass it through with the byte-identical `out`. (When the raw MFP errors
-//! or filters a row we assert nothing.)
+//! call is not guaranteed to be a no-op. The oracle constrains both
+//! directions of the filter, and neither direction of an error: optimize is
+//! allowed to *drop* an error, because `remove_undemanded` deletes unused
+//! (possibly fallible) map expressions, and it is allowed to *introduce* one,
+//! because sorting predicates by position can surface an error the raw order
+//! rejected the row before reaching. So when either side errors we assert
+//! nothing. A row whose lazy evaluation absorbs an `AND`/`OR` operand error
+//! is skipped outright, because `optimize` may legally surface that error or
+//! fold on it (see `absorbs_and_or_operand_error`). Otherwise: a row the raw
+//! MFP passes cleanly with output `out` the optimized plan must pass with
+//! the byte-identical `out`, and a row the raw MFP cleanly filters the
+//! optimized plan must not pass.
//!
//! * **Temporal lowering.** Add predicates of the form `mz_now() e` (and
//! conjunctions of them) over a bounded `mz_timestamp` expression `e`, then
@@ -51,18 +56,23 @@
//! at `T`" (`lower <= T < upper`, with the non-temporal predicates also
//! passing) agrees with a substitution reference: the same MFP with every
//! `mz_now()` replaced by the literal `mz_timestamp` `T`, evaluated
-//! non-temporally. The compared timestamps are kept well below `u64::MAX` so
-//! `StepMzTimestamp` (the `+1` the lowering inserts for `<=`/`>`/`=`) never
-//! overflows, which keeps the substitution equivalence exact. As above the
-//! oracle is one-directional: if the reference errors at `T` we assert
-//! nothing.
+//! non-temporally. The probed times are a random sample plus the interval's
+//! own endpoints, because an off-by-one in the operator translation is
+//! observable at an endpoint and nowhere else. The compared timestamps are
+//! kept well below `u64::MAX` so `StepMzTimestamp` (the `+1` the lowering
+//! inserts for `<=`/`>`/`=`) never overflows, which keeps the substitution
+//! equivalence exact. As above the oracle is one-directional: if the reference
+//! errors at `T` we assert nothing.
#![no_main]
+use std::collections::{BTreeMap, BTreeSet};
+
use libfuzzer_sys::arbitrary::{self, Arbitrary, Unstructured};
use libfuzzer_sys::fuzz_target;
use mz_expr::{
- func, Eval, EvalError, MapFilterProject, MirScalarExpr, SafeMfpPlan, UnmaterializableFunc,
+ Eval, EvalError, MapFilterProject, MirScalarExpr, SafeMfpPlan, UnmaterializableFunc,
+ VariadicFunc, func,
};
use mz_repr::{Datum, Diff, ReprScalarType, Row, RowArena, Timestamp};
@@ -275,9 +285,13 @@ fn gen_ts_expr(u: &mut Unstructured, cols: &[Ty]) -> arbitrary::Result MirScalarExpr {
MirScalarExpr::literal_ok(
@@ -335,13 +349,14 @@ fn reference_present(plan: &mz_expr::SafeMfpPlan, row: &Row) -> Option {
}
}
+type Interval = Option<(Timestamp, Option)>;
+
/// Read the validity interval a temporal plan assigns to `row`, evaluating once
/// with `time = 0`, `diff = +1`, and an always-valid frontier. Returns:
/// * `Err(())` if evaluation errored,
/// * `Ok(None)` if the (non-temporal part of the) plan rejected the row,
/// * `Ok(Some((lower, upper)))` for the half-open interval `[lower, upper)`
/// (`upper == None` means unbounded above).
-type Interval = Option<(Timestamp, Option)>;
fn temporal_interval(plan: &mz_expr::MfpPlan, row: &Row) -> Result {
let arena = RowArena::new();
let mut datums: Vec = row.iter().collect();
@@ -419,6 +434,96 @@ fn eval_raw(mfp: &MapFilterProject, row: &Row) -> Option> {
Some(Some(out))
}
+/// Does evaluating `expr` on this row absorb an `And`/`Or` operand error?
+///
+/// This is `mir_scalar_reduce`'s `absorbs_and_or_operand_error`, the per-row
+/// refinement of `MirScalarExpr::could_hit_nonstrict_error_fold`, minus the
+/// `ErrorIfNull` arm for a function this target's vocabulary lacks. Rows it
+/// reports are skipped: reduce's error propagation may fold the absorbed error
+/// into the plan (CLU-137), and memoization may hoist a fallible subexpression
+/// shared under the `And`/`Or` into a mapped column that is evaluated eagerly,
+/// surfacing the error. Both outcomes are legal for `optimize`, because
+/// `AND`/`OR` evaluation order is undefined (STG-54), so such a row constrains
+/// nothing. See `run` for the two mechanisms side by side.
+///
+/// NOTE: this deliberately evaluates *every* operand of an `And`/`Or`, even
+/// though `And::eval` short-circuits on the first `false` and so may never
+/// reach a later erroring operand. Error propagation and memoization are both
+/// position-insensitive, so a short-circuit-aware version would report "no
+/// absorption" for rows the optimized plan nonetheless errors on.
+fn absorbs_and_or_operand_error<'a>(
+ expr: &'a MirScalarExpr,
+ datums: &[Datum<'a>],
+ arena: &'a RowArena,
+) -> bool {
+ match expr {
+ MirScalarExpr::Column(..)
+ | MirScalarExpr::Literal(..)
+ | MirScalarExpr::CallUnmaterializable(_) => false,
+ MirScalarExpr::CallUnary { expr, .. } => absorbs_and_or_operand_error(expr, datums, arena),
+ MirScalarExpr::CallBinary { expr1, expr2, .. } => {
+ absorbs_and_or_operand_error(expr1, datums, arena)
+ || absorbs_and_or_operand_error(expr2, datums, arena)
+ }
+ MirScalarExpr::CallVariadic { func, exprs } => {
+ let nested = exprs
+ .iter()
+ .any(|expr| absorbs_and_or_operand_error(expr, datums, arena));
+
+ if !matches!(func, VariadicFunc::And(_) | VariadicFunc::Or(_)) {
+ return nested;
+ }
+
+ let is_and = matches!(func, VariadicFunc::And(_));
+ let mut has_absorbing_value = false;
+ let mut has_error = false;
+ for expr in exprs {
+ match expr.eval(datums, arena) {
+ Ok(Datum::False) if is_and => has_absorbing_value = true,
+ Ok(Datum::True) if !is_and => has_absorbing_value = true,
+ Err(_) => has_error = true,
+ _ => {}
+ }
+ }
+
+ nested || (has_absorbing_value && has_error)
+ }
+ MirScalarExpr::If { cond, then, els } => {
+ if absorbs_and_or_operand_error(cond, datums, arena) {
+ return true;
+ }
+ match cond.eval(datums, arena) {
+ Ok(Datum::True) => absorbs_and_or_operand_error(then, datums, arena),
+ Ok(Datum::False | Datum::Null) => absorbs_and_or_operand_error(els, datums, arena),
+ _ => false,
+ }
+ }
+ }
+}
+
+/// `absorbs_and_or_operand_error` over every map and predicate of `mfp`, in the
+/// column context `eval_raw` evaluates them in.
+///
+/// A map expression that fails to re-evaluate counts as absorbing. That arm is
+/// reachable only for a row `eval_raw` filtered before reaching the map, and
+/// "assert nothing" is the safe answer for a row this function cannot model.
+fn mfp_absorbs_and_or_operand_error(mfp: &MapFilterProject, row: &Row) -> bool {
+ let arena = RowArena::new();
+ let mut datums: Vec = row.iter().collect();
+ for expr in &mfp.expressions {
+ if absorbs_and_or_operand_error(expr, &datums, &arena) {
+ return true;
+ }
+ match expr.eval(&datums, &arena) {
+ Ok(datum) => datums.push(datum),
+ Err(_) => return true,
+ }
+ }
+ mfp.predicates
+ .iter()
+ .any(|(_, predicate)| absorbs_and_or_operand_error(predicate, &datums, &arena))
+}
+
/// Non-temporal preservation: raw unoptimized MFP semantics vs. the `optimize`d
/// plan's output.
fn run_nontemporal(
@@ -440,24 +545,52 @@ fn run_nontemporal(
for _ in 0..ROWS_PER_MFP {
let row = gen_input_row(u, types)?;
- // Only a row the raw MFP passes through cleanly constrains the optimized
- // plan. An error or a filtered row lets optimize legitimately differ.
- let Some(Some(out_ref)) = eval_raw(&mfp, &row) else {
+ // A raw error constrains nothing: optimize may legitimately drop it.
+ let Some(raw) = eval_raw(&mfp, &row) else {
continue;
};
+ // Neither does a row that absorbs an `And`/`Or` operand error: optimize
+ // may legally turn its outcome into an error, a different value, or a
+ // different pass/fail. See `run`.
+ if mfp_absorbs_and_or_operand_error(&mfp, &row) {
+ continue;
+ }
+
let arena = RowArena::new();
let mut datums_p: Vec = row.iter().collect();
let mut buf_p = Row::default();
- match safe_opt.evaluate_into(&mut datums_p, &arena, &mut buf_p) {
- Ok(Some(out)) => assert_eq!(
+ // Every panic prints both MFPs: which rewrite `optimize` got wrong is not
+ // recoverable from the row alone, and the crash artifact only replays
+ // under a `cargo fuzz` build.
+ let opt_mfp: &MapFilterProject = &safe_opt;
+ let opt = safe_opt.evaluate_into(&mut datums_p, &arena, &mut buf_p);
+ match (raw, opt) {
+ (Some(out_ref), Ok(Some(out))) => assert_eq!(
&out_ref, out,
- "optimize changed the projected output\n row = {row:?}\n out_ref = {out_ref:?}\n out_opt = {out:?}"
+ "optimize changed the projected output\n row = {row:?}\n out_ref = {out_ref:?}\n out_opt = {out:?}\n mfp = {mfp:?}\n optimized = {opt_mfp:?}"
+ ),
+ (Some(_), Ok(None)) => panic!(
+ "optimize filtered out a row the raw MFP passed\n row = {row:?}\n mfp = {mfp:?}\n optimized = {opt_mfp:?}"
),
- Ok(None) => panic!("optimize filtered out a row the raw MFP passed\n row = {row:?}"),
- Err(e) => panic!(
- "optimize errored on a row the raw MFP passed cleanly\n row = {row:?}\n err = {e:?}"
+ (Some(_), Err(e)) => panic!(
+ "optimize errored on a row the raw MFP passed cleanly\n row = {row:?}\n err = {e:?}\n mfp = {mfp:?}\n optimized = {opt_mfp:?}"
),
+ // The raw MFP cleanly filtered the row, so the optimized plan must
+ // not pass it. `optimize` never drops a predicate beyond
+ // `predicates.dedup()` on exact copies, and no rewrite can turn a
+ // predicate's `false`/`null` into `true`. Reordering only changes
+ // which predicate is reached first. The one rewrite that does change
+ // a value, replacing a mapped expression `e` with `#c` when a
+ // predicate `#c = e` is present, is value-preserving on any row where
+ // that predicate holds, and on a row where it does not the predicate
+ // itself is still there to reject the row.
+ (None, Ok(Some(out))) => panic!(
+ "optimize passed a row the raw MFP cleanly filtered\n row = {row:?}\n out_opt = {out:?}\n mfp = {mfp:?}\n optimized = {opt_mfp:?}"
+ ),
+ // Filtered on both sides, or the optimized plan errored where the raw
+ // order rejected the row first. Both are allowed.
+ (None, Ok(None)) | (None, Err(_)) => {}
}
}
Ok(())
@@ -476,31 +609,53 @@ fn run_temporal(
return Ok(());
};
- // Sample the logical times to probe, and build each substitution reference
- // plan once (it depends only on `t`, not on the row). A reference plan that
- // fails to lower is dropped: we simply assert nothing at that time.
- let mut times: Vec<(u64, mz_expr::SafeMfpPlan)> = Vec::with_capacity(TIMES_PER_MFP);
+ // Logical times probed against every row.
+ let mut sampled = Vec::with_capacity(TIMES_PER_MFP);
for _ in 0..TIMES_PER_MFP {
- let t = u.int_in_range(0u64..=TS_BOUND + 2)?;
- if let Some(ref_plan) = reference_plan(&mfp, t) {
- times.push((t, ref_plan));
- }
+ sampled.push(u.int_in_range(0u64..=TS_BOUND + 2)?);
}
+ // A substitution reference depends only on `t`, not on the row, so it is
+ // built once per time and shared across rows. `None` records that lowering
+ // rejected it, meaning we assert nothing at that time. The map is filled
+ // lazily because the interval endpoints probed below are row-dependent.
+ let mut refs: BTreeMap> = BTreeMap::new();
+
for _ in 0..ROWS_PER_MFP {
let row = gen_input_row(u, types)?;
- let interval = temporal_interval(&plan, &row);
// A plan that errors on this row lets the lowered plan legitimately
// differ from the substitution reference, so assert nothing.
- let interval = match interval {
- Err(()) => continue,
- Ok(iv) => iv,
+ let Ok(interval) = temporal_interval(&plan, &row) else {
+ continue;
};
- for (t, ref_plan) in × {
- // One-directional: only constrain the plan when the reference is a
- // clean pass/fail. A reference error lets the lowered plan differ.
- let Some(present_ref) = reference_present(ref_plan, &row) else {
+ let mut times: BTreeSet = sampled.iter().copied().collect();
+ // Probe the interval's own endpoints, not just sampled times. An
+ // off-by-one in the lowering (a missing or spurious `StepMzTimestamp`,
+ // or a wrong width for `mz_now() = e`) moves an endpoint by one, so it
+ // is observable at that endpoint and at no other time. Sampled times
+ // reach an endpoint only incidentally, and mostly the endpoint 0, which
+ // is what every draw decodes to once a short input leaves `Unstructured`
+ // with no data. A nonzero endpoint needs the sample to hit it exactly.
+ if let Some((lower, upper)) = &interval {
+ for t in [Some(u64::from(*lower)), upper.map(u64::from)] {
+ let Some(t) = t else { continue };
+ times.insert(t);
+ if let Some(before) = t.checked_sub(1) {
+ times.insert(before);
+ }
+ }
+ }
+
+ for t in times {
+ let ref_plan = refs.entry(t).or_insert_with(|| reference_plan(&mfp, t));
+ // One-directional: only constrain the plan when the reference lowers
+ // and is a clean pass/fail. A reference error lets the lowered plan
+ // differ.
+ let Some(present_ref) = ref_plan
+ .as_ref()
+ .and_then(|ref_plan| reference_present(ref_plan, &row))
+ else {
continue;
};
let present_plan = match &interval {
@@ -508,7 +663,7 @@ fn run_temporal(
// failed): it is absent at every `t`.
None => false,
Some((lower, upper)) => {
- let tt = Timestamp::new(*t);
+ let tt = Timestamp::new(t);
*lower <= tt && upper.map(|up| tt < up).unwrap_or(true)
}
};
@@ -563,6 +718,36 @@ fn run(u: &mut Unstructured) -> arbitrary::Result<()> {
.filter(filters)
.project(projection);
+ // Two mechanisms let `optimize` legally change what a row yields once lazy
+ // `And`/`Or` evaluation absorbs an operand error (`Or::eval` returns `true`
+ // the moment it sees one, dropping any error it collected, and `And::eval`
+ // does the same for `false`). Reduce's generic error propagation, run by
+ // `optimize` on every expression, replaces the whole call with any
+ // operand's literal error even though `eval` would have absorbed it (the
+ // open bug CLU-137), and the folded literal's non-nullable type can license
+ // a rewrite above it into a different projected value or a dropped row. And
+ // memoization hoists a fallible subexpression shared under an `And`/`Or`
+ // into a mapped column that `evaluate_inner` runs eagerly, surfacing the
+ // absorbed error. The latter is not a bug: `AND`/`OR` evaluation order is
+ // undefined, in Materialize as in Postgres (STG-54), so the surfaced error
+ // is a legal outcome of the same plan.
+ //
+ // Non-temporal mode handles both row-precisely, skipping exactly the rows
+ // that absorb an operand error (see `absorbs_and_or_operand_error`).
+ // Temporal mode skips the whole MFP on the coarse, expression-level
+ // predicate: its reference side substitutes a literal for `mz_now()` and
+ // re-runs `optimize` per probed time, so "absorbs on this row" has no
+ // single answer there.
+ if temporal
+ && mfp
+ .expressions
+ .iter()
+ .chain(mfp.predicates.iter().map(|(_, p)| p))
+ .any(|e| e.could_hit_nonstrict_error_fold())
+ {
+ return Ok(());
+ }
+
if temporal {
run_temporal(u, mfp, &types)
} else {
diff --git a/src/expr/fuzz/fuzz_targets/mir_scalar_reduce.rs b/src/expr/fuzz/fuzz_targets/mir_scalar_reduce.rs
index cc002a60a841d..975bd23ab40e6 100644
--- a/src/expr/fuzz/fuzz_targets/mir_scalar_reduce.rs
+++ b/src/expr/fuzz/fuzz_targets/mir_scalar_reduce.rs
@@ -37,17 +37,36 @@
//! batch of random rows. The check is mostly one-directional: reduce is allowed
//! to *eliminate* a runtime error (e.g. `If(c, x, x)` becomes `x`, dropping `c`,
//! and `x AND false` becomes `false`), so we only require that a successful
-//! `Ok(v)` result is preserved exactly. The exception is non-strict AND/OR
-//! error absorption, where the oracle allows a reduced error if the original
-//! row succeeded by absorbing an operand error. Every value type compared is
-//! exact (no float/numeric normalization), so equality is the right oracle.
+//! `Ok(v)` result is preserved exactly. Every value type compared is exact (no
+//! float/numeric normalization), so equality is the right oracle.
+//!
+//! The exception is a row that absorbs a non-strict AND/OR operand error, the
+//! shape of the open bug CLU-137: `Or::eval` returns `true` the moment it sees a
+//! true operand and drops any error it collected, while reduce's error
+//! propagation replaces the whole call with the operand's literal error. Such a
+//! row is skipped outright.
+//!
+//! NOTE: this carve-out used to be gated on the reduced side also being an error,
+//! on the reasoning that "an absorbed error cannot resurface as a value". That is
+//! false, and the fuzzer falsified it: once the absorbed error becomes a
+//! `Literal(Err)`, every rewrite downstream is reading a value the original never
+//! produced, and some of them yield an `Ok` rather than propagating. The observed
+//! case reduced `CastBoolToString(IsNull(If(..)))` from `Ok("true")` to a literal
+//! `Ok("false")`, with `Or([Literal(Err), Column(3)])` folding to `Literal(Err)`
+//! where it evaluates to `Ok(True)`. The wrong value is downstream of CLU-137,
+//! not a second defect, so the whole row goes.
+//!
+//! Leaf and atom distributions are tuned against the folds rather than left
+//! uniform, because a literal-heavy expression is annihilated by constant
+//! folding, null propagation, or error propagation long before it reaches the
+//! rewrites above. See `gen_leaf` and `gen_bool_atoms`.
#![no_main]
use libfuzzer_sys::arbitrary::{self, Arbitrary, Unstructured};
use libfuzzer_sys::fuzz_target;
use mz_expr::func::variadic::{And, Or};
-use mz_expr::{func, Eval, EvalError, MirScalarExpr, VariadicFunc};
+use mz_expr::{Eval, EvalError, MirScalarExpr, VariadicFunc, func};
use mz_repr::{Datum, ReprColumnType, ReprScalarType, Row, RowArena};
// Column layout: a contiguous block per type. Columns are nullable.
@@ -137,10 +156,20 @@ fn nonstr_datum(u: &mut Unstructured, ty: Ty) -> arbitrary::Result arbitrary::Result {
let st = scalar_ty(ty);
- Ok(match u.int_in_range(0u8..=3)? {
- 0 => {
+ Ok(match u.int_in_range(0u8..=5)? {
+ 0..=2 => {
let col = match ty {
Ty::Int => COL_INT0 + u.int_in_range(0..=N_INT - 1)?,
Ty::Long => COL_LONG0 + u.int_in_range(0..=N_LONG - 1)?,
@@ -149,14 +178,14 @@ fn gen_leaf(u: &mut Unstructured, ty: Ty) -> arbitrary::Result {
};
MirScalarExpr::column(col)
}
- 1 => match ty {
+ 3 => match ty {
Ty::Str => {
let s = gen_string(u)?;
MirScalarExpr::literal_ok(Datum::String(&s), st)
}
_ => MirScalarExpr::literal_ok(nonstr_datum(u, ty)?, st),
},
- 2 => MirScalarExpr::literal_null(st),
+ 4 => MirScalarExpr::literal_null(st),
// An error literal exercises reduce's error propagation/ordering.
_ => MirScalarExpr::literal(Err(EvalError::DivisionByZero), st),
})
@@ -173,11 +202,27 @@ fn gen_bool_atoms(u: &mut Unstructured, depth: u32) -> arbitrary::Result arbitrary::Result {
- match u.int_in_range(0u8..=4)? {
+ match u.int_in_range(0u8..=7)? {
0 => {
let cond = gen_expr(u, Ty::Bool, d)?;
let then = gen_expr(u, Ty::Long, d)?;
let els = gen_expr(u, Ty::Long, d)?;
Ok(cond.if_then_else(then, els))
}
+ // int8 arithmetic, mirroring the int4 vocabulary: `%` divides by
+ // zero and negate/abs overflow at `i64::MIN`.
1 => Ok(gen_expr(u, Ty::Long, d)?
.call_binary(gen_expr(u, Ty::Long, d)?, func::AddInt64)),
2 => Ok(gen_expr(u, Ty::Long, d)?
.call_binary(gen_expr(u, Ty::Long, d)?, func::SubInt64)),
3 => Ok(gen_expr(u, Ty::Long, d)?
.call_binary(gen_expr(u, Ty::Long, d)?, func::MulInt64)),
+ 4 => Ok(gen_expr(u, Ty::Long, d)?
+ .call_binary(gen_expr(u, Ty::Long, d)?, func::ModInt64)),
+ 5 => Ok(gen_expr(u, Ty::Long, d)?.call_unary(func::NegInt64)),
+ 6 => Ok(gen_expr(u, Ty::Long, d)?.call_unary(func::AbsInt64)),
// Casts that produce an int8.
_ => match u.int_in_range(0u8..=1)? {
0 => Ok(gen_expr(u, Ty::Int, d)?.call_unary(func::CastInt32ToInt64)),
@@ -347,6 +398,25 @@ fn gen_row(u: &mut Unstructured) -> arbitrary::Result {
Ok(row)
}
+/// Does evaluating `expr` on this row absorb an `And`/`Or` operand error?
+///
+/// This is the per-row refinement of `MirScalarExpr::could_hit_nonstrict_error_fold`,
+/// which answers the same question for a whole expression. The coarse predicate is
+/// what the plan-level oracles use; here a row-precise answer is worth the extra
+/// code, because the CLU-137 shape is common in this generator and skipping every
+/// expression carrying it would cost most of the comparisons.
+///
+/// It models `And` and `Or`, the only non-strict variadics this target generates.
+/// `ErrorIfNull`, the third one the shared predicate knows about, is absent from
+/// the vocabulary above; adding it there needs a matching arm here.
+///
+/// NOTE: this deliberately evaluates *every* operand of an `And`/`Or`, even
+/// though `And::eval` short-circuits on the first `false` and so may never reach
+/// a later erroring operand. Reduce's error propagation is equally
+/// position-insensitive (it fires on any literal-error operand, wherever it
+/// sits), so a short-circuit-aware version here would report "no absorption" for
+/// rows whose reduced form is nonetheless an error, and the oracle would flag
+/// them.
fn absorbs_and_or_operand_error<'a>(
expr: &'a MirScalarExpr,
datums: &[Datum<'a>],
@@ -413,12 +483,15 @@ fn run(u: &mut Unstructured) -> arbitrary::Result<()> {
let folded = reduced.eval(&datums, &arena);
// The invariant is one-directional. `reduce` is permitted to eliminate
// a runtime error, for example when `reduce_if` collapses `If(c, x, x)`
- // to `x` and drops `c`. So an `Err` original may become anything. The
- // known exception in the other direction: non-strict AND/OR error
- // absorption, where a successful `false AND ` or
- // `true OR ` may reduce through the absorbed error. Outer
- // expressions can then fold that error into another value, for example
- // `IS NULL` reducing to `false`, so skip the whole known shape.
+ // to `x` and drops `c`. So an `Err` original may become anything.
+ //
+ // A row that absorbed a non-strict AND/OR operand error is skipped
+ // entirely, not merely allowed to reduce to an error. Reduce's error
+ // propagation is not absorption-aware (CLU-137), so from the moment it
+ // installs that `Literal(Err)` the reduced expression is evaluating
+ // something the original never computed, and the divergence surfaces as a
+ // wrong *value* as readily as an error. See the module doc for the case
+ // that showed it.
if original.is_ok() {
if absorbs_and_or_operand_error(&expr, &datums, &arena) {
continue;
diff --git a/src/expr/fuzz/fuzz_targets/timezone_convert.rs b/src/expr/fuzz/fuzz_targets/timezone_convert.rs
index 80f7651d63f7e..7dc0d154d8afb 100644
--- a/src/expr/fuzz/fuzz_targets/timezone_convert.rs
+++ b/src/expr/fuzz/fuzz_targets/timezone_convert.rs
@@ -19,6 +19,11 @@
//! barely run. So most of the time we pick a real IANA zone *with DST* (and a
//! few fixed offsets) so the transition math actually executes. A minority arm
//! still feeds an arbitrary string to keep the parser's reject paths covered.
+//!
+//! The result is packed into a `Row` and read back rather than dropped: the
+//! leap-second bugs this math is prone to produce an out-of-contract
+//! `NaiveTime` that `chrono` hands back without complaint, and only panic once
+//! something decodes a `Row` holding it. Evaluating alone would miss them.
#![no_main]
@@ -28,7 +33,7 @@ use libfuzzer_sys::fuzz_target;
use mz_expr::{Eval, MirScalarExpr, UnaryFunc, func};
use mz_pgtz::timezone::{Timezone, TimezoneSpec};
use mz_repr::adt::timestamp::CheckedTimestamp;
-use mz_repr::{Datum, ReprScalarType, RowArena};
+use mz_repr::{Datum, ReprScalarType, Row, RowArena};
/// Real zones whose offsets shift (DST / sub-hour / historical), plus a couple
/// of fixed offsets, the inputs that actually exercise the conversion math.
@@ -46,6 +51,11 @@ const ZONES: &[&str] = &[
"UTC",
"+05:30",
"-08",
+ // Sub-minute offset. The whole-minute offsets above can only ever move a
+ // leap second to another `:59`, so they never reach the folding branch in
+ // `checked_{add,sub}_with_leapsecond`. The named zones do reach it, but
+ // only at their pre-standardization LMT offsets.
+ "+00:00:01",
];
fn run(u: &mut Unstructured) -> arbitrary::Result<()> {
@@ -67,8 +77,32 @@ fn run(u: &mut Unstructured) -> arbitrary::Result<()> {
};
tz
};
- let secs = u.int_in_range(-8_000_000_000_000i64..=8_000_000_000_000)?;
- let nanos = u.int_in_range(0u32..=999_999_999)?;
+ // 3-in-4 draws land in 1900-2100, where the DST transitions of the zones
+ // above actually live. A uniform draw over the whole representable range
+ // (±253,000 years) would hit an ambiguous or nonexistent local time with
+ // probability ~5e-8. The wide arm bottoms out at `LOW_DATE` (-4713-12-31,
+ // -210_863_606_400): `CheckedTimestamp` rejects everything below that, so
+ // drawing down to chrono's own limit instead would leave roughly half of
+ // all iterations evaluating nothing at all.
+ let mut secs = if u.int_in_range(0u8..=3)? != 0 {
+ u.int_in_range(-2_208_988_800i64..=4_102_444_800)?
+ } else {
+ u.int_in_range(-210_863_606_400i64..=8_000_000_000_000)?
+ };
+ // chrono encodes a leap second as `nanos >= 1_000_000_000`, and only accepts
+ // that encoding when the second-of-minute is 59. That encoding is exactly
+ // what the folding in `checked_{add,sub}_with_leapsecond` exists for, so
+ // construct it deliberately rather than hoping a uniform `nanos` draw lands
+ // there (it never can) or that `secs` happens to be aligned.
+ let nanos = if bool::arbitrary(u)? {
+ // `DateTime::from_timestamp` derives the second-of-day as
+ // `secs.rem_euclid(86_400)`, and `86_400 % 60 == 0`, so aligning `secs`
+ // on `rem_euclid(60) == 59` satisfies chrono's precondition.
+ secs = secs - secs.rem_euclid(60) + 59;
+ u.int_in_range(1_000_000_000u32..=1_999_999_999)?
+ } else {
+ u.int_in_range(0u32..=999_999_999)?
+ };
let Some(dt) = DateTime::from_timestamp(secs, nanos) else {
return Ok(());
};
@@ -78,14 +112,20 @@ fn run(u: &mut Unstructured) -> arbitrary::Result<()> {
if let Ok(ts) = CheckedTimestamp::from_timestamplike(dt.naive_utc()) {
let expr = MirScalarExpr::literal_ok(Datum::Timestamp(ts), ReprScalarType::Timestamp)
.call_unary(UnaryFunc::TimezoneTimestamp(func::TimezoneTimestamp(tz)));
- let _ = expr.eval(&[], &arena);
+ if let Ok(d) = expr.eval(&[], &arena) {
+ let _ = Row::pack_slice(&[d]).unpack_first();
+ }
}
// TIMESTAMPTZ `AT TIME ZONE tz` -> TIMESTAMP.
if let Ok(tstz) = CheckedTimestamp::from_timestamplike(dt) {
let expr = MirScalarExpr::literal_ok(Datum::TimestampTz(tstz), ReprScalarType::TimestampTz)
- .call_unary(UnaryFunc::TimezoneTimestampTz(func::TimezoneTimestampTz(tz)));
- let _ = expr.eval(&[], &arena);
+ .call_unary(UnaryFunc::TimezoneTimestampTz(func::TimezoneTimestampTz(
+ tz,
+ )));
+ if let Ok(d) = expr.eval(&[], &arena) {
+ let _ = Row::pack_slice(&[d]).unpack_first();
+ }
}
Ok(())
}
diff --git a/src/expr/fuzz/like_pattern_compile.dict b/src/expr/fuzz/like_pattern_compile.dict
new file mode 100644
index 0000000000000..68ef0f068b31d
--- /dev/null
+++ b/src/expr/fuzz/like_pattern_compile.dict
@@ -0,0 +1,20 @@
+# libFuzzer dictionary for the like_pattern_compile target, resolved by
+# `dict_for` in test/cargo-fuzz/mzcompose.py.
+#
+# These tokens pay off only on the target's fully-arbitrary arm, where the input
+# bytes become the pattern and the text directly. Random Unicode there almost
+# never contains a LIKE metacharacter, so that arm stays on trivial literal
+# patterns without help. On the structured arm the input is a recipe of generator
+# choices, and a token is just more choice bytes carrying no structural signal.
+
+# The metacharacters, and an escape sequence.
+"%"
+"_"
+"\\"
+"\\%"
+# Two `%` with non-empty suffixes: the shape that routes to the regex engine.
+"%a%b"
+# Multi-byte literals, so byte offsets and character counts diverge in the
+# `%` backtracking: é (U+00E9, 2 bytes) and 漢 (U+6F22, 3 bytes).
+"\xc3\xa9"
+"\xe6\xbc\xa2"
diff --git a/src/expr/src/scalar.rs b/src/expr/src/scalar.rs
index 37d325c422922..1be02a7ee0144 100644
--- a/src/expr/src/scalar.rs
+++ b/src/expr/src/scalar.rs
@@ -1407,6 +1407,44 @@ impl VisitChildren for MirScalarExpr {
}
impl MirScalarExpr {
+ /// Reports whether this expression contains a non-strict variadic call with an
+ /// operand that could error, the shape of the open bug CLU-137.
+ ///
+ /// `And`, `Or` and `ErrorIfNull` do not evaluate every operand: `Or::eval`
+ /// returns `true` the moment it sees a true operand and drops any error it
+ /// collected, `And::eval` does the same for `false`, and `ErrorIfNull`
+ /// evaluates its message operand only when the first operand is NULL. Yet
+ /// `reduce_call_variadic`'s generic error propagation replaces the whole call
+ /// with any operand's literal error, wherever it sits. So `reduce` can turn a
+ /// row the expression should evaluate into an error, and, because that literal
+ /// is typed non-nullable, it can go on to license a nullability-dependent
+ /// rewrite and yield a different *value* rather than an error.
+ ///
+ /// Fuzz oracles that compare evaluation across `reduce` use this to skip the
+ /// shape rather than rediscover CLU-137 on every run. It lives here, next to
+ /// the fold it describes, so the several oracles that need it cannot drift
+ /// apart on which functions count as non-strict.
+ ///
+ /// Deliberately conservative: it asks whether an operand *could* error rather
+ /// than whether it already holds a literal error, because `reduce` folds a
+ /// column-free fallible operand (`1 / 0`) to a literal error first and absorbs
+ /// it after.
+ pub fn could_hit_nonstrict_error_fold(&self) -> bool {
+ let mut hit = false;
+ self.visit_pre(|e| {
+ if let MirScalarExpr::CallVariadic { func, exprs } = e {
+ let non_strict = matches!(
+ func,
+ VariadicFunc::And(_) | VariadicFunc::Or(_) | VariadicFunc::ErrorIfNull(_)
+ );
+ if non_strict && exprs.iter().any(|operand| operand.could_error()) {
+ hit = true;
+ }
+ }
+ });
+ hit
+ }
+
/// Iterates through references to child expressions.
pub fn children(&self) -> impl DoubleEndedIterator- {
let mut first = None;
diff --git a/src/interchange/fuzz/fuzz_targets/avro_decode_fuzzed_schema.rs b/src/interchange/fuzz/fuzz_targets/avro_decode_fuzzed_schema.rs
index 73aa7da8de494..12f646698c7dd 100644
--- a/src/interchange/fuzz/fuzz_targets/avro_decode_fuzzed_schema.rs
+++ b/src/interchange/fuzz/fuzz_targets/avro_decode_fuzzed_schema.rs
@@ -25,9 +25,20 @@
//! and *Avro-binary-encode a random value against that same type*, so the body
//! is valid by construction and the decoder walks all the way through. Coverage
//! guidance then learns which byte streams produce which shapes. We don't lose
-//! the error-path coverage a random body gave, though: a quarter of the inputs
-//! feed the raw remaining bytes, and others truncate or single-byte-corrupt the
-//! valid encoding. Either way, an accepted schema must never panic.
+//! the error-path coverage a random body gave, though: up to a quarter of the
+//! inputs feed the raw remaining bytes, and others truncate or
+//! single-byte-corrupt the valid encoding. Either way, an accepted schema must
+//! never panic.
+//!
+//! NOTE: every "don't build a valid body" branch must be gated on there being
+//! input left. `Unstructured::int_in_range` does not fail on an exhausted
+//! `Unstructured`, it returns the *low end* of the range forever, and schema
+//! generation consumes input greedily, so an ungated `int_in_range(0..=3)? == 0`
+//! branch is taken by every exhausted input. That funnelled the majority of
+//! executions into a zero-length body (`take_rest` on nothing, or
+//! truncate-to-zero), which fails at the first `read_exact` and walks none of
+//! the deep decode logic this target exists to reach. Exhaustion must fall
+//! through to the clean-encoding path, never select a degenerate one.
//!
//! Beyond "never panic", we add one error oracle. Most bodies *should* be
//! rejected (random bytes, truncations, an out-of-range `enum` index, an
@@ -37,8 +48,21 @@
//! (plain scalars, `fixed`, and structural composites over them, see
//! `decode_infallible`), the decoder is round-tripping bytes it *must* accept,
//! so there a decode error is a real bug and we assert success. A panic-only
-//! oracle never notices a "valid input wrongly rejected" regression (cf.
-//! #37087's deferred union-promotion error).
+//! oracle never notices a "valid input wrongly rejected" regression.
+//!
+//! NOTE: the scope of that oracle is the *identity-resolution* decode path only.
+//! We build the decoder with `WriterSchemaProvider::None`, for which
+//! `AvroSchemaResolver::resolve` hands back the reader schema verbatim, so no
+//! `SchemaPiece::Resolve*` piece is ever constructed. The deferred
+//! union-promotion class of regression (#37087), where a failed writer/reader
+//! variant pairing is stored as an `Err` inside the resolved union and re-raised
+//! at decode time, lives entirely in that resolution machinery and cannot be
+//! reached from here. It is covered by
+//! `src/avro/fuzz/fuzz_targets/schema_resolve.rs`, which fuzzes `resolve_schemas`
+//! against `from_avro_datum`. Reaching it through the interchange-side
+//! Row-packing decoder would need a separate target that builds the resolved
+//! schema with `resolve_schemas` and hands it to `AvroFlatDecoder` directly,
+//! since `Decoder` offers no way to inject a writer schema without a registry.
//!
//! A fraction of inputs instead drive a round-trip *correctness* oracle
//! (`run_roundtrip`): a decode that succeeds but yields the *wrong* datum slips
@@ -49,10 +73,12 @@
//!
//! Rather than freeze the schema-dependent knobs at one value each, we vary the
//! ones that drive distinct decode arithmetic: `decimal` precision (1..=39, the
-//! `NUMERIC_DATUM_MAX_PRECISION` boundary), scale (0..=precision, where
-//! `parse_decimal` rejects scale > precision and `twos_complement_be_to_numeric`
-//! interprets it), and the backing `fixed` size, so the two's-complement byte
-//! run and the precision/scale interaction are not pinned. The decimal
+//! `NUMERIC_DATUM_MAX_PRECISION` boundary, further capped by the backing `fixed`
+//! size so `parse_fixed` does not silently demote the decimal to a plain
+//! `fixed`), scale (0..=precision, where `parse_decimal` rejects scale >
+//! precision and `twos_complement_be_to_numeric` interprets it), and the backing
+//! `fixed` size, so the two's-complement byte run and the precision/scale
+//! interaction are not pinned. The decimal
//! *value* bytes are likewise biased (see `push_twos_complement` /
//! `gen_decimal_len`) toward the patterns that stress that arithmetic rather
//! than only uniform-random runs: the empty run (== 0), all-`0x00`/`0xFF` sign
@@ -61,10 +87,23 @@
//! `json` logical field (a `string` tagged `connect.name:io.debezium.data.Json`)
//! whose body is real JSON text, reaching the `AvroFlatDecoder::json` ->
//! `JsonbPacker` path that a plain string never touches. And multi-variant
-//! *essential* unions like `["int","string"]`, accepted only as a record field
-//! (each non-null variant expands to its own nullable column) and rejected
-//! elsewhere, exercising `get_union_columns`' field-invention/expansion logic
-//! that the `["null", T]` nullability pattern alone never reaches.
+//! *essential* unions like `["int","string"]`, which occupy one column per
+//! non-null variant, exercising the multi-column `AvroFlatDecoder::union_branch`
+//! arithmetic (one `Datum` pushed per non-null variant, `Datum::Null` for every
+//! unselected one) that the two-branch `["null", T]` pattern never reaches.
+//!
+//! NOTE: this target stops at `parse_schema`. `Decoder::new` ->
+//! `AvroSchemaResolver::new` -> `parse_schema` is the whole chain, so none of
+//! `src/interchange/src/avro/schema.rs`'s SQL-side validation runs here:
+//! `validate_schema_1` / `validate_schema_2` / `get_union_columns` are reachable
+//! only from `schema_to_relationdesc`, which nothing here calls. The union
+//! column-name invention (`get_union_columns`' `format!("{}{}", n, i + 1)`, the
+//! `UNKNOWN_COLUMN_NAME` fallback, the resulting `RelationDesc`) is therefore
+//! untested by this target. Where the generator does keep to a validator rule
+//! (essential unions only as record fields, decimal precision <= 39) the reason
+//! is that production runs `schema_to_relationdesc` before ever building a
+//! `Decoder`, so those are the only schemas the decoder sees in practice. It is
+//! not that this target would reject the others.
#![no_main]
@@ -125,22 +164,39 @@ enum Ty {
Nullable(Box
),
/// A multi-variant *essential* union of non-null variants, optionally with a
/// leading `null`: e.g. `["int","string"]` or `["null","int","string"]`.
- /// `validate_schema_2` rejects this everywhere except as a record field,
- /// where `get_union_columns` expands it to one nullable column per non-null
- /// variant. Stored as (has_null, variants). Generated only as a record field.
+ /// At decode time `AvroFlatDecoder::union_branch` packs one `Datum` per
+ /// non-null variant. Stored as (has_null, variants). Generated only as a
+ /// record field, because that is the only position the SQL validator accepts
+ /// it in and therefore the only one production reaches (this target does not
+ /// run that validator, see the module docs). Avro itself would accept the
+ /// same union anywhere a type is expected.
EssentialUnion(bool, Vec),
}
-/// Generate a `decimal`'s precision/scale. `parse_decimal` requires
-/// `0 <= scale <= precision` and the SQL validator caps precision at
-/// `NUMERIC_DATUM_MAX_PRECISION` (39). Pick within those bounds so the schema is
-/// accepted and we vary the decode arithmetic across the whole legal range.
-fn gen_decimal_params(u: &mut Unstructured) -> arbitrary::Result<(u32, u32)> {
- let precision = u.int_in_range(1u32..=39)?;
+/// Materialize's `NUMERIC_DATUM_MAX_PRECISION`. The SQL Avro validator caps
+/// `decimal` precision here, and production only hands the decoder schemas that
+/// passed it, so stay within it even though this target never runs that check.
+const MAX_DECIMAL_PRECISION: u32 = 39;
+
+/// Generate a `decimal`'s precision/scale with precision in
+/// `1..=max_precision`. `parse_decimal` requires `0 <= scale <= precision`, so
+/// pick the scale from the chosen precision and vary the decode arithmetic
+/// across the whole legal range.
+fn gen_decimal_params(u: &mut Unstructured, max_precision: u32) -> arbitrary::Result<(u32, u32)> {
+ let precision = u.int_in_range(1u32..=max_precision)?;
let scale = u.int_in_range(0u32..=precision)?;
Ok((precision, scale))
}
+/// The largest `decimal` precision `parse_fixed` will honour over a `size`-byte
+/// `fixed`, mirroring its own bound. Past it the logical type is silently
+/// demoted to a plain `fixed` (a `warn!`, not a parse error), so the schema
+/// stops exercising the decimal path and the harness is left believing a
+/// `DecimalFixed` node is a decimal when the decoder sees a `fixed`.
+fn max_precision_over_fixed(size: u32) -> u32 {
+ ((f64::from(8 * size - 1)) * 2f64.log10()).floor() as u32
+}
+
/// Generate one syntactically valid Avro type. `counter` keeps named types
/// (record/enum/fixed) unique within the schema, since duplicate names make Avro
/// schema parsing fail, wasting the whole input. This never returns an
@@ -164,15 +220,19 @@ fn gen_ty(u: &mut Unstructured, counter: &mut u32, depth: u32) -> arbitrary::Res
_ => Ty::Bytes,
},
1 => {
- let (p, s) = gen_decimal_params(u)?;
+ let (p, s) = gen_decimal_params(u, MAX_DECIMAL_PRECISION)?;
Ty::DecimalBytes(p, s)
}
2 => {
*counter += 1;
- let (p, s) = gen_decimal_params(u)?;
// `fixed` requires a positive size. Allow runs both shorter and
// longer than the canonical 16/24 to vary the two's-complement path.
let size = u.int_in_range(1u32..=40)?;
+ // Bound the precision by the byte run, else `parse_fixed` demotes
+ // the decimal to a plain `fixed` and this node stops testing the
+ // decimal path at all (`Ty::Fixed` already covers plain runs).
+ let max = max_precision_over_fixed(size).min(MAX_DECIMAL_PRECISION);
+ let (p, s) = gen_decimal_params(u, max)?;
Ty::DecimalFixed(*counter, size, p, s)
}
3 => Ty::Date,
@@ -181,7 +241,9 @@ fn gen_ty(u: &mut Unstructured, counter: &mut u32, depth: u32) -> arbitrary::Res
6 => Ty::Uuid,
7 => {
*counter += 1;
- let size = u.int_in_range(0u32..=24)?;
+ // `parse_fixed` rejects a non-positive size outright, which would
+ // fail `Decoder::new` and discard the whole input, schema and body.
+ let size = u.int_in_range(1u32..=24)?;
Ty::Fixed(*counter, size)
}
8 => {
@@ -201,7 +263,18 @@ fn gen_ty(u: &mut Unstructured, counter: &mut u32, depth: u32) -> arbitrary::Res
}
11 => Ty::Array(Box::new(gen_ty(u, counter, depth - 1)?)),
12 => Ty::Map(Box::new(gen_ty(u, counter, depth - 1)?)),
- _ => Ty::Nullable(Box::new(gen_ty(u, counter, depth - 1)?)),
+ _ => {
+ // `UnionSchema::new` rejects `["null",["null",T]]` ("Unions may not
+ // directly contain a union"), which would fail `Decoder::new` and
+ // discard the whole input. Collapse rather than re-roll, so the
+ // input the inner type already consumed is not wasted either.
+ let inner = gen_ty(u, counter, depth - 1)?;
+ if matches!(inner, Ty::Nullable(_)) {
+ inner
+ } else {
+ Ty::Nullable(Box::new(inner))
+ }
+ }
})
}
@@ -377,7 +450,11 @@ fn encode_json(u: &mut Unstructured, out: &mut Vec) -> arbitrary::Result<()>
}
5 => "[]".into(),
6 => "{}".into(),
- 7 => format!("[{},{},null]", u.arbitrary::()?, u.arbitrary::()?),
+ 7 => format!(
+ "[{},{},null]",
+ u.arbitrary::()?,
+ u.arbitrary::()?
+ ),
// Not valid JSON: exercise the decoder's BadJson error path.
_ => "{".into(),
};
@@ -392,7 +469,11 @@ fn encode_json(u: &mut Unstructured, out: &mut Vec) -> arbitrary::Result<()>
/// (`len <= 17`), the `negate_twos_complement_le` path (any negative value), and
/// the wide-representation precision-overflow handling. Most of the time it
/// still emits a uniform-random run so coverage guidance keeps exploring.
-fn push_twos_complement(u: &mut Unstructured, len: usize, out: &mut Vec) -> arbitrary::Result<()> {
+fn push_twos_complement(
+ u: &mut Unstructured,
+ len: usize,
+ out: &mut Vec,
+) -> arbitrary::Result<()> {
let fill = match u.int_in_range(0u8..=9)? {
0 => 0x00u8, // zero / positive sign-extension
1 => 0xFF, // -1 / negative sign-extension
@@ -459,7 +540,11 @@ fn encode_value(u: &mut Unstructured, ty: &Ty, out: &mut Vec) -> arbitrary::
// The fixed-backed decimal reads exactly `size` bytes (no length prefix),
// so only the byte pattern varies.
Ty::DecimalFixed(_, size, _, _) => {
- push_twos_complement(u, usize::try_from(*size).expect("fixed size fits usize"), out)?;
+ push_twos_complement(
+ u,
+ usize::try_from(*size).expect("fixed size fits usize"),
+ out,
+ )?;
}
Ty::Json => encode_json(u, out)?,
Ty::Uuid => {
@@ -703,10 +788,11 @@ fn run_roundtrip(u: &mut Unstructured) -> arbitrary::Result<()> {
}
schema.push_str("]}");
- let Ok(mut decoder) = Decoder::new(&schema, &[], WriterSchemaProvider::None, "fuzz".into()) else {
- // A record of plain scalars always validates. If not, nothing to check.
- return Ok(());
- };
+ // `Decoder::new` only parses the schema, and this one is built from a fixed
+ // template over plain scalars, so a parse failure is a harness bug rather
+ // than an input we should skip.
+ let mut decoder = Decoder::new(&schema, &[], WriterSchemaProvider::None, "fuzz".into())
+ .expect("plain-scalar record schema parses");
let mut body = Vec::new();
for (val, nullable) in &cols {
@@ -728,7 +814,9 @@ fn run_roundtrip(u: &mut Unstructured) -> arbitrary::Result<()> {
"Avro decode produced the wrong row for a plain-scalar record\nschema = {schema}",
),
Ok(Err(e)) => panic!("plain-scalar record failed to decode (schema = {schema}): {e}"),
- Err(e) => panic!("plain-scalar record hit a transient decode error (schema = {schema}): {e}"),
+ Err(e) => {
+ panic!("plain-scalar record hit a transient decode error (schema = {schema}): {e}")
+ }
}
Ok(())
}
@@ -741,8 +829,8 @@ fn run(mut u: Unstructured) -> arbitrary::Result<()> {
return run_roundtrip(&mut u);
}
- // Top-level reader schema: a record whose fields span everything
- // `validate_schema_2` accepts.
+ // Top-level reader schema: a record whose fields span every single-column
+ // shape the decoder handles.
let mut counter = 0u32;
let nfields = u.int_in_range(1u8..=8)?;
let mut fields = Vec::with_capacity(nfields.into());
@@ -757,37 +845,51 @@ fn run(mut u: Unstructured) -> arbitrary::Result<()> {
// No CSR client and confluent_wire_format = false, so decode is
// self-contained (no network) over the generated reader schema.
- let Ok(mut decoder) = Decoder::new(&schema, &[], WriterSchemaProvider::None, "fuzz".into()) else {
+ let Ok(mut decoder) = Decoder::new(&schema, &[], WriterSchemaProvider::None, "fuzz".into())
+ else {
return Ok(());
};
- // Body: usually a valid encoding (so the decoder runs deep), but a quarter
- // of the time the raw remaining bytes, and otherwise a valid encoding
- // occasionally truncated or single-byte corrupted. The non-valid forms keep
- // the decoder's error paths covered: short read, bad length/union tag,
- // inconsistent content.
+ // Body: usually a valid encoding (so the decoder runs deep), but up to a
+ // quarter of the time the raw remaining bytes, and otherwise a valid
+ // encoding occasionally truncated or single-byte corrupted. The non-valid
+ // forms keep the decoder's error paths covered: short read, bad length/union
+ // tag, inconsistent content.
+ // Both "don't build a valid body" decisions are gated on input remaining,
+ // because an exhausted `Unstructured` returns the low end of every range and
+ // would otherwise steer the majority of executions into an empty body (see
+ // the module docs).
// `assert_success` is set only when the body is a *clean* (uncorrupted)
// encoding of a type tree every node of which is guaranteed to decode for
// any value the encoder can produce (see `decode_infallible`). In that one
// case the decoder is round-tripping bytes it must accept, so a decode
// *error* is a real bug. The panic-only oracle that the random-bytes,
// truncated, corrupted, and value-validated (`enum`/`decimal`/`json`/…)
- // arms rely on would silently miss it, the same class of regression as the
- // deferred union-promotion error in #37087.
+ // arms rely on would silently miss it. This covers the identity-resolution
+ // decode path only, see the module docs for the resolved-schema gap.
let mut assert_success = false;
- let body = if u.int_in_range(0u8..=3)? == 0 {
+ // Only take the raw tail when there actually is one to feed.
+ let body = if u.len() >= 8 && u.int_in_range(0u8..=3)? == 0 {
u.take_rest().to_vec()
} else {
let mut b = Vec::new();
encode_value(&mut u, &row, &mut b)?;
- match u.int_in_range(0u8..=9)? {
- 0 if !b.is_empty() => {
- let keep = u.int_in_range(0usize..=b.len())?;
+ let corruption = if u.is_empty() {
+ None
+ } else {
+ Some(u.int_in_range(0u8..=9)?)
+ };
+ match corruption {
+ // `0..=len - 1`, not `0..=len`: a `keep == len` draw would leave the
+ // body untouched, so the arm would not always truncate.
+ Some(0) if !b.is_empty() => {
+ let keep = u.int_in_range(0usize..=b.len() - 1)?;
b.truncate(keep);
}
- 1 if !b.is_empty() => {
+ // A nonzero mask, so the arm always flips at least one bit.
+ Some(1) if !b.is_empty() => {
let i = u.int_in_range(0usize..=b.len() - 1)?;
- b[i] ^= u.arbitrary::()?;
+ b[i] ^= u.int_in_range(1u8..=u8::MAX)?;
}
// Uncorrupted body: if its type is guaranteed decodable, the decode
// below must succeed.
@@ -809,6 +911,15 @@ fn run(mut u: Unstructured) -> arbitrary::Result<()> {
"decoder rejected a clean, in-range Avro body whose every field is guaranteed \
decodable; such a round-trip must succeed",
);
+ // `decode` reads through `bytes`, so a clean body must be consumed in
+ // full. A length-accounting bug that under-consumes while still packing
+ // a well-formed `Row` passes every other oracle here.
+ assert!(
+ bytes.is_empty(),
+ "decoder left {} of {} bytes of a clean Avro body unconsumed (schema = {schema})",
+ bytes.len(),
+ body.len(),
+ );
}
Ok(())
}
diff --git a/src/interchange/fuzz/fuzz_targets/json_encode.rs b/src/interchange/fuzz/fuzz_targets/json_encode.rs
index f660328db5424..6bbdf039c839d 100644
--- a/src/interchange/fuzz/fuzz_targets/json_encode.rs
+++ b/src/interchange/fuzz/fuzz_targets/json_encode.rs
@@ -16,6 +16,13 @@
//! that can't be serialized, corrupts/halts a sink, so encoding then serializing
//! must never panic for any well-typed row.
//!
+//! The blast radius is wider than the sinks. `TypedDatum::json` is also the
+//! result encoder for the HTTP `/api/sql` and WebSocket SQL APIs, which carry no
+//! `catch_unwind`, so a panic there aborts `environmentd` under
+//! `install_enhanced_handler` rather than just failing one sink task. Those
+//! callers pass `JsonNumberPolicy::ConvertNumberToString`, while
+//! `encode_datums_as_json` hardcodes `KeepAsNumber`, so we drive both.
+//!
//! Beyond scalars, we generate *composite* column types (`List`, `Map`,
//! `Record`, multi-dimensional `Array`, and `Jsonb`) plus the remaining scalar
//! shapes with their own encode logic (`Uuid`, `Char`/`VarChar` padding,
@@ -33,15 +40,16 @@
use chrono::{DateTime, NaiveTime, Utc};
use libfuzzer_sys::arbitrary::{self, Arbitrary, Unstructured};
use libfuzzer_sys::fuzz_target;
-use mz_interchange::json::encode_datums_as_json;
+use mz_interchange::encode::TypedDatum;
+use mz_interchange::json::{JsonNumberPolicy, ToJson, encode_datums_as_json};
use mz_repr::adt::char::CharLength;
use mz_repr::adt::date::Date;
use mz_repr::adt::interval::Interval;
use mz_repr::adt::jsonb::JsonbPacker;
-use mz_repr::adt::mz_acl_item::{AclItem, MzAclItem};
-use mz_repr::adt::range::Range;
+use mz_repr::adt::mz_acl_item::{AclItem, AclMode, MzAclItem};
+use mz_repr::adt::range::{Range, RangeLowerBound, RangeUpperBound};
use mz_repr::adt::system::Oid;
-use mz_repr::adt::timestamp::CheckedTimestamp;
+use mz_repr::adt::timestamp::{CheckedTimestamp, HIGH_DATE, LOW_DATE};
use mz_repr::adt::varchar::VarCharMaxLength;
use mz_repr::role_id::RoleId;
use mz_repr::strconv::parse_numeric;
@@ -67,29 +75,40 @@ enum GType {
Array(Vec, SqlScalarType),
}
+/// Draw a `DateTime` inside `CheckedTimestamp`'s bounds.
+///
+/// Drawing from the full `i64` (or even `±8e12`) second range spends most of the
+/// budget on values `from_timestamplike` rejects, and never lands on the
+/// boundary days. Drawing from `[LOW_DATE 00:00:00, HIGH_DATE 23:59:59]` instead
+/// makes every draw valid, so the two conversions below cannot fail: `secs` is
+/// well inside chrono's own year range, and `nanos` cannot carry past the end of
+/// `HIGH_DATE`.
+fn gen_ts(u: &mut Unstructured) -> arbitrary::Result> {
+ let low = LOW_DATE
+ .and_hms_opt(0, 0, 0)
+ .expect("midnight is a valid time")
+ .and_utc()
+ .timestamp();
+ let high = HIGH_DATE
+ .and_hms_opt(23, 59, 59)
+ .expect("23:59:59 is a valid time")
+ .and_utc()
+ .timestamp();
+ let secs = u.int_in_range(low..=high)?;
+ let nanos = u.int_in_range(0u32..=999_999_999)?;
+ Ok(DateTime::from_timestamp(secs, nanos).expect("secs drawn within chrono's range"))
+}
+
fn gen_naive_ts(
u: &mut Unstructured,
) -> arbitrary::Result> {
- let secs = u.int_in_range(-8_000_000_000_000i64..=8_000_000_000_000)?;
- let nanos = u.int_in_range(0u32..=999_999_999)?;
- Ok(DateTime::from_timestamp(secs, nanos)
- .and_then(|d| CheckedTimestamp::from_timestamplike(d.naive_utc()).ok())
- .unwrap_or_else(|| {
- CheckedTimestamp::from_timestamplike(
- DateTime::from_timestamp(0, 0).unwrap().naive_utc(),
- )
- .unwrap()
- }))
+ Ok(CheckedTimestamp::from_timestamplike(gen_ts(u)?.naive_utc())
+ .expect("drawn within CheckedTimestamp's bounds"))
}
fn gen_utc_ts(u: &mut Unstructured) -> arbitrary::Result>> {
- let secs = u.int_in_range(-8_000_000_000_000i64..=8_000_000_000_000)?;
- let nanos = u.int_in_range(0u32..=999_999_999)?;
- Ok(DateTime::from_timestamp(secs, nanos)
- .and_then(|d| CheckedTimestamp::from_timestamplike(d).ok())
- .unwrap_or_else(|| {
- CheckedTimestamp::from_timestamplike(DateTime::from_timestamp(0, 0).unwrap()).unwrap()
- }))
+ Ok(CheckedTimestamp::from_timestamplike(gen_ts(u)?)
+ .expect("drawn within CheckedTimestamp's bounds"))
}
/// Generate a scalar type (a leaf of the composite tree, and the element type of
@@ -117,11 +136,27 @@ fn gen_scalar_type(u: &mut Unstructured) -> arbitrary::Result {
18 => SqlScalarType::MzTimestamp,
19 => SqlScalarType::Uuid,
20 => {
- let length = CharLength::try_from(i64::from(u.int_in_range(1u8..=12)?)).ok();
+ // An unqualified `bpchar` (`length: None`) reaches `format_str_pad`'s
+ // no-padding branch, which a length-qualified `Char` never does.
+ let length = if u.ratio(1u8, 4u8)? {
+ None
+ } else {
+ Some(
+ CharLength::try_from(i64::from(u.int_in_range(1u8..=12)?))
+ .expect("1..=12 is a valid char length"),
+ )
+ };
SqlScalarType::Char { length }
}
21 => {
- let max_length = VarCharMaxLength::try_from(i64::from(u.int_in_range(1u8..=12)?)).ok();
+ let max_length = if u.ratio(1u8, 4u8)? {
+ None
+ } else {
+ Some(
+ VarCharMaxLength::try_from(i64::from(u.int_in_range(1u8..=12)?))
+ .expect("1..=12 is a valid varchar max length"),
+ )
+ };
SqlScalarType::VarChar { max_length }
}
// Range over Int32. `unwrap_range().to_string()` is the only logic.
@@ -160,7 +195,15 @@ fn gen_type(u: &mut Unstructured, depth: u32) -> arbitrary::Result {
let ndims = u.int_in_range(1usize..=3)?;
let mut dims = Vec::with_capacity(ndims);
for _ in 0..ndims {
- dims.push(u.int_in_range(0usize..=3)?);
+ // A single zero-length axis collapses the cardinality to 0, so
+ // `encode_array` walks no elements at all. Drawing lengths
+ // uniformly from `0..=3` made that the outcome for ~45% of
+ // arrays, so keep it reachable but rare.
+ dims.push(if u.ratio(1u8, 16u8)? {
+ 0
+ } else {
+ u.int_in_range(1usize..=3)?
+ });
}
GType::Array(dims, gen_scalar_type(u)?)
}
@@ -201,6 +244,26 @@ fn to_scalar_type(ty: >ype) -> SqlScalarType {
}
/// Build a small, valid JSON string of bounded depth for the `Jsonb` path.
+///
+/// TODO: Generate serde_json's arbitrary-precision magic key here once SS-157 is
+/// fixed. `JsonbPacker`'s `KeyClassifier` recognises the object key
+/// `$serde_json::private::Number` and runs the payload through
+/// `strconv::parse_numeric`, which accepts positive quiet NaN, bypassing the
+/// `cast_jsonbable_to_jsonb` guard that the SQL cast path relies on. That is the
+/// only route by which a non-finite numeric enters a `Jsonb`, and
+/// `JsonbRef::to_serde_json` then panics on it, so the literal
+/// `{"$serde_json::private::Number":"NaN"}` (and that value nested inside an
+/// array or object, to cover `JsonbDatum::serialize`'s list/map recursion) is
+/// what this generator has to produce to guard the fix. It is left out until the
+/// fix lands, because this target is in the `FRUITFUL` set and would otherwise
+/// report the same known crash on every release-qualification run.
+///
+/// Adding it also requires relaxing the `pack_str` call in `push_typed`: `"NaN"`
+/// is the only payload `parse_numeric` accepts, so `Infinity`, `sNaN`, `1e5000`
+/// and the like make `pack_str` return `Err`, which today's `.expect()` would
+/// turn into a *harness* panic. `pack_str` collects its commands before touching
+/// the packer, so a rejection leaves the packer untouched and a valid
+/// `Datum::JsonNull` can be substituted instead.
fn gen_json_text(u: &mut Unstructured) -> arbitrary::Result {
Ok(match u.int_in_range(0u8..=7)? {
0 => "null".into(),
@@ -244,9 +307,9 @@ fn push_scalar(
// represent, exactly the encoding edge we want to exercise.
SqlScalarType::Float32 => packer.push(Datum::Float32(f32::arbitrary(u)?.into())),
SqlScalarType::Float64 => packer.push(Datum::Float64(f64::arbitrary(u)?.into())),
- SqlScalarType::String
- | SqlScalarType::VarChar { .. }
- | SqlScalarType::Char { .. } => packer.push(Datum::String(<&str>::arbitrary(u)?)),
+ SqlScalarType::String | SqlScalarType::VarChar { .. } | SqlScalarType::Char { .. } => {
+ packer.push(Datum::String(<&str>::arbitrary(u)?))
+ }
SqlScalarType::Bytes => packer.push(Datum::Bytes(<&[u8]>::arbitrary(u)?)),
SqlScalarType::Numeric { .. } => {
let s = format!("{}.{}", i64::arbitrary(u)?, u32::arbitrary(u)?);
@@ -254,8 +317,10 @@ fn push_scalar(
packer.push(Datum::Numeric(n));
}
SqlScalarType::Date => {
- let d = Date::from_pg_epoch(i32::arbitrary(u)?)
- .unwrap_or_else(|_| Date::from_pg_epoch(0).unwrap());
+ // Drawing from the full `i32` day range was rejected ~68% of the
+ // time and never reached the boundary days.
+ let days = u.int_in_range(Date::LOW_DAYS..=Date::HIGH_DAYS)?;
+ let d = Date::from_pg_epoch(days).expect("days drawn within Date's bounds");
packer.push(Datum::Date(d));
}
SqlScalarType::Time => {
@@ -275,14 +340,57 @@ fn push_scalar(
packer.push(Datum::MzTimestamp(Timestamp::from(u64::arbitrary(u)?)))
}
SqlScalarType::Uuid => packer.push(Datum::Uuid(Uuid::from_u128(u.arbitrary::()?))),
- // The empty range is always valid and its `to_string` is exercised.
- SqlScalarType::Range { .. } => packer.push(Datum::Range(Range { inner: None })),
+ // `Display for Range` short-circuits the empty range to the constant
+ // "empty", so only a finite range reaches `RangeInner`/`RangeBound`'s
+ // Display, where the actual rendering logic lives.
+ SqlScalarType::Range { .. } => {
+ if u.ratio(1u8, 8u8)? {
+ packer.push(Datum::Range(Range { inner: None }));
+ } else {
+ // `RangeBound::new` maps `Datum::Null` to an infinite bound.
+ let (a, b) = (i32::arbitrary(u)?, i32::arbitrary(u)?);
+ let lower = RangeLowerBound::new(
+ if u.ratio(1u8, 8u8)? {
+ Datum::Null
+ } else {
+ Datum::Int32(a.min(b))
+ },
+ bool::arbitrary(u)?,
+ );
+ let upper = RangeUpperBound::new(
+ if u.ratio(1u8, 8u8)? {
+ Datum::Null
+ } else {
+ Datum::Int32(a.max(b))
+ },
+ bool::arbitrary(u)?,
+ );
+ // `push_range` canonicalizes before it touches the packer, so a
+ // rejected range (an exclusive bound whose step oversteps
+ // `i32`) leaves the packer untouched and we can substitute the
+ // empty range.
+ if packer.push_range(Range::new(Some((lower, upper)))).is_err() {
+ packer.push(Datum::Range(Range { inner: None }));
+ }
+ }
+ }
+ // `AclMode::empty()` renders no privilege bits at all, so fuzz the mode.
+ // `from_bits_truncate` drops undefined bits, which is what the row
+ // encoding round-trips.
SqlScalarType::MzAclItem => {
- let item = MzAclItem::empty(RoleId::User(u.arbitrary::()?), RoleId::Public);
+ let item = MzAclItem {
+ grantee: RoleId::User(u.arbitrary::()?),
+ grantor: RoleId::Public,
+ acl_mode: AclMode::from_bits_truncate(u64::arbitrary(u)?),
+ };
packer.push(Datum::MzAclItem(item));
}
SqlScalarType::AclItem => {
- let item = AclItem::empty(Oid(u.arbitrary::()?), Oid(u.arbitrary::()?));
+ let item = AclItem {
+ grantee: Oid(u.arbitrary::()?),
+ grantor: Oid(u.arbitrary::()?),
+ acl_mode: AclMode::from_bits_truncate(u64::arbitrary(u)?),
+ };
packer.push(Datum::AclItem(item));
}
// `gen_scalar_type` never produces other types.
@@ -306,7 +414,10 @@ fn push_typed(
packer.push(Datum::Null);
} else {
let text = gen_json_text(u)?;
- // The text is always valid JSON by construction.
+ // The text is always valid JSON by construction, and every
+ // number it contains is one `strconv::parse_numeric` accepts.
+ // See `gen_json_text`'s TODO before generating a payload that
+ // `pack_str` can reject.
JsonbPacker::new(packer)
.pack_str(&text)
.expect("generated valid json");
@@ -381,7 +492,9 @@ fn push_typed(
// The dims and element count match by construction, so this can't
// return an error. If it somehow does, fall back to an empty array.
if packer.try_push_array(&array_dims, elems).is_err() {
- packer.try_push_array(&[], std::iter::empty::()).unwrap();
+ packer
+ .try_push_array(&[], std::iter::empty::())
+ .unwrap();
}
}
}
@@ -420,7 +533,20 @@ fn run(u: &mut Unstructured) -> arbitrary::Result<()> {
// Encode the row's datums as JSON (the sink path) and serialize the result.
// Neither step may panic for any well-typed row.
let value = encode_datums_as_json(row.iter(), &columns);
- let _ = serde_json::to_vec(&value);
+ // Serialize the way the sink does: `JsonEncoder::encode_unchecked` calls
+ // `Value::to_string`, and `ToString` panics when the `Display` impl errors,
+ // which `Display for Value` does for any serialization failure. So a
+ // serialization error is a *panic* in production, where `to_vec` would only
+ // have returned an `Err` that an oracle could not see.
+ let _ = value.to_string();
+
+ // `encode_datums_as_json` hardcodes `KeepAsNumber`. The HTTP `/api/sql` and
+ // WebSocket SQL result encoders pass `ConvertNumberToString`, which recurses
+ // through every composite arm, so drive that policy too.
+ for (datum, (_, typ)) in row.iter().zip(&columns) {
+ let value = TypedDatum::new(datum, typ).json(&JsonNumberPolicy::ConvertNumberToString);
+ let _ = value.to_string();
+ }
Ok(())
}
diff --git a/src/interchange/fuzz/fuzz_targets/protobuf_decode_fuzzed_schema.rs b/src/interchange/fuzz/fuzz_targets/protobuf_decode_fuzzed_schema.rs
index c2cf3c6a0e91b..61668ebaee589 100644
--- a/src/interchange/fuzz/fuzz_targets/protobuf_decode_fuzzed_schema.rs
+++ b/src/interchange/fuzz/fuzz_targets/protobuf_decode_fuzzed_schema.rs
@@ -23,13 +23,15 @@
//! repeated -> list, nested message -> record). Instead we keep the schema
//! structured (`MsgDef`/`FieldTy`), build the `FileDescriptorSet` from it, and
//! protobuf-binary-encode a valid body for the top message against that same
-//! structure, bounding recursion through cyclic message refs by emitting empty
-//! nested messages at the depth limit. Both wire formats are exercised: the
-//! Confluent variant gets the 5-byte schema-registry header prepended so its
-//! body decodes just as deeply. We keep the error-path coverage a random body
-//! gave, though: a quarter of the inputs feed the raw remaining bytes, and
-//! others truncate or single-byte-corrupt the valid encoding. Neither
-//! validation nor decoding may panic.
+//! structure, bounding recursion through cyclic message refs by omitting
+//! message fields entirely once the depth limit is reached. The wire format is
+//! fuzzer-chosen per input: the Confluent variant gets the six-byte
+//! schema-registry header prepended (magic byte, schema id, message index) so
+//! its body decodes just as deeply, and one input in eight instead lets the
+//! fuzzer pick those bytes so the header's rejection arms stay covered. We keep
+//! the error-path coverage a random body gave, though: a quarter of the inputs
+//! feed the raw remaining bytes, and others truncate or single-byte-corrupt the
+//! valid encoding. Neither validation nor decoding may panic.
//!
//! Beyond the well-formed-schema happy path, we drive the descriptor-validation
//! surface harder. Some schemas grow a `map` field (a repeated
@@ -157,7 +159,8 @@ fn gen_schema(u: &mut Unstructured) -> arbitrary::Result<(Vec, Vec)>
Ok((msgs, enum_nvals))
}
-fn field_proto(f: &FieldDef) -> FieldDescriptorProto {
+/// Build the descriptor entry for the `fi`th field of a message.
+fn field_proto(fi: usize, f: &FieldDef) -> FieldDescriptorProto {
let (ty, type_name) = match &f.ty {
FieldTy::Int32 => (Type::Int32, None),
FieldTy::Int64 => (Type::Int64, None),
@@ -186,7 +189,14 @@ fn field_proto(f: &FieldDef) -> FieldDescriptorProto {
Label::Optional
};
FieldDescriptorProto {
- name: Some(format!("f{}", f.number - 1)),
+ // NOTE: the name must come from the position, not from `f.number`. A
+ // name derived from the number makes every duplicate number a duplicate
+ // *name* too, and `DescriptorPool::decode` rejects the name clash first
+ // ("camel-case name of field 'f0' conflicts with field 'f0'"). The
+ // duplicate-number descriptor this target means to generate would then
+ // never exist, and the reject rate would swallow a large share of all
+ // inputs before any Materialize code runs.
+ name: Some(format!("f{fi}")),
number: Some(f.number),
label: Some(label as i32),
r#type: Some(ty as i32),
@@ -219,7 +229,12 @@ fn build_fds(msgs: &[MsgDef], enum_nvals: &[u8]) -> Vec {
.enumerate()
.map(|(mi, m)| DescriptorProto {
name: Some(format!("M{mi}")),
- field: m.fields.iter().map(field_proto).collect(),
+ field: m
+ .fields
+ .iter()
+ .enumerate()
+ .map(|(fi, f)| field_proto(fi, f))
+ .collect(),
..Default::default()
})
.collect();
@@ -286,6 +301,24 @@ fn encode_len_delimited(number: i32, bytes: &[u8], out: &mut Vec) {
out.extend_from_slice(bytes);
}
+/// Pick a value for a field of enum `E{j}`, whose declared values are
+/// `0..nvals`.
+///
+/// One draw in eight is `nvals` itself, which is out of range. prost decodes it
+/// fine (proto3 enums are open, the number survives as `Value::EnumNumber`),
+/// but `pack_value` then fails the row with "unknown enum value" and
+/// `pack_message` propagates that, so every field *after* the enum goes
+/// unpacked. Keeping it a minority leaves most bodies packing in full while
+/// still covering the rejection.
+fn enum_value(enum_nvals: &[u8], j: u8, u: &mut Unstructured) -> arbitrary::Result {
+ let nvals = u64::from(enum_nvals[usize::from(j)]);
+ if u.int_in_range(0u8..=7)? == 0 {
+ Ok(nvals)
+ } else {
+ Ok(u.int_in_range(0..=nvals - 1)?)
+ }
+}
+
/// Encode one occurrence of a field (tag + value).
fn encode_field(
msgs: &[MsgDef],
@@ -330,10 +363,7 @@ fn encode_field(
}
FieldTy::Enum(j) => {
encode_tag(n, 0, out);
- // Valid values are 0..nvals. `nvals` itself is one out-of-range
- // index (proto3 open enums accept it, the decoder maps it).
- let nvals = u64::from(enum_nvals[usize::from(*j)]);
- encode_varint(u.int_in_range(0u64..=nvals)?, out);
+ encode_varint(enum_value(enum_nvals, *j, u)?, out);
}
// wire type 1: 64-bit.
FieldTy::Fixed64 => {
@@ -379,12 +409,12 @@ fn encode_field(
encode_len_delimited(n, &b, out);
}
FieldTy::Message(r) => {
- // Bound recursion through cyclic refs: at the depth limit emit an
- // empty (all-defaults) nested message.
+ // Recursion through cyclic message refs terminates because
+ // `encode_message` gives a message field zero occurrences once
+ // `depth` reaches 0, so this arm only ever runs with `depth > 0`
+ // and `depth - 1` cannot underflow.
let mut nested = Vec::new();
- if depth > 0 {
- encode_message(msgs, enum_nvals, usize::from(*r), depth - 1, u, &mut nested)?;
- }
+ encode_message(msgs, enum_nvals, usize::from(*r), depth - 1, u, &mut nested)?;
encode_len_delimited(n, &nested, out);
}
FieldTy::Map => {
@@ -453,10 +483,7 @@ fn encode_scalar_value(
encode_varint(((v << 1) ^ (v >> 63)) as u64, out);
}
FieldTy::Bool => encode_varint(u.int_in_range(0u64..=1)?, out),
- FieldTy::Enum(j) => {
- let nvals = u64::from(enum_nvals[usize::from(*j)]);
- encode_varint(u.int_in_range(0u64..=nvals)?, out);
- }
+ FieldTy::Enum(j) => encode_varint(enum_value(enum_nvals, *j, u)?, out),
FieldTy::Fixed64 => out.extend_from_slice(&u.arbitrary::()?.to_le_bytes()),
FieldTy::Sfixed64 => out.extend_from_slice(&u.arbitrary::()?.to_le_bytes()),
FieldTy::Double => out.extend_from_slice(&u.arbitrary::()?.to_le_bytes()),
@@ -484,7 +511,8 @@ fn encode_message(
for f in &msg.fields {
// Repeated fields get 0..=3 occurrences, singular fields 0..=1
// (proto3 fields are optional). Message fields at the depth limit are
- // omitted entirely to break cycles.
+ // omitted entirely to break cycles. `encode_field` relies on that: it
+ // decrements `depth` unguarded for a message field.
let max = if matches!(f.ty, FieldTy::Message(_)) && depth == 0 {
0
} else if f.repeated {
@@ -522,6 +550,35 @@ fn run(mut u: Unstructured) -> arbitrary::Result<()> {
let root = usize::from(u.int_in_range(0u8..=u8::try_from(msgs.len() - 1).unwrap())?);
let root_name = format!("fuzz.M{root}");
+ // The wire format is chosen per input rather than both being run over the
+ // same body. Once `extract_protobuf_header` has stripped the prefix the two
+ // paths are byte-identical, so running both would just rebuild the whole
+ // descriptor pool (the most expensive step here) to repeat the same decode.
+ let confluent_wire_format = bool::arbitrary(&mut u)?;
+
+ // The Confluent protobuf prefix is magic byte 0, a 4-byte big-endian schema
+ // id, then a message-index array. `extract_protobuf_header` accepts only the
+ // single-byte `0` form (the file's first message), so a well-formed prefix
+ // is SIX zero bytes, not five. At five the body's own first byte stands in
+ // for the message index, and for a structured body that byte is always a
+ // field tag `(number << 3) | wire_type` with `number >= 1`, hence always
+ // `>= 8`, hence always rejected. The body would never once be decoded.
+ //
+ // One input in eight lets the fuzzer pick the prefix instead, so the header's
+ // rejection arms (bad magic, nonzero message index, buffer too short to hold
+ // magic + schema id, or to hold a message index after them) stay covered
+ // rather than depending on the accident of a body starting with a zero.
+ let confluent_prefix = if u.int_in_range(0u8..=7)? == 0 {
+ let len = u.int_in_range(0usize..=6)?;
+ let mut p = Vec::with_capacity(len);
+ for _ in 0..len {
+ p.push(u.arbitrary::()?);
+ }
+ p
+ } else {
+ vec![0u8; 6]
+ };
+
// Body for the root message: usually a valid encoding (so prost decodes
// through to the Row conversion), but a quarter of the time the raw remaining
// bytes, and otherwise a valid encoding occasionally truncated or single-byte
@@ -546,25 +603,20 @@ fn run(mut u: Unstructured) -> arbitrary::Result<()> {
b
};
- for confluent_wire_format in [false, true] {
- let Ok(descriptors) = DecodedDescriptors::from_bytes(&fds, root_name.clone()) else {
- return Ok(());
- };
- let Ok(mut decoder) = Decoder::new(descriptors, confluent_wire_format) else {
- return Ok(());
- };
- // The Confluent variant strips a 5-byte schema-registry header (magic
- // byte 0 + 4-byte schema id) before the body. Prepend one so its body
- // decodes just as deeply as the raw variant.
- let payload = if confluent_wire_format {
- let mut p = vec![0u8; 5];
- p.extend_from_slice(&body);
- p
- } else {
- body.clone()
- };
- let _ = decoder.decode(&payload);
- }
+ let Ok(descriptors) = DecodedDescriptors::from_bytes(&fds, root_name) else {
+ return Ok(());
+ };
+ let Ok(mut decoder) = Decoder::new(descriptors, confluent_wire_format) else {
+ return Ok(());
+ };
+ let payload = if confluent_wire_format {
+ let mut p = confluent_prefix;
+ p.extend_from_slice(&body);
+ p
+ } else {
+ body
+ };
+ let _ = decoder.decode(&payload);
Ok(())
}
diff --git a/src/mysql-util/fuzz/fuzz_targets/mysql_table_desc_proto_roundtrip.rs b/src/mysql-util/fuzz/fuzz_targets/mysql_table_desc_proto_roundtrip.rs
index c11cae3e28352..ee3f705201200 100644
--- a/src/mysql-util/fuzz/fuzz_targets/mysql_table_desc_proto_roundtrip.rs
+++ b/src/mysql-util/fuzz/fuzz_targets/mysql_table_desc_proto_roundtrip.rs
@@ -14,7 +14,7 @@
//! Input generation is split across three arms keyed off the first input
//! byte so a single byte stream exercises all of them over time:
//!
-//! 1. **Valid-value arm.** A 32-byte seed (drawn from the input) drives
+//! 1. **Valid-value arm.** A 32-byte seed (zero-padded from the input) drives
//! proptest's `Arbitrary for MySqlTableDesc` to build a *structurally
//! valid, deeply-populated* descriptor. Non-empty columns with real
//! `SqlColumnType`s, every `MySqlColumnMeta` variant, and a populated
@@ -24,11 +24,12 @@
//! decodes to near-empty messages).
//!
//! 2. **Duplicate/unsorted-keys arm.** Crafts a `ProtoMySqlTableDesc`
-//! whose `keys` field (a repeated proto `Vec`) contains duplicates and
-//! out-of-order entries to probe the classic `Vec -> BTreeSet -> Vec`
-//! round-trip trap: the *first* decode normalizes (dedups + sorts), so
-//! we assert the conversion is a *fixed point* afterwards rather than
-//! byte-identical to the crafted input.
+//! whose `keys` field (a repeated proto `Vec`) contains duplicates in
+//! non-sorted order, then asserts the two properties the
+//! `Vec -> BTreeSet` decode owes its callers: the decoded descriptor does
+//! not depend on the wire order, and the duplicates collapse. The
+//! conversion itself must succeed, because the crafted proto is valid by
+//! construction.
//!
//! 3. **Raw-bytes arm.** Decode arbitrary bytes and, if they happen to form a
//! valid descriptor, check the proto round-trip is stable. This guards
@@ -36,13 +37,32 @@
#![no_main]
+use std::sync::OnceLock;
+
use libfuzzer_sys::fuzz_target;
-use mz_mysql_util::{MySqlKeyDesc, MySqlTableDesc, ProtoMySqlKeyDesc, ProtoMySqlTableDesc};
+use mz_mysql_util::{MySqlTableDesc, ProtoMySqlKeyDesc, ProtoMySqlTableDesc};
use mz_proto::{ProtoType, RustType};
-use proptest::strategy::{Strategy, ValueTree};
+use proptest::strategy::{BoxedStrategy, Strategy, ValueTree};
use proptest::test_runner::{Config, RngAlgorithm, TestRng, TestRunner};
use prost::Message;
+// `Arbitrary::arbitrary()` rebuilds the entire boxed strategy graph on every
+// call: `SqlScalarType`'s ~31-variant `Union` plus a second copy of it for
+// `Array`, the `prop_recursive` wrapper, and a `.*` regex compile per
+// `any::()` leaf. `Config::default()` re-reads the process environment.
+// Both are per-process constants, so pay for them once instead of once per
+// execution. libFuzzer runs a single execution at a time per process, so a
+// `thread_local` suffices for the non-`Sync` strategy.
+thread_local! {
+ static DESC_STRATEGY: BoxedStrategy =
+ ::arbitrary().boxed();
+}
+
+fn config() -> Config {
+ static CONFIG: OnceLock = OnceLock::new();
+ CONFIG.get_or_init(Config::default).clone()
+}
+
/// Assert that a `MySqlTableDesc` survives a full Rust round-trip through
/// its proto representation unchanged, including a re-encode/decode of the
/// wire bytes.
@@ -54,13 +74,17 @@ fn assert_rust_roundtrip(orig: &MySqlTableDesc) {
let round: MySqlTableDesc = proto2
.into_rust()
.expect("re-encoded MySqlTableDesc must convert back to Rust");
- assert_eq!(orig, &round, "MySqlTableDesc changed across proto roundtrip");
+ assert_eq!(
+ orig, &round,
+ "MySqlTableDesc changed across proto roundtrip"
+ );
}
/// Decode `bytes` as a proto, and if it is a valid descriptor, assert the
-/// proto round-trip is stable. Used by both the crafted and raw-bytes arms,
-/// where the *first* decode may normalize a `Vec`-shaped field into a
-/// `BTreeSet`, so we only require idempotence from that normalized value on.
+/// proto round-trip is stable. Used by the raw-bytes arm, where a decode or
+/// conversion failure is a legitimate outcome, and where the *first* decode may
+/// normalize a `Vec`-shaped field into a `BTreeSet`, so we only require
+/// idempotence from that normalized value on.
fn check_decoded(bytes: &[u8]) {
let Ok(proto) = ProtoMySqlTableDesc::decode(bytes) else {
return;
@@ -101,63 +125,72 @@ fn craft_unsorted_dup_keys(data: &[u8]) -> ProtoMySqlTableDesc {
}
}
+/// Encode a hand-built proto, decode the wire bytes, and convert to Rust. Every
+/// step must succeed: the input is structurally valid by construction, so a
+/// failure is the bug the crafted arm exists to catch, for example a
+/// `from_proto` hardened to reject duplicate or unsorted wire keys.
+fn decode_crafted(proto: &ProtoMySqlTableDesc) -> MySqlTableDesc {
+ ProtoMySqlTableDesc::decode(proto.encode_to_vec().as_slice())
+ .expect("hand-built proto must decode")
+ .into_rust()
+ .expect("duplicate/unsorted keys must convert")
+}
+
fuzz_target!(|data: &[u8]| {
- // Reserve the first byte as a mode selector and the next 32 bytes as the
- // proptest seed. Everything after that feeds the raw-bytes / crafting
- // logic so a single input can drive any arm.
+ // The first byte selects the arm, everything after it is that arm's input.
+ // The arms overlap in the byte stream on purpose: only one of them runs per
+ // execution, so reading them all from byte 1 keeps every mutation live for
+ // whichever arm the mode byte picks. The proptest seed is zero-padded rather
+ // than all-or-nothing, otherwise every input shorter than 33 bytes would
+ // reuse the all-zero seed and libFuzzer grows inputs up from empty.
let mode = data.first().copied().unwrap_or(0);
+ let tail = data.get(1..).unwrap_or(&[]);
let mut seed = [0u8; 32];
- let seed_src = data.get(1..33).unwrap_or(&[]);
- seed[..seed_src.len()].copy_from_slice(seed_src);
- let rest = data.get(33..).unwrap_or(&[]);
+ let n = tail.len().min(32);
+ seed[..n].copy_from_slice(&tail[..n]);
match mode % 3 {
0 => {
// Valid-value arm: drive proptest's Arbitrary from the seed.
- let mut runner = TestRunner::new_with_rng(
- Config::default(),
- TestRng::from_seed(RngAlgorithm::ChaCha, &seed),
- );
- let value = match ::arbitrary()
- .new_tree(&mut runner)
- {
+ let mut runner =
+ TestRunner::new_with_rng(config(), TestRng::from_seed(RngAlgorithm::ChaCha, &seed));
+ let value = match DESC_STRATEGY.with(|s| s.new_tree(&mut runner)) {
Ok(tree) => tree.current(),
Err(_) => return,
};
assert_rust_roundtrip(&value);
-
- // The proptest-built value is already normalized (its keys come
- // from a BTreeSet). Sanity-check that the BTreeSet semantics hold
- // after a wire decode by encoding and re-decoding, then confirm
- // re-collecting the keys into a fresh set is idempotent.
- let decoded: MySqlTableDesc = value
- .into_proto()
- .encode_to_vec()
- .as_slice()
- .pipe(ProtoMySqlTableDesc::decode)
- .expect("decode")
- .into_rust()
- .expect("into_rust");
- let recollected: std::collections::BTreeSet =
- decoded.keys.iter().cloned().collect();
- assert_eq!(decoded.keys, recollected, "key set not idempotent");
}
1 => {
// Duplicate/unsorted-keys arm.
- let proto = craft_unsorted_dup_keys(rest);
- check_decoded(proto.encode_to_vec().as_slice());
+ let proto = craft_unsorted_dup_keys(tail);
+ let decoded = decode_crafted(&proto);
+ // The wire field is a repeated Vec and the Rust field a BTreeSet, so
+ // decoding must be insensitive to the wire order and must collapse
+ // the duplicates rather than keep them or reject the message.
+ //
+ // NOTE: rotate, don't reverse. `craft_unsorted_dup_keys` lays the
+ // keys out as a palindrome, so reversing them would re-encode the
+ // very same wire bytes and assert nothing.
+ let mut permuted = proto;
+ permuted.keys.rotate_left(1);
+ assert_eq!(
+ decoded,
+ decode_crafted(&permuted),
+ "wire key order changed the decoded descriptor"
+ );
+ // `craft_unsorted_dup_keys` builds exactly two distinct keys (their
+ // `columns` differ in length, so they can never compare equal),
+ // each repeated twice.
+ assert_eq!(
+ decoded.keys.len(),
+ 2,
+ "duplicate wire keys did not collapse"
+ );
+ assert_rust_roundtrip(&decoded);
}
_ => {
// Raw-bytes arm: decode arbitrary bytes directly.
- check_decoded(rest);
+ check_decoded(tail);
}
}
});
-
-/// Tiny extension trait so the valid-value arm can read top-to-bottom.
-trait Pipe: Sized {
- fn pipe(self, f: impl FnOnce(Self) -> R) -> R {
- f(self)
- }
-}
-impl Pipe for T {}
diff --git a/src/persist-client/fuzz/fuzz_targets/rollup_proto_roundtrip.rs b/src/persist-client/fuzz/fuzz_targets/rollup_proto_roundtrip.rs
index 2eb1b38d52892..45a8741c3bd81 100644
--- a/src/persist-client/fuzz/fuzz_targets/rollup_proto_roundtrip.rs
+++ b/src/persist-client/fuzz/fuzz_targets/rollup_proto_roundtrip.rs
@@ -10,7 +10,11 @@
//! Fuzz target: exercises `ProtoRollup` decoding and the `Rollup`
//! `from_proto` conversion. A rollup is a full state snapshot read from blob on
//! load, so a decoder panic on a corrupted/crafted blob makes the shard
-//! unrecoverable.
+//! unrecoverable. NOTE: an `Err` is not a *good* outcome on that path either.
+//! `UntypedState::decode` `.expect()`s the conversion, so a rejected rollup
+//! still aborts the process. What this target buys is a decoder that fails
+//! predictably and boundedly (no hang, no OOM, no wild slicing) plus a working
+//! error path for the CLI/inspect callers that do surface it.
//!
//! Decoding *random* bytes as a protobuf almost never yields a `ProtoRollup`
//! that survives `into_rust`: the conversion needs a well-formed shard id
@@ -21,15 +25,20 @@
//! `ProtoRollup` on the protobuf wire from fuzzer-chosen parameters. The first
//! byte selects a mode:
//!
-//! * mode 0: feed the remaining bytes straight to `ProtoRollup::decode`
-//! (the robustness arm: must never panic, and any value that
-//! converts must survive a proto re-encode round trip losslessly).
+//! * mode 0: feed the remaining bytes straight to `ProtoRollup::decode` (the
+//! robustness arm: must never panic, and any value that converts must be
+//! *stable* under a further round trip). Arbitrary bytes can be non-canonical,
+//! so this arm can only assert stability, not losslessness. Mode 1 covers that.
//! * mode 1: synthesize a valid rollup *with* inlined diffs whose bounds
-//! satisfy the invariants, decode it (the happy path that the invariant
-//! checks must accept), and round-trip it.
+//! satisfy the invariants, then require that it converts and that the
+//! conversion is *lossless*: the hand-built message is canonical, so
+//! re-encoding the converted value must reproduce it field for field.
//! * mode 2: synthesize a valid rollup, then *perturb a single invariant
//! field* (drop the rollups map, or shift `diffs.lower`/`diffs.upper` off
-//! the expected seqno). `from_proto` must reject it with `Err`, never panic.
+//! the expected seqno). `from_proto` must reject it with an `Err` that names
+//! the perturbed field, never panic. Matching the message matters: a bare
+//! `is_err()` would keep passing if the rollup started being rejected for
+//! some unrelated reason, hiding that the mutation stopped being tested.
#![no_main]
@@ -219,7 +228,11 @@ fn build_rollup(u: &mut Unstructured, mutate: Option) -> Vec {
} else {
None
};
- put_bytes(&mut buf, 16, &rollups_entry(*seqno, &hollow_rollup(&key, sz)));
+ put_bytes(
+ &mut buf,
+ 16,
+ &rollups_entry(*seqno, &hollow_rollup(&key, sz)),
+ );
}
}
@@ -257,9 +270,11 @@ fn build_rollup(u: &mut Unstructured, mutate: Option) -> Vec {
_ => {}
}
- // Pick in-range diff seqnos only for the un-mutated/drop-rollups cases.
- // `from_proto` does not bound-check individual diff seqnos, so this is
- // about producing realistic content rather than satisfying an invariant.
+ // Diff seqnos start at `lower`, which is the shifted one for
+ // `ShiftLower`, so the mutated cases can place them outside the true
+ // range. That is harmless: `from_proto` checks only the `lower`/`upper`
+ // bounds and never the individual diff seqnos, so this is about
+ // producing realistic content rather than satisfying an invariant.
let mut diffs: Vec<(u64, Vec)> = Vec::new();
if state_seqno > latest_rollup_seqno {
let span = upper.saturating_sub(lower).min(4);
@@ -275,16 +290,29 @@ fn build_rollup(u: &mut Unstructured, mutate: Option) -> Vec