Skip to content

fix(opml): correct import progress math and saved-count reporting - #221

Merged
chhoumann merged 1 commit into
masterfrom
chhoumann/deepsec-opml-progress
Jun 29, 2026
Merged

chhoumann merged 1 commit into
masterfrom
chhoumann/deepsec-opml-progress

Conversation

@chhoumann

Copy link
Copy Markdown
Owner

Summary

Fixes two correctness bugs in OPML import (src/opml.ts), flagged by the deepsec finding other-logic-bug (922f3003f7).

1. Progress notice showed NaN%

When every feed in the imported OPML is already subscribed, newPodcastsToAdd.length is 0, so updateProgress computed (0 / 0) * 100 = NaN and (NaN).toFixed(1) rendered the string "NaN". Promise.all([]) resolves immediately, so the loop never re-ran progress and the user saw Importing... 0/0 podcasts completed (NaN%).

Fix: guard the zero denominator and show No new podcasts to import. instead.

2. Completion message over-reported the saved count

savedFeeds is keyed by title, and the save loop skips collisions (if (feeds[pod.title]) continue;). But the completion message reported validPodcasts.length (feeds fetched), so two imported feeds sharing a <title> (distinct URLs), or a new-URL feed whose title matches an existing saved feed, were silently dropped yet still counted as "saved".

Fix: count the feeds actually written (savedCount) and report duplicate-title drops on their own counter, so the summary reflects what was stored:

OPML import complete. Saved 1 new podcasts. Skipped 1 existing podcasts. Skipped 1 with duplicate titles.

Tests

src/opml.test.ts now captures the strings rendered into the progress Notice and asserts:

  • no NaN is ever shown when every feed is already subscribed
  • the summary reports the saved count, not the fetched count, on duplicate titles
  • a title-collision against an existing feed counts as not saved and preserves the original
  • URL-skipped and title-dropped feeds land in distinct counters

All four new tests fail against the old code.

Gates

lint, typecheck, build, and the full test suite (772+ tests) pass. Scoped to src/opml.ts and src/opml.test.ts only.

Note: do NOT merge - opened per the deepsec fix workflow.

When every feed in an imported OPML is already subscribed,
newPodcastsToAdd.length is 0, so updateProgress computed (0 / 0) * 100
= NaN and flashed "Importing... 0/0 podcasts completed (NaN%)". Guard
the zero denominator and show "No new podcasts to import." instead.

The save loop keys feeds by title and skips collisions, but the
completion message reported validPodcasts.length (the number fetched),
over-reporting "saved" when two imported feeds share a title or an
imported feed's title matches an existing saved feed. Count the feeds
actually written and report dropped duplicate-title collisions
separately so the summary reflects what was stored.

Resolves deepsec finding other-logic-bug (922f3003f7).
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying podnotes with  Cloudflare Pages  Cloudflare Pages

Latest commit: c274acc
Status: ✅  Deploy successful!
Preview URL: https://f23184be.podnotes.pages.dev
Branch Preview URL: https://chhoumann-deepsec-opml-progr.podnotes.pages.dev

View logs

Comment thread src/opml.test.ts
@chhoumann
chhoumann merged commit a79e529 into master Jun 29, 2026
3 checks passed
@chhoumann
chhoumann deleted the chhoumann/deepsec-opml-progress branch June 29, 2026 07:27
github-actions Bot pushed a commit that referenced this pull request Jul 9, 2026
## [2.17.3](2.17.2...2.17.3) (2026-07-09)

### Bug Fixes

* **feed/search:** parse feed once + content-based search cache ([#225](#225)) ([053d51f](053d51f)), closes [#149](#149)
* make episode identity key collision-resistant and prototype-safe ([#226](#226)) ([a5683db](a5683db))
* **opml:** correct import progress math and saved-count reporting ([#221](#221)) ([a79e529](a79e529))
* **security:** validate feed/URI URLs and cap download size ([#223](#223)) ([edef281](edef281))
* **template:** neutralize feed-controlled note injection ([#228](#228)) ([ef4ecbd](ef4ecbd))
* **timestamp:** escape live table-cell pipe after an escaped backslash ([#227](#227)) ([a34dfca](a34dfca))
* **transcription:** resolve three deepsec transcription-pipeline bugs ([#224](#224)) ([83c34e7](83c34e7))
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.17.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant