fix(helpers): fix DecodeURIIfNeeded idempotence for '?' and '#' in URIs

[wizzomafizzo]

Fixes #690 and #706 — nightly fuzz crashes in FuzzDecodeURIIfNeeded.

Both inputs violated the idempotence property (decode(decode(x)) == decode(x)).

Custom-scheme branch (steAm://%2F%3F): %3F decoded to a literal ?, which was then parsed as a query separator on the second pass. Fix: extend the segment re-encoder to escape ? → %3F and # → %23 in addition to /.

http/https branch (http://?#?#%): Per RFC 3986, # introduces the fragment and takes precedence over ?. The old code extracted the fragment from the query string after the fact, so a fragment containing ? was misread as a query separator on the second parse. Fix: extract fragment from the raw URI (split on first #) before calling ParseURIComponents.

Regression corpus files from both issues are included.

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of special characters in custom scheme URIs (/, ?, #) to prevent unintended URI structure alterations.
  • Enhanced fragment and query parameter parsing in standard web schemes for better idempotency.

• Tests

  • Added fuzz test cases for URI decoding edge cases.
  • Updated test expectations for custom scheme URI handling.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR fixes a crash discovered in nightly fuzz tests by enhancing URI decoding logic to properly re-encode special delimiter characters (#, ?, /) when processing custom Zaparoo schemes and standard web schemes, and adds two crash-reproducing test cases to the fuzz corpus.

Changes

Cohort / File(s)	Summary
Fuzz Test Corpus pkg/helpers/testdata/fuzz/FuzzDecodeURIIfNeeded/b1dd9e28e8156f0d, pkg/helpers/testdata/fuzz/FuzzDecodeURIIfNeeded/ecaa49a609591b96	Added two new fuzz test input files capturing crash cases: one with custom scheme containing percent-encoded delimiters (steAm://%2F%3F) and one with standard http scheme containing fragment and query delimiters (http://?#?#%).
URI Decoding Logic pkg/helpers/uris.go	Enhanced DecodeURIIfNeeded to re-encode gen-delims (/, ?, #) using strings.NewReplacer for custom schemes instead of only re-encoding /. For standard schemes, refactored fragment/query extraction to use raw input and first # occurrence rather than deriving from parsed query, ensuring idempotent behavior.
Test Expectations pkg/helpers/uris_test.go	Updated TestDecodeURIIfNeeded_EdgeCases kodi scheme test case to expect # re-encoded as %23 in output (kodi-movie://456/The Matrix%23play).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• #681: Both PRs modify DecodeURIIfNeeded logic and related fuzz corpus files to address URI decoding behavior and prevent regressions from nightly fuzz runs.

Poem

🐰 A fuzz-fuzzy tale of URIs most wild,
With # and ? schemes running unreconciled,
Now reencoded just right, those delims take flight,
The corpus grows wise with each crash in the night! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: fixing idempotence in DecodeURIIfNeeded for '?' and '#' characters, which directly addresses the core issue being fixed.
Linked Issues check	✅ Passed	The PR successfully addresses issue #690 by adding regression corpus files (FuzzDecodeURIIfNeeded test cases) and fixing DecodeURIIfNeeded idempotence for '?' and '#' in both custom-scheme and http/https branches.
Out of Scope Changes check	✅ Passed	All changes are directly in scope: two fuzz test input files added for regression testing, uris.go modified to fix the core idempotence issue, and uris_test.go updated to reflect corrected expected behavior.

_{✏️ 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/fuzz-decode-uri-idempotence

---

_{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_test.go (1)

339-420: Add an explicit table case for http://?#?#% in this unit test block.

The fuzz seed is great for reproduction, but pinning the expected decoded string here would make CI regressions easier to diagnose without fuzz context.

As per coding guidelines, "Write tests for all new code — use testify/mock, sqlmock, afero, and patterns from pkg/testing/".

🤖 Prompt for AI Agents

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

In `@pkg/helpers/uris_test.go` around lines 339 - 420, Add a new table entry to
the tests slice inside TestDecodeURIIfNeeded_EdgeCases for the fuzz seed input
"http://?#?#%" so CI pins the expected output; e.g. add { name:
"http_fuzz_seed_question_marks", input: "http://?#?#%", expected: "http://?#?#%"
} alongside the other cases in the tests variable used by
TestDecodeURIIfNeeded_EdgeCases to ensure the decoder's behavior for that
edge-case is explicitly asserted.

🤖 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_test.go`:
- Around line 339-420: Add a new table entry to the tests slice inside
TestDecodeURIIfNeeded_EdgeCases for the fuzz seed input "http://?#?#%" so CI
pins the expected output; e.g. add { name: "http_fuzz_seed_question_marks",
input: "http://?#?#%", expected: "http://?#?#%" } alongside the other cases in
the tests variable used by TestDecodeURIIfNeeded_EdgeCases to ensure the
decoder's behavior for that edge-case is explicitly asserted.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 91356531-9422-4b28-b2bb-340c9db3edf2

📥 Commits

Reviewing files that changed from the base of the PR and between d808581 and 3375c62.

📒 Files selected for processing (4)

• pkg/helpers/testdata/fuzz/FuzzDecodeURIIfNeeded/b1dd9e28e8156f0d
• pkg/helpers/testdata/fuzz/FuzzDecodeURIIfNeeded/ecaa49a609591b96
• pkg/helpers/uris.go
• pkg/helpers/uris_test.go