Skip to content

Normalize media URLs and fix double-encoded ampersands - #71

Draft
EngDawood wants to merge 1 commit into
mainfrom
claude/ecstatic-franklin-7b9zv7
Draft

EngDawood wants to merge 1 commit into
mainfrom
claude/ecstatic-franklin-7b9zv7

Conversation

@EngDawood

Copy link
Copy Markdown
Owner

Summary

Refactors media URL cleaning logic into a reusable cleanMediaUrl() function that handles both whitespace normalization and double-encoded ampersand decoding. This fixes an issue where some RSS bridges (like ImgsedBridge) double-encode ampersands in query strings, causing CDN 403 errors when the signed URLs are corrupted.

Key Changes

  • Extract cleanMediaUrl() helper function — Centralizes URL normalization logic that was previously duplicated across 9 call sites. The function:

    • Strips whitespace from URLs
    • Iteratively decodes double-encoded ampersands (&, &, & → &)
    • Handles undefined gracefully
  • Replace inline .replace(/\s+/g, '') calls — Updates all 9 occurrences in extractMedia() and extractMediaFromRSS() to use the new helper, improving maintainability and fixing the ampersand decoding issue

  • Add test coverage — New test case validates that double-encoded ampersands in Atom feed enclosure URLs are properly decoded to restore valid signed CDN URLs

  • Preserve fallback error diagnostics — Updates queue-handler.ts to log the original error message when a fallback post succeeds, enabling better troubleshooting of why the primary send failed

Implementation Details

The cleanMediaUrl() function uses a loop to handle nested encoding (e.g., & → & → &), ensuring all variants of double-encoded ampersands are resolved regardless of how many layers of encoding were applied by upstream bridges.

https://claude.ai/code/session_013A1FKXZw6wmBhXyLkn3ZKC

Some bridges emit media URLs as `&`, which reached us as a literal
`&`. The `#` turned the signed CDN query string into a fragment, so
Instagram returned 403, both URL and download sends failed, and posts went
out as the text + "View original post" fallback instead of the media.

Also record the original send error on successful fallback post_log rows
so future fallbacks are diagnosable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013A1FKXZw6wmBhXyLkn3ZKC
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
rss-bridge 829bba7 Sep 22 2026, 08:24 PM

Copy link
Copy Markdown
Owner Author

CI test is red, but not because of this PR.

The job fails at pnpm lint (eslint app) with 2 errors in app/src/components/tabs/BundlesTab.tsx, a file this PR does not touch:

  • 50:21 react-hooks/set-state-in-effect (useEffect(() => { load(); }, [load]))
  • 113:7 @typescript-eslint/no-unused-expressions (isIn ? next.delete(feedId) : next.add(feedId))

main fails with the same errors (run 35669406765), and CI on main has been red for at least 5 runs. No fix PR exists yet, so I did not widen this PR.

Proposed patch, verified locally (pnpm lint exits 0, tsc -p app/tsconfig.app.json clean):

--- a/app/src/components/tabs/BundlesTab.tsx
+++ b/app/src/components/tabs/BundlesTab.tsx
@@ -47,7 +47,16 @@
-  useEffect(() => { load(); }, [load]);
+  // Initial fetch: set state only in the async callback (react-hooks/set-state-in-effect)
+  useEffect(() => {
+    let cancelled = false;
+    callApi('list_bundles').then(res => {
+      if (cancelled) return;
+      if (!res.error) setBundles(res.data || []);
+      setLoading(false);
+    });
+    return () => { cancelled = true; };
+  }, [callApi]);
@@ -110,7 +119,7 @@
-      isIn ? next.delete(feedId) : next.add(feedId);
+      if (isIn) next.delete(feedId); else next.add(feedId);

Note: lint stops the job before the pnpm test step. Locally, the workers vitest pool needs a Cloudflare login because of the remote ai binding, so the test step may fail in CI next.


Generated by Claude Code

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.

2 participants