fix: decode URL-encoded Slack audio filenames#118
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 56 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe changes add URL decoding functionality to the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/content/selectAttachedAudioClip.ts`:
- Line 35: The line computing songFilename in selectAttachedAudioClip uses
decodeURIComponent on a user-controlled urlPath which can throw on malformed
percent-encoding; wrap the decodeURIComponent call in a try/catch and on
URIErrors (or any exceptions) fall back to the raw filename (the result of
urlPath.split("/").pop() or "attached-audio.mp3") so the function doesn't throw
for invalid percent-encoding in audioUrl.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb768f30-caba-4966-bf96-28c0cd316ba0
📒 Files selected for processing (2)
src/content/__tests__/selectAttachedAudioClip.test.tssrc/content/selectAttachedAudioClip.ts
| // Derive filename from URL | ||
| const urlPath = new URL(audioUrl).pathname; | ||
| const songFilename = urlPath.split("/").pop() ?? "attached-audio.mp3"; | ||
| const songFilename = decodeURIComponent(urlPath.split("/").pop() ?? "attached-audio.mp3"); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does JavaScript decodeURIComponentthrow aURIErroron malformed percent-encoded input (for example%ZZ), and what is the recommended defensive handling pattern?
💡 Result:
Yes. decodeURIComponent must throw a URIError when it encounters malformed percent-encoding—e.g. a % not followed by two hex digits (so %ZZ, %, %A, etc.), or when the decoded bytes are not valid UTF‑8. [1], [2]
Defensive handling pattern (recommended): wrap decoding in try/catch, and treat URIError as “bad input” (return a fallback, or surface a validation error), while rethrowing other exceptions.
export function safeDecodeURIComponent(s, fallback = s) {
try {
return decodeURIComponent(s);
} catch (e) {
if (e instanceof URIError) return fallback; // or: return null / throw new Error("Bad encoding")
throw e;
}
}Sources: MDN decodeURIComponent() (exceptions) [1]; ECMAScript spec “Decode” operation (throws URIError on non-hex escapes / invalid UTF‑8) [2].
[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/decodeURIComponent
[2] https://www.ecma-international.org/wp-content/uploads/ECMA-262_5th_edition_december_2009.pdf
🏁 Script executed:
cat -n src/content/selectAttachedAudioClip.tsRepository: recoupable/tasks
Length of output: 3500
Guard decodeURIComponent against malformed filenames.
The audioUrl parameter accepts user-provided URLs. If a URL's pathname contains invalid percent-encoding (e.g., %ZZ or incomplete escapes), decodeURIComponent throws a URIError and halts execution. Wrap the call in a try/catch block and fall back to the raw filename on encoding errors.
Suggested patch
- const songFilename = decodeURIComponent(urlPath.split("/").pop() ?? "attached-audio.mp3");
+ const rawFilename = urlPath.split("/").pop() ?? "attached-audio.mp3";
+ let songFilename = rawFilename;
+ try {
+ songFilename = decodeURIComponent(rawFilename);
+ } catch {
+ // Keep raw filename if encoding is malformed
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const songFilename = decodeURIComponent(urlPath.split("/").pop() ?? "attached-audio.mp3"); | |
| const rawFilename = urlPath.split("/").pop() ?? "attached-audio.mp3"; | |
| let songFilename = rawFilename; | |
| try { | |
| songFilename = decodeURIComponent(rawFilename); | |
| } catch { | |
| // Keep raw filename if encoding is malformed | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/content/selectAttachedAudioClip.ts` at line 35, The line computing
songFilename in selectAttachedAudioClip uses decodeURIComponent on a
user-controlled urlPath which can throw on malformed percent-encoding; wrap the
decodeURIComponent call in a try/catch and on URIErrors (or any exceptions) fall
back to the raw filename (the result of urlPath.split("/").pop() or
"attached-audio.mp3") so the function doesn't throw for invalid percent-encoding
in audioUrl.
sweetmantech
force-pushed
the
fix/decode-audio-filename
branch
from
542b100 to
672efc7
Compare
…Song - Decode URI components and replace spaces with hyphens in audio filenames - Add detailed logging around fal.storage.upload and fal-ai/whisper calls to diagnose Bad Request errors (logs filename, size, audioUrl, error) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sweetmantech
force-pushed
the
fix/decode-audio-filename
branch
from
672efc7 to
43a13d5
Compare
1 participant
Summary
%20-encoded spaces in filenamesApiError: Bad RequestdecodeURIComponent()to decode the filename before passing to transcriptionContext
Triggered by prod error:
run_cmnf0e48g9zqw0on6kv3z4iw4— fileSingin%20in%20the%20Rain%202.mp3Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit