Skip to content

fix: escape Content-Disposition filenames and guard malformed align responses - #10

Merged
carochacs merged 2 commits into
mainfrom
claude/next-issues-f1t8df
Aug 19, 2026
Merged

fix: escape Content-Disposition filenames and guard malformed align responses#10
carochacs merged 2 commits into
mainfrom
claude/next-issues-f1t8df

Conversation

@carochacs

Copy link
Copy Markdown
Collaborator

What

Two small robustness/security fixes found during a prior security audit:

  • ls_export: title/artist were inserted into Content-Disposition: filename="{safe_name}.lrc" without escaping ", so a quote in title/artist could break out of the quoted filename parameter and corrupt the download filename. Now escapes "/\ for the legacy filename= fallback and adds an RFC 5987 filename*=UTF-8''... form.
  • ls_align: resp.json() wasn't wrapped in a try/except, so a non-JSON response from the configured align server produced an unhandled 500. Now returns a clean 502 on a malformed response.

Fixes #7, Fixes #8

Checklist

  • Verified routes.py parses (python3 -c "import ast; ast.parse(...)")
  • Bumped plugin.json version (1.0.1 → 1.0.2)

Generated by Claude Code

…esponses

- ls_export now quote-escapes title/artist before embedding them in the
  Content-Disposition filename param, and adds an RFC 5987 filename*=
  fallback, so an embedded quote can't corrupt/break out of the header.
- ls_align now wraps resp.json() in a try/except so a non-JSON response
  from the configured align server returns a clean 502 instead of an
  unhandled 500.

Fixes #7, #8

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Minor suggestions only — both fixes are sound.

Reviewed changes

  • ls_align malformed-response guard (routes.py:223-229): wraps resp.json() in a try/except ValueError, returning a clean 502 instead of an unhandled 500 when the alignment server returns non-JSON. Correct exception to catch (JSONDecodeError inherits from ValueError in requests).
  • ls_export Content-Disposition escaping (routes.py:270-283): escapes " and \ in the legacy filename= parameter, and adds an RFC 5987 filename*=UTF-8''... form with urllib.parse.quote for non-ASCII names. Standard and correct approach.

Pullfrog  | View workflow run | Using Big Pickle (free) | 𝕏

Comment thread routes.py
pullfrog review on PR #10 flagged that the \\ -> \\\\ replacement in
ascii_name is a no-op today (safe_name already stripped \\ above) —
harmless but confusing without context. Added a one-line comment
explaining it's a belt-and-suspenders guard for if that stripping
ever changes, rather than dead code.
@get-flashbacks get-flashbacks deleted a comment from coderabbitai Bot Aug 19, 2026
@carochacs
carochacs merged commit 8165c99 into main Aug 19, 2026
4 checks passed
@carochacs
carochacs deleted the claude/next-issues-f1t8df branch August 19, 2026 04:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security][Low] Unhandled exception on malformed align-server response [Security][Low] Content-Disposition filename not quote-escaped in ls_export

2 participants