fix(helpers): relax URI decode fuzz invariant

[wizzomafizzo]

Summary

• Remove the broad DecodeURIIfNeeded idempotence fuzz assertion that flagged valid single-pass HTTP decoding behavior.
• Add regression coverage for the %25000 fuzz input and keep custom virtual path preservation focused on parse boundaries.
• Update fuzz testing guidance to use product-relevant invariants instead of decoder idempotence.

Fixes #735
Fixes #723

Summary by CodeRabbit

• Documentation

  • Clarified URI decoding comments to explain why percent-escapes remain encoded for parse-boundary preservation.

• Tests

  • Added coverage for double-encoded URI sequences.
  • Updated fuzz test invariants and assertions to remove idempotence checks and strengthen scheme-handling validation.
  • Adjusted unit test expectations and descriptions for encoding edge cases.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 20a2b059-f1e9-419a-b443-2f12b7164970

📥 Commits

Reviewing files that changed from the base of the PR and between 848f791 and 89f4aa7.

📒 Files selected for processing (1)

• pkg/helpers/uris_fuzz_test.go

---

📝 Walkthrough

Walkthrough

Updates to URI decoding comments, unit tests, and fuzz tests: idempotence assertions removed; wording changed to emphasize parse-boundary/preserved percent-escapes and scheme-handling; new double-encoded percent test case added to fuzz corpus and unit tests.

Changes

Cohort / File(s)	Summary
Comments pkg/helpers/uris.go	Reworded header and inline comments to explain preservation of percent-escapes for parse-boundary reasons instead of describing idempotence; no executable changes.
Fuzz tests & docs pkg/helpers/uris_fuzz_test.go, pkg/testing/FUZZ_TESTING.md	Removed idempotence re-decode assertion; added http:///%25000 to corpus; replaced invariant with scheme-handling checks and adjusted error reporting.
Unit tests pkg/helpers/uris_test.go	Added HTTP case for double-encoded percent (%25... → single-percent escape in output); updated failure-case descriptions to reflect preservation of encoded % for custom path structure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• #721: Implements code changes to preserve custom-scheme percent-escapes referenced by the updated tests and comments.
• #714: Overlaps on DecodeURIIfNeeded behavior for literal % and double-encoded sequences; related test/corpus adjustments.
• #708: Prior changes addressing idempotence and raw-fragment extraction that motivated removal of idempotence assertions here.

Poem

🐇 I nibble bytes where percent signs play,

I keep their shapes so paths can stay,
Fuzz seeds tossed, a curious sight,
Tests now sing: preserve, don't fight.
Hop, hop—two encodings, one bright light.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(helpers): relax URI decode fuzz invariant' accurately describes the main change—relaxing the idempotence assertion in the URI decode fuzz test.
Linked Issues check	✅ Passed	The PR removes a broad idempotence fuzz assertion and updates invariants to address crashes reported in #735 and #723, aligning with the objective of relaxing problematic fuzz test constraints.
Out of Scope Changes check	✅ Passed	All changes are scoped to fuzz testing improvements and documentation updates for the URI decoder, directly addressing the linked issues without unrelated modifications.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/decode-uri-fuzz-invariant

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sentry]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[coderabbitai]

🧹 Nitpick comments (1)

pkg/helpers/uris_fuzz_test.go (1)

74-88: Strengthen Property 4 to assert unknown schemes are unchanged.

Right now it only verifies scheme identity, so payload/path mutation could slip through for unsupported schemes. Adding an explicit unknown-scheme equality check would align this fuzz target with the updated fuzz guidance.

Suggested invariant tightening

-		// Property 4: Should preserve scheme if present and valid
+		// Property 4: Unknown schemes should remain unchanged
+		if strings.HasPrefix(uri, "example://") && result != uri {
+			t.Errorf("Unknown scheme changed: %q -> %q", uri, result)
+		}
+
+		// Property 5: Should preserve scheme if present and valid
 		if strings.Contains(uri, "://") && !virtualpath.ContainsControlChar(uri) {
 			schemeEnd := strings.Index(uri, "://")
 			schemeEnd2 := strings.Index(result, "://")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/helpers/uris_fuzz_test.go` around lines 74 - 88, The test currently only
asserts that valid schemes are preserved; extend Property 4 in
pkg/helpers/uris_fuzz_test.go to also assert that when origScheme is not valid
(virtualpath.IsValidScheme(origScheme) == false) the entire scheme substring
(origScheme) and the following "://" are preserved exactly in result (i.e.,
origScheme == resultScheme or equivalently the prefix up to schemeEnd2 matches),
and fail the fuzz case with t.Errorf if they differ; locate the existing block
using variables uri, result, schemeEnd, schemeEnd2, origScheme and resultScheme
and add the additional branch that enforces equality for unknown/unsupported
schemes.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/helpers/uris_fuzz_test.go`:
- Around line 74-88: The test currently only asserts that valid schemes are
preserved; extend Property 4 in pkg/helpers/uris_fuzz_test.go to also assert
that when origScheme is not valid (virtualpath.IsValidScheme(origScheme) ==
false) the entire scheme substring (origScheme) and the following "://" are
preserved exactly in result (i.e., origScheme == resultScheme or equivalently
the prefix up to schemeEnd2 matches), and fail the fuzz case with t.Errorf if
they differ; locate the existing block using variables uri, result, schemeEnd,
schemeEnd2, origScheme and resultScheme and add the additional branch that
enforces equality for unknown/unsupported schemes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1605636e-834a-4ab3-aa8c-7d38545527c5

📥 Commits

Reviewing files that changed from the base of the PR and between 1dd6fec and 848f791.

📒 Files selected for processing (4)

• pkg/helpers/uris.go
• pkg/helpers/uris_fuzz_test.go
• pkg/helpers/uris_test.go
• pkg/testing/FUZZ_TESTING.md