Skip to content

fix(playground): safely proxy novel covers and eliminate string coercion hazard - #2814

Open
7ui77 wants to merge 4 commits into
lnreader:masterfrom
7ui77:fix/playground-cover-proxy
Open

7ui77 wants to merge 4 commits into
lnreader:masterfrom
7ui77:fix/playground-cover-proxy

Conversation

@7ui77

@7ui77 7ui77 commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Summary

 <NovelCard> (src/components/novel-card.tsx)
-  <img src={novel.cover || /static/coverNotAvailable.webp} />
+  <img src={getCoverUrl(novel.cover)} />

 <ParseNovelSection> (src/components/parse-novel.tsx)
-  <img src={(sourceNovel.cover ? / : ) + sourceNovel.cover || /static/coverNotAvailable.webp} />
+  <img src={getCoverUrl(sourceNovel.cover)} />

 src/lib/utils.ts
+  export const getCoverUrl = (cover?: string | null): string => {
+    if (!cover) return /static/coverNotAvailable.webp;
+    if (cover.startsWith(http://) || cover.startsWith(https://)) return `/${cover}`;
+    return cover;
+  };

Unifies playground cover image resolution through a defensive getCoverUrl utility:

  1. Proxies external novel covers through Vite dev proxy (/https:...), resolving 403 Forbidden / CORS anti-hotlink blocks in catalogue views.
  2. Eliminates string coercion trap ("" + undefined === "undefined") when novel covers are missing.

Evidence

  • Before:

    • novel-card.tsx requested raw cover URLs without proxying, causing broken images on popular/catalogue views due to CORS / anti-hotlinking protections.
    • parse-novel.tsx string concatenation evaluated undefined cover to "/undefined" instead of falling back to default asset.
    • Browser CDP evaluation on popular list:
      { "totalCards": 20, "loadedImages": 0, "naturalWidth": 0 }
  • After:

    • getCoverUrl safely routes HTTP/HTTPS covers through the local reverse proxy while cleanly falling back to /static/coverNotAvailable.webp on falsy values.
    • Browser CDP evaluation on FreeWebNovel popular list:
      { "totalCards": 20, "loadedImages": 20, "naturalWidth": 600, "naturalHeight": 800 }

Merge Danger

Door: two-way

Purely frontend dev playground UI helper. Reversible with zero impact on plugin scraper schemas or production runtime contracts.

Blast Radius: Localized

Scattered across playground cover renderings in novel-card.tsx and parse-novel.tsx. No production build or package distribution artifacts affected.

Checklist

  • Test changes in Plugin Playground or the app
  • Commit messages follow type(scope): description

@greptile-apps

greptile-apps Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 3/5

[Low impact] The change affects cover images in the playground’s novel browsing and parsing views.

Fix the HTTP route and preserve image bytes in alternate fetch modes before merging.

Findings

  1. P1 HTTP covers stop loading ▶
  2. P1 Alternate modes break covers ▶

Summary

Adds getCoverUrl and shares it between catalogue cards and novel details.

  • Novel cards and parse details route covers through one resolver.

Reviews (1) · Last reviewed commit: "fix(playground): extract getCoverUrl hel..." · Reviewed by Greptile

Comment thread src/lib/utils.ts
Comment on lines +45 to +46
if (cover.startsWith('http://') || cover.startsWith('https://')) {
return `/${cover}`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 HTTP covers stop loading

getCoverUrl turns an http:// cover into /http://..., but Vite only mounts the proxy at /https:. Popular and search cards now request a local path instead of the image URL, so valid HTTP covers that previously loaded directly stop displaying. Add HTTP proxy support or leave HTTP URLs unchanged.

<div className="relative mb-2 overflow-hidden rounded-lg bg-muted aspect-[3/4]">
<img
src={novel.cover || '/static/coverNotAvailable.webp'}
src={getCoverUrl(novel.cover)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Alternate modes break covers

Routing catalogue covers through the proxy breaks them when the user selects Node Fetch or Curl in Settings. proxy.ts sends Node Fetch responses through text() and Curl responses through a string, which changes JPEG, PNG, and WebP bytes. These cards previously loaded images directly. Preserve image bytes in both modes or use a separate image route before sending all covers through this proxy.

Knowledge Base Used: Plugin browser application

@7ui77

7ui77 commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

@rajarsheechatterjee can you check this pr?

@rajarsheechatterjee

Copy link
Copy Markdown
Member

Could you check if the comments are valid?

@7ui77

7ui77 commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

@rajarsheechatterjee Yes, both comments from Greptile were valid:

  1. HTTP covers route: vite.config.ts was only mounting /https:, so any http:// covers were returning 404/SPA HTML fallback.
  2. Alternate fetch modes breaking covers: NODE_FETCH was reading response bodies with await res2.text(), and CURL was executing with default string output, both of which corrupted binary image bytes (JPEG/PNG/WebP) when switched in Settings.

I've pushed follow-up commits addressing both issues:

  • Added /http: middleware in vite.config.ts and dynamic protocol/path extraction in proxy.ts.
  • Added support for protocol-relative cover URLs (//...) via getCoverUrl.
  • Preserved raw binary buffers using Buffer.from(await res2.arrayBuffer()) for NODE_FETCH and { encoding: 'buffer', maxBuffer: 50MB } for CURL.

This branch has not been deployed

No deployments
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