Skip to content

refactor(ffmpeg): resolve per-domain request headers from a registry - #168

Merged
Rushaway merged 2 commits into
masterfrom
refactor/156-per-domain-header-profiles
Sep 6, 2026
Merged

refactor(ffmpeg): resolve per-domain request headers from a registry#168
Rushaway merged 2 commits into
masterfrom
refactor/156-per-domain-header-profiles

Conversation

@Rushaway

@Rushaway Rushaway commented Sep 5, 2026

Copy link
Copy Markdown
Member

Summary

  • The MyInstants-specific Referer/Sec-Fetch-* headers were applied by if needs_cf_bypass and "myinstants.com" in uri: buried in the generic streaming path. Every additional Cloudflare-protected host would have meant another in uri special case in shared transport code.
  • Replaced it with a DOMAIN_HEADER_PROFILES registry keyed by domain plus a get_domain_headers() lookup, so adding a host is a data entry at the top of the file rather than a branch in the fetch path.

Deliberate behavior change worth reviewing

The lookup now matches the URI's hostname (exact or subdomain) instead of substring-matching the whole URI. The old check also fired for URLs that merely contained the string:

URL old new
https://www.myinstants.com/media/sounds/x.mp3
https://myinstants.com/media/sounds/y.mp3
https://cdn.myinstants.com/sounds/z.mp3
https://evil.com/redirect?to=myinstants.com/x.mp3
https://notmyinstants.com/x.mp3

The last two mean the old code would send Referer: https://www.myinstants.com to an unrelated (potentially attacker-chosen) host. I treated tightening that as part of the fix rather than faithfully preserving it — say the word if you'd rather keep substring semantics.

Closes #156

Test plan

  • Compared old vs new predicate across 7 URL shapes (apex, www, cdn subdomain, unrelated hosts, and the two spoofing cases above) — real MyInstants URLs all still match.
  • Confirmed the resolved header set is byte-identical to the previous hardcoded block.

Note: this and #150 both touch the same region of FFmpegAudioPlayer, and #167 changes the get_cf_session call just above it — whichever lands later will need a rebase.

🤖 Generated with Claude Code

The MyInstants-specific Referer/Sec-Fetch-* headers were applied by an
`if needs_cf_bypass and "myinstants.com" in uri` check buried in the
generic streaming path, so every additional host would have meant another
special case in shared transport code.

Replaced it with a DOMAIN_HEADER_PROFILES registry keyed by domain and a
get_domain_headers() lookup, so a new host is a data entry.

The lookup matches the URI's hostname (exact or subdomain) instead of
substring-matching the whole URI. That is a deliberate behavior change:
the old check also fired for URLs that merely contained the string, such
as https://evil.com/redirect?to=myinstants.com/x.mp3 or
https://notmyinstants.com/x.mp3, leaking a myinstants.com Referer to
unrelated hosts. Real MyInstants URLs (apex, www and cdn subdomains) still
match.

Bumps VERSION to 1.8.21.

Closes #156

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Dolly132 <109222243+Dolly132@users.noreply.github.com>
@Rushaway
Rushaway requested a review from Dolly132 September 5, 2026 13:23

@Rushaway Rushaway left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Self-review: flagging the one non-mechanical bit -- I switched from substring matching the whole URI to hostname matching, which intentionally stops sending the myinstants.com Referer to hosts that merely have the string in their path/query. I think that's a fix rather than a regression, but it is a behavior change so it should be a conscious call, not something you discover later.

@Dolly132 Dolly132 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I mean, this is probably useful once a lifetime, but it's worth merging regardless.

@Rushaway
Rushaway merged commit 9a83ced into master Sep 6, 2026
2 checks passed
@Rushaway
Rushaway deleted the refactor/156-per-domain-header-profiles branch September 6, 2026 10:11
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.

FFmpegAudioPlayer hardcodes MyInstants-specific headers inside the generic streaming path

2 participants