From 413827a6a7115c8d9d9a1c1b156067d9c5923327 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Mon, 12 Oct 2026 19:51:25 +0200 Subject: [PATCH 1/2] test: reproduce unsafe name lookups and ambiguous moved-file links --- src/URIHandler.test.ts | 261 +++++++++++++++++- src/persistence/merge.test.ts | 110 ++++++++ src/persistence/podNotesData.test.ts | 94 ++++++- src/services/privateFeeds.test.ts | 38 +++ src/store/index.test.ts | 91 +++++- .../PodNotesSettingsTab.import.test.ts | 49 ++++ src/utility/episodeListEntry.test.ts | 35 +++ src/utility/episodeStatus.test.ts | 28 ++ tests/e2e/journey-feeds.test.ts | 44 +++ tests/e2e/journey-links-local.test.ts | 45 +++ 10 files changed, 782 insertions(+), 13 deletions(-) diff --git a/src/URIHandler.test.ts b/src/URIHandler.test.ts index 89c3f65a..38047ea9 100644 --- a/src/URIHandler.test.ts +++ b/src/URIHandler.test.ts @@ -1,5 +1,5 @@ import * as obsidian from "obsidian"; -import { TFile, type ObsidianProtocolData } from "obsidian"; +import { TFile, TFolder, type ObsidianProtocolData } from "obsidian"; import { get } from "svelte/store"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; @@ -50,6 +50,7 @@ const emptyLocalFiles: Playlist = { let doc: FakeDocument; let player: PlayerController; let vaultFiles: string[] = []; +let vaultFolders: string[] = []; const state = () => get(player.state); const media = () => doc.media; @@ -88,6 +89,7 @@ function vaultFile(path: string): TFile { const name = path.split("/").pop() ?? path; return Object.assign(new TFile(), { path, + name, extension: name.split(".").pop() ?? "", basename: name.replace(/\.[^.]+$/, ""), stat: { ctime: 0, mtime: 0, size: 1 }, @@ -109,15 +111,19 @@ function loadedAt(episode: Episode, time: number) { beforeEach(() => { vaultFiles = []; + vaultFolders = []; doc = new FakeDocument(); const host = { settings: structuredClone(DEFAULT_SETTINGS), app: { vault: { - getAbstractFileByPath: vi.fn((path: string) => - vaultFiles.includes(path) ? vaultFile(path) : null, - ), + getAbstractFileByPath: vi.fn((path: string) => { + if (vaultFiles.includes(path)) return vaultFile(path); + if (vaultFolders.includes(path)) return Object.assign(new TFolder(), { path }); + return null; + }), getResourcePath: (file: TFile) => `app://resource/${file.path}`, + getFiles: () => vaultFiles.map(vaultFile), }, fileManager: { generateMarkdownLink: (file: TFile) => `[[${file.path}]]` }, }, @@ -305,11 +311,11 @@ describe("podNotesURIHandler", () => { expect(mockEpisodesAt).not.toHaveBeenCalled(); }); - test("resolves a moved/renamed local file by name instead of routing to the feed parser (LF-08)", async () => { + test("plays a moved local file from where it is now instead of routing to the feed parser (LF-08)", async () => { const localEpisode: LocalEpisode = { title: "Local Note", - streamUrl: "Old/Local Note.mp3", - url: "Old/Local Note.mp3", + streamUrl: "app://resource/Old/Local Note.mp3", + url: "[[Old/Local Note.mp3]]", description: "", content: "", podcastName: "local file", @@ -318,10 +324,17 @@ describe("podNotesURIHandler", () => { localFiles.set({ ...emptyLocalFiles, episodes: [localEpisode] }); // The file was moved: the recorded path no longer resolves to a file, and // the url has no http(s) scheme, so it must be treated as a vault path. + vaultFiles = ["New/Local Note.mp3"]; await link({ url: "Old/Local Note.mp3", episodeName: "Local Note", time: "15" }); + metadata(); - expect(state()).toMatchObject({ episode: { title: "Local Note" }, currentTime: 15 }); + expect(state().episode).toMatchObject({ + title: "Local Note", + filePath: "New/Local Note.mp3", + }); + expect(media().src).toBe("app://resource/New/Local Note.mp3"); + expect(media().currentTime).toBe(15); expect(get(viewState)).toBe(ViewState.Player); // A schemeless path must never hit the feed parser. expect(mockEpisodesAt).not.toHaveBeenCalled(); @@ -725,6 +738,238 @@ describe("a timestamp link PodNotes wrote for the loaded episode", () => { }); }); +describe("a link to a vault file that is no longer at its path", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + test("plays the one vault file with that name, even when another file is registered for it", async () => { + localFiles.set({ ...emptyLocalFiles, episodes: [clip("B")] }); + vaultFiles = ["C/Clip.mp3"]; + + await link({ url: "A/Clip.mp3", episodeName: "Clip", time: "12" }); + metadata(); + + expect(media().src).toBe("app://resource/C/Clip.mp3"); + expect(media().currentTime).toBe(12); + }); + + test("shows a notice and plays nothing when several vault files have that name", async () => { + const notice = vi.spyOn(obsidian, "Notice"); + localFiles.set({ ...emptyLocalFiles, episodes: [clip("B")] }); + vaultFiles = ["B/Clip.mp3", "C/Clip.mp3"]; + + await link({ url: "A/Clip.mp3", episodeName: "Clip", time: "12" }); + + expect(notice).toHaveBeenCalledWith( + 'More than one vault file is named "Clip.mp3". Update the link with the correct path.', + ); + expect(state().episode).toBeUndefined(); + expect(media().src).toBe(""); + expect(get(viewState)).toBe(ViewState.PodcastGrid); + }); + + test("shows a notice and plays nothing when no vault file has that name", async () => { + const notice = vi.spyOn(obsidian, "Notice"); + localFiles.set({ ...emptyLocalFiles, episodes: [clip("A")] }); + vaultFiles = ["B/Other.mp3"]; + + await link({ url: "A/Clip.mp3", episodeName: "Clip", time: "12" }); + + expect(notice).toHaveBeenCalledWith('No vault file is named "Clip.mp3".'); + expect(state().episode).toBeUndefined(); + expect(media().src).toBe(""); + expect(mockEpisodesAt).not.toHaveBeenCalled(); + }); + + test("counts files named like the link's literal '+' before its legacy '+'-as-space form", async () => { + vaultFiles = ["New/A+B.mp3", "Other/A B.mp3"]; + + await link({ url: "Old/A+B.mp3", episodeName: "A+B", time: "20" }); + metadata(); + + expect(media().src).toBe("app://resource/New/A+B.mp3"); + }); + + test("does not fall back to the legacy '+'-as-space form when the literal name is ambiguous", async () => { + const notice = vi.spyOn(obsidian, "Notice"); + vaultFiles = ["X/A+B.mp3", "Y/A+B.mp3", "Z/A B.mp3"]; + + await link({ url: "Old/A+B.mp3", episodeName: "A+B", time: "20" }); + + expect(notice).toHaveBeenCalledWith( + 'More than one vault file is named "A+B.mp3". Update the link with the correct path.', + ); + expect(media().src).toBe(""); + }); + + test("plays the loaded local file from where it moved to", async () => { + vaultFiles = ["A/Clip.mp3"]; + loadedAt(clip("A"), 15); + vaultFiles = ["C/Clip.mp3"]; + + await link({ url: "A/Clip.mp3", episodeName: "Clip", time: "30" }); + metadata(); + + expect(media().src).toBe("app://resource/C/Clip.mp3"); + expect(media().currentTime).toBe(30); + }); + + test("plays the loaded local file from where it moved to when a folder took its path", async () => { + vaultFiles = ["A/Clip.mp3"]; + loadedAt(clip("A"), 15); + vaultFiles = ["C/Clip.mp3"]; + vaultFolders = ["A/Clip.mp3"]; + + await link({ url: "A/Clip.mp3", episodeName: "Clip", time: "30" }); + metadata(); + + expect(media().src).toBe("app://resource/C/Clip.mp3"); + expect(media().currentTime).toBe(30); + }); + + test("plays the file at the link's legacy '+'-as-space path, not the loaded '+' file that moved away", async () => { + const plusClip: LocalEpisode = { + title: "Clip+One", + streamUrl: "app://resource/A/Clip+One.mp3", + url: "[[A/Clip+One.mp3]]", + description: "", + content: "", + podcastName: "local file", + filePath: "A/Clip+One.mp3", + }; + localFiles.set({ ...emptyLocalFiles, episodes: [plusClip] }); + vaultFiles = ["A/Clip+One.mp3", "A/Clip One.mp3"]; + loadedAt(plusClip, 15); + vaultFiles = ["C/Clip+One.mp3", "A/Clip One.mp3"]; + + await link({ url: "A/Clip+One.mp3", episodeName: "Clip+One", time: "30" }); + metadata(); + + expect(media().src).toBe("app://resource/A/Clip One.mp3"); + expect(media().currentTime).toBe(30); + }); + + test.each(["mp4", "webm"])( + "plays a moved .%s registered as video as video", + async (extension) => { + localFiles.set({ ...emptyLocalFiles, episodes: [clip("A", extension, "video")] }); + vaultFiles = [`B/Clip.${extension}`]; + + await link({ url: `A/Clip.${extension}`, episodeName: "Clip", time: "12" }); + metadata(); + + expect(state().mediaType).toBe("video"); + expect(media().tagName).toBe("VIDEO"); + expect(media().src).toBe(`app://resource/B/Clip.${extension}`); + expect(media().currentTime).toBe(12); + }, + ); + + test.each(["mp4", "webm"])( + "plays a loaded .%s video from where it moved to as video", + async (extension) => { + vaultFiles = [`A/Clip.${extension}`]; + loadedAt(clip("A", extension, "video"), 15); + vaultFiles = [`B/Clip.${extension}`]; + + await link({ url: `A/Clip.${extension}`, episodeName: "Clip", time: "30" }); + metadata(); + + expect(state().mediaType).toBe("video"); + expect(media().tagName).toBe("VIDEO"); + expect(media().src).toBe(`app://resource/B/Clip.${extension}`); + expect(media().currentTime).toBe(30); + }, + ); + + test("plays the moved file as it is registered at its new path", async () => { + localFiles.set({ + ...emptyLocalFiles, + episodes: [clip("A", "mp4", "video"), clip("B", "mp4", "audio")], + }); + vaultFiles = ["B/Clip.mp4"]; + + await link({ url: "A/Clip.mp4", episodeName: "Clip", time: "12" }); + metadata(); + + expect(state().mediaType).toBe("audio"); + expect(media().tagName).toBe("AUDIO"); + expect(media().src).toBe("app://resource/B/Clip.mp4"); + }); + + test("does not take a media type from a same-titled file registered at another path", async () => { + localFiles.set({ ...emptyLocalFiles, episodes: [clip("X", "mp4", "video")] }); + vaultFiles = ["B/Clip.mp4"]; + + await link({ url: "A/Clip.mp4", episodeName: "Clip", time: "12" }); + metadata(); + + expect(state().mediaType).toBe("audio"); + expect(media().tagName).toBe("AUDIO"); + expect(media().src).toBe("app://resource/B/Clip.mp4"); + }); + + test.each(["mp4", "webm"])( + "plays a moved .%s registered as video as video from a legacy link with '+' for its folder's space", + async (extension) => { + localFiles.set({ + ...emptyLocalFiles, + episodes: [clip("Old Folder", extension, "video")], + }); + vaultFiles = [`New/Clip.${extension}`]; + + await link({ url: `Old+Folder/Clip.${extension}`, episodeName: "Clip", time: "12" }); + metadata(); + + expect(state().mediaType).toBe("video"); + expect(media().tagName).toBe("VIDEO"); + expect(media().src).toBe(`app://resource/New/Clip.${extension}`); + expect(media().currentTime).toBe(12); + }, + ); + + test.each(["mp4", "webm"])( + "plays a loaded .%s video from where it moved to as video from a legacy link with '+' for its folder's space", + async (extension) => { + vaultFiles = [`Old Folder/Clip.${extension}`]; + loadedAt(clip("Old Folder", extension, "video"), 15); + vaultFiles = [`New/Clip.${extension}`]; + + await link({ url: `Old+Folder/Clip.${extension}`, episodeName: "Clip", time: "30" }); + metadata(); + + expect(state().mediaType).toBe("video"); + expect(media().tagName).toBe("VIDEO"); + expect(media().src).toBe(`app://resource/New/Clip.${extension}`); + expect(media().currentTime).toBe(30); + }, + ); + + test("does not take a media type from a file registered at the link's legacy path when that file has another name", async () => { + const legacyNamed: LocalEpisode = { + title: "A B", + streamUrl: "app://resource/Old Folder/A B.mp4", + url: "[[Old Folder/A B.mp4]]", + description: "", + content: "", + podcastName: "local file", + filePath: "Old Folder/A B.mp4", + mediaType: "video", + }; + localFiles.set({ ...emptyLocalFiles, episodes: [legacyNamed] }); + vaultFiles = ["New/A+B.mp4"]; + + await link({ url: "Old+Folder/A+B.mp4", episodeName: "A+B", time: "12" }); + metadata(); + + expect(state().mediaType).toBe("audio"); + expect(media().tagName).toBe("AUDIO"); + expect(media().src).toBe("app://resource/New/A+B.mp4"); + expect(media().currentTime).toBe(12); + }); +}); + describe("links that arrive while another link loads its feed", () => { test("a link refused before its fetch leaves the earlier one to play", async () => { let resolveFeed!: (episodes: Episode[]) => void; diff --git a/src/persistence/merge.test.ts b/src/persistence/merge.test.ts index b7ca6b8a..34e1f642 100644 --- a/src/persistence/merge.test.ts +++ b/src/persistence/merge.test.ts @@ -613,3 +613,113 @@ describe("restoreDropped", () => { expect(restoreDropped(base.queue, disk.queue, raw.queue, ["queue"])).toEqual(disk.queue); }); }); + +describe("entries named like Object.prototype members", () => { + const names = ["toString", "__proto__", "constructor", "hasOwnProperty"]; + const entryOf: Record unknown> = { + savedFeeds: (name) => feed(name), + playedEpisodes: (name) => ({ ...played("Ep 1", 10), podcastName: name }), + podNotes: (name) => ({ episodeName: "Ep 1", filePath: `${name}.md`, podcastFeedKey: name }), + playlists: (name) => playlist(name, [episode("Ep 1", name)]), + downloadedEpisodes: (name) => [ + { ...episode("Ep 1", name), filePath: `${name}/Ep 1.mp3`, size: 1 }, + ], + }; + const cases = Object.keys(entryOf).flatMap((map) => names.map((name) => [map, name])); + + function reread(doc: T): T { + return JSON.parse(JSON.stringify(doc)); + } + + function withEntry(doc: Data, map: string, name: string): Data { + Object.defineProperty(doc[map], name, { + value: entryOf[map](name), + enumerable: true, + writable: true, + configurable: true, + }); + return reread(doc); + } + + function entriesOf(doc: Data, map: string): Record { + return reread(doc)[map] as Record; + } + + it.each(cases)("keeps a %s entry named %s that the other device added", (map, name) => { + const base = data(); + const local = withEntry(reread(base), map, "Other"); + const disk = withEntry(reread(base), map, name); + + const merged = entriesOf(mergeDataFile(base, local, disk), map); + + expect(Object.keys(merged)).toEqual(expect.arrayContaining([name, "Other"])); + expect(merged[name]).toEqual(entryOf[map](name)); + }); + + it.each(cases)("leaves out a %s entry named %s that the other device removed", (map, name) => { + const base = withEntry(data(), map, name); + const local = withEntry(reread(base), map, "Other"); + const disk = reread(base); + delete (disk[map] as Record)[name]; + + const merged = entriesOf(mergeDataFile(base, local, disk), map); + + expect(Object.keys(merged)).toContain("Other"); + expect(Object.keys(merged)).not.toContain(name); + }); + + it("keeps every feed of two libraries that meet, whichever device merges", () => { + const started = withEntry( + withEntry(data(), "savedFeeds", "toString"), + "savedFeeds", + "constructor", + ); + started.libraryId = ""; + const joined = withEntry( + withEntry(data(), "savedFeeds", "__proto__"), + "savedFeeds", + "hasOwnProperty", + ); + joined.libraryId = "2026-10-11T09:00:00.000Z 0b9d"; + + for (const [local, disk] of [ + [started, joined], + [joined, started], + ]) { + const merged = entriesOf( + mergeDataFile(local, reread(local), reread(disk)), + "savedFeeds", + ); + expect(Object.keys(merged).sort()).toEqual(["Pod A", ...names].sort()); + expect(merged["__proto__"]).toEqual(feed("__proto__")); + } + }); + + it.each(names)("does not bring back a feed named %s the other device removed", (name) => { + const base = withEntry(data(), "savedFeeds", name); + const disk = reread(base); + delete (disk.savedFeeds as Record)[name]; + + const restored = restoreDropped(base.savedFeeds, disk.savedFeeds, reread(disk).savedFeeds, [ + "savedFeeds", + ]); + + expect(reread(restored)).toEqual({ "Pod A": feed("Pod A") }); + expect(Object.keys(restored)).toEqual(["Pod A"]); + }); + + it.each(names)("puts back a download list named %s that the codec dropped", (name) => { + const base = withEntry(data(), "downloadedEpisodes", name); + const disk = reread(base); + delete (disk.downloadedEpisodes as Record)[name]; + const raw = reread(base).downloadedEpisodes as Record; + Object.defineProperty(raw, name, { value: "not a list", enumerable: true }); + + const restored = restoreDropped(base.downloadedEpisodes, disk.downloadedEpisodes, raw, [ + "downloadedEpisodes", + ]); + + expect(Object.keys(restored)).toContain(name); + expect(reread(restored)[name]).toEqual(entryOf.downloadedEpisodes(name)); + }); +}); diff --git a/src/persistence/podNotesData.test.ts b/src/persistence/podNotesData.test.ts index 38606445..80046dfd 100644 --- a/src/persistence/podNotesData.test.ts +++ b/src/persistence/podNotesData.test.ts @@ -355,7 +355,7 @@ describe("PodNotes data schema", () => { expect((persisted.queue as Record).futurePlaylistField).toBe("kept"); }); - it("removes prototype-pollution keys", () => { + it("removes a root __proto__ key and keeps a playlist named constructor", () => { const raw = JSON.parse( '{"schemaVersion":1,"__proto__":{"polluted":true},"playlists":{"constructor":{"episodes":[]}}}', ) as Record; @@ -363,11 +363,99 @@ describe("PodNotes data schema", () => { const persisted = encodePodNotesData(decoded.settings, decoded.unknownFields); expect(decoded.unknownFields).not.toHaveProperty("__proto__"); - expect(decoded.settings.playlists).toEqual({}); - expect(persisted).not.toHaveProperty("__proto__"); + expect(Object.keys(decoded.settings.playlists)).toEqual(["constructor"]); + expect(Object.keys(JSON.parse(JSON.stringify(persisted)))).not.toContain("__proto__"); expect(({} as { polluted?: boolean }).polluted).toBeUndefined(); }); + describe("entries named like Object.prototype members", () => { + const names = ["toString", "__proto__", "constructor", "hasOwnProperty"]; + const maps = [ + "savedFeeds", + "podNotes", + "playedEpisodes", + "playlists", + "downloadedEpisodes", + ] as const; + + function library(entryNames: readonly string[]): unknown { + const entries = (value: (name: string) => unknown) => + Object.fromEntries(entryNames.map((name) => [name, value(name)])); + return JSON.parse( + JSON.stringify({ + schemaVersion: PODNOTES_DATA_SCHEMA_VERSION, + libraryId: "library", + savedFeeds: entries((name) => ({ + title: name, + url: `https://example.com/${name}.xml`, + artworkUrl: "", + })), + podNotes: entries((name) => ({ + episodeName: "Ep", + filePath: `Notes/${name}.md`, + podcastFeedKey: name, + })), + playedEpisodes: entries((name) => ({ + title: "Ep", + podcastName: name, + time: 5, + duration: 10, + finished: false, + })), + playlists: entries((name) => ({ + name, + icon: "list", + episodes: [], + shouldEpisodeRemoveAfterPlay: false, + shouldRepeat: false, + })), + downloadedEpisodes: entries((name) => [ + { + title: "Ep", + podcastName: name, + streamUrl: `https://example.com/${name}.mp3`, + url: "", + description: "", + content: "", + filePath: `${name}/Ep.mp3`, + size: 1, + }, + ]), + }), + ); + } + + it("keeps every entry across a save and a reload", () => { + const loaded = decodePodNotesData(library(names)); + const saved = JSON.stringify(encodePodNotesData(loaded.settings, loaded.unknownFields)); + const reloaded = decodePodNotesData(JSON.parse(saved)); + + expect(loaded.warnings).toEqual([]); + for (const map of maps) { + expect(Object.keys(reloaded.settings[map]).sort()).toEqual([...names].sort()); + } + expect(reloaded.settings.savedFeeds["__proto__"].url).toBe( + "https://example.com/__proto__.xml", + ); + const [download] = reloaded.settings.downloadedEpisodes[names[0]]; + expect(download.filePath).toBe(`${names[0]}/Ep.mp3`); + expect( + JSON.stringify(encodePodNotesData(reloaded.settings, reloaded.unknownFields)), + ).toBe(saved); + expect(({} as { url?: string }).url).toBeUndefined(); + }); + + it("finds nothing under a name the library does not have", () => { + const { settings } = decodePodNotesData(library([])); + + for (const map of maps) { + for (const name of [...names, "valueOf"]) { + expect(settings[map][name], `${map}.${name}`).toBeUndefined(); + } + } + }); + }); + it.each([[[]], ["data"], [42], [true]])("fails closed for malformed root %o", (value) => { expect(() => decodePodNotesData(value)).toThrowError(PodNotesDataError); }); diff --git a/src/services/privateFeeds.test.ts b/src/services/privateFeeds.test.ts index fa851adf..e683699a 100644 --- a/src/services/privateFeeds.test.ts +++ b/src/services/privateFeeds.test.ts @@ -1,6 +1,11 @@ import { describe, expect, it, vi } from "vitest"; import type { SecretStorage } from "obsidian"; import { DEFAULT_SETTINGS } from "src/constants"; +import { + decodePodNotesData, + encodePodNotesData, + PODNOTES_DATA_SCHEMA_VERSION, +} from "src/persistence/podNotesData"; import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; import type { PodcastFeed } from "src/types/PodcastFeed"; import { FeedUrlRepository } from "./FeedUrlRepository"; @@ -178,6 +183,39 @@ describe("protectPrivateFeeds", () => { expect(safe.downloadedEpisodes.Patreon[0].feedUrl).toBe("podnotes-private-feed:Patreon"); }); + it("replaces the moved URL in a loaded library's downloads and playlists under any name", () => { + const { feedUrls } = repository(); + const queued = episode("Ep 2", PRIVATE_URL); + const { settings: loaded } = decodePodNotesData( + JSON.parse( + JSON.stringify({ + schemaVersion: PODNOTES_DATA_SCHEMA_VERSION, + libraryId: "library", + savedFeeds: { toString: feed({ title: "toString", url: PRIVATE_URL }) }, + playlists: { + constructor: { + ...DEFAULT_SETTINGS.queue, + name: "constructor", + episodes: [queued], + }, + }, + downloadedEpisodes: { + toString: [{ ...queued, filePath: "Ep 2.mp3", size: 1 }], + }, + }), + ), + ); + + const { settings: safe } = protectPrivateFeeds(loaded, feedUrls); + const [download] = Object.values(safe.downloadedEpisodes)[0]; + const [listed] = Object.values(safe.playlists)[0].episodes; + + expect(JSON.stringify(encodePodNotesData(safe))).not.toContain("se-cret"); + expect(download.feedUrl).toBe("podnotes-private-feed:toString"); + expect(download.episodeDate).toEqual(queued.episodeDate); + expect(listed.feedUrl).toBe("podnotes-private-feed:toString"); + }); + it("is idempotent: a second run over protected settings changes nothing", () => { const { feedUrls } = repository(); const first = protectPrivateFeeds( diff --git a/src/store/index.test.ts b/src/store/index.test.ts index d31b2ef7..31169de5 100644 --- a/src/store/index.test.ts +++ b/src/store/index.test.ts @@ -1,5 +1,6 @@ -import { get } from "svelte/store"; -import { beforeEach, describe, expect, test, vi } from "vitest"; +import type { Vault } from "obsidian"; +import { get, type Writable } from "svelte/store"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import type { Episode } from "src/types/Episode"; import type { IPodNotes } from "src/types/IPodNotes"; @@ -11,15 +12,18 @@ import { MAX_EPISODE_LIST_LIMIT, QUEUE_SETTINGS, } from "src/constants"; +import { selectSource } from "src/player/source"; import { bindStoresToSettings } from "src/store/persistence"; import { dedupeEpisodesByKey, downloadedEpisodes, localFiles, playedEpisodes, + playlists, queue, reorderEpisodes, sanitizeEpisodeListLimit, + savedFeeds, } from "./index"; const episode: Episode = { @@ -550,3 +554,86 @@ describe("sanitizeEpisodeListLimit (issue #114)", () => { expect(sanitizeEpisodeListLimit(10 ** 9)).toBe(MAX_EPISODE_LIST_LIMIT); }); }); + +describe("stores keyed by a podcast, playlist or episode name", () => { + const names = ["toString", "__proto__", "constructor", "hasOwnProperty"]; + const stores = [ + ["savedFeeds", savedFeeds], + ["downloadedEpisodes", downloadedEpisodes], + ["playedEpisodes", playedEpisodes], + ["playlists", playlists], + ] as unknown as [string, Writable>][]; + + afterEach(() => { + for (const field of ["time", "finished"]) { + delete (Object.prototype as Record)[field]; + } + }); + + test.each(stores)( + "%s keeps the names it is given, after a set and a replacing update, and finds no other", + (_, store) => { + store.set(Object.fromEntries(names.map((name) => [name, { name }]))); + store.update((current) => ({ ...current, Other: { name: "Other" } })); + + const current = get(store); + expect(Object.keys(current).sort()).toEqual([...names, "Other"].sort()); + expect(current["__proto__"]).toEqual({ name: "__proto__" }); + for (const absent of ["valueOf", "isPrototypeOf"]) { + expect(current[absent], absent).toBeUndefined(); + } + }, + ); + + test.each(names)("streams an episode of a podcast named %s that has no downloads", (name) => { + downloadedEpisodes.set({}); + const of = { ...episode, podcastName: name }; + const vault = { getAbstractFileByPath: () => null } as unknown as Vault; + + expect(downloadedEpisodes.getEpisode(of)).toBeUndefined(); + expect(downloadedEpisodes.isEpisodeDownloaded(of)).toBeFalsy(); + expect(selectSource(of, get(downloadedEpisodes), vault).src).toBe(episode.streamUrl); + }); + + test.each(names)("downloads an episode of a podcast named %s", (name) => { + downloadedEpisodes.set({}); + const of = { ...episode, podcastName: name }; + + downloadedEpisodes.addEpisode(of, `${name}/Shared title.mp3`, 1); + + expect(downloadedEpisodes.getEpisode(of)?.filePath).toBe(`${name}/Shared title.mp3`); + expect(downloadedEpisodes.isEpisodeDownloaded(of)).toBe(true); + expect(Object.keys(get(downloadedEpisodes))).toEqual([name]); + }); + + test("marks an episode titled __proto__ played without touching other objects", () => { + playedEpisodes.set({}); + const of = { ...episode, title: "__proto__", podcastName: "" }; + + playedEpisodes.markAsPlayed(of); + + expect(playedEpisodes.get(of)?.finished).toBe(true); + expect(({} as { finished?: boolean }).finished).toBeUndefined(); + }); + + test("marking one show's episode played leaves another show's same-titled legacy progress alone", () => { + const showA: PlayedEpisode = { + title: episode.title, + podcastName: "Show A", + time: 30, + duration: 60, + finished: false, + }; + playedEpisodes.set({ [episode.title]: { ...showA } }); + const showBEpisode = { ...episode, podcastName: "Show B" }; + + playedEpisodes.markAsPlayed(showBEpisode); + + expect(get(playedEpisodes)[episode.title]).toEqual(showA); + expect(playedEpisodes.get({ ...episode, podcastName: "Show A" })).toEqual(showA); + expect(get(playedEpisodes)["Show B::Shared title"]).toMatchObject({ + podcastName: "Show B", + finished: true, + }); + }); +}); diff --git a/src/ui/settings/PodNotesSettingsTab.import.test.ts b/src/ui/settings/PodNotesSettingsTab.import.test.ts index f68598c5..dcf4e459 100644 --- a/src/ui/settings/PodNotesSettingsTab.import.test.ts +++ b/src/ui/settings/PodNotesSettingsTab.import.test.ts @@ -5,6 +5,7 @@ import type { App } from "obsidian"; import * as obsidian from "obsidian"; import { DEFAULT_SETTINGS } from "src/constants"; +import { decodePodNotesData, PODNOTES_DATA_SCHEMA_VERSION } from "src/persistence/podNotesData"; import PodNotes from "src/main"; import { PlayerController } from "src/player/PlayerController"; import { episodeListLimit, hidePlayedEpisodes, savedFeeds } from "src/store"; @@ -89,6 +90,54 @@ describe("PodNotesSettingsTab settings import", () => { expect(plugin.saveSettingsStrict).toHaveBeenCalledTimes(2); }); + it("puts back a feed named __proto__ when an import that removed it fails", async () => { + const showFeed = { + title: "__proto__", + url: "https://example.com/feed.xml", + artworkUrl: "", + }; + const laterFeed = { title: "Later", url: "https://example.com/later.xml", artworkUrl: "" }; + const previous = decodePodNotesData( + JSON.parse( + JSON.stringify({ + schemaVersion: PODNOTES_DATA_SCHEMA_VERSION, + libraryId: "library", + savedFeeds: Object.fromEntries([["__proto__", showFeed]]), + }), + ), + ).settings; + const plugin = { + settings: previous, + credentials: { storeValues: vi.fn(() => ({})) }, + feedUrls: { resolve: vi.fn(() => null) }, + saveSettings: vi.fn().mockResolvedValue(undefined), + saveSettingsStrict: vi + .fn() + .mockImplementationOnce(() => { + plugin.settings.savedFeeds.Later = laterFeed; + return Promise.reject(new Error("disk full")); + }) + .mockResolvedValueOnce(undefined), + } as unknown as PodNotes; + const tab = new PodNotesSettingsTab({} as App, withPlayer(plugin)); + vi.spyOn(tab, "update").mockImplementation(() => {}); + vi.spyOn(console, "error").mockImplementation(() => undefined); + + await ( + tab as unknown as { + applyImportedSettings: ( + imported: Partial, + ) => Promise; + } + ).applyImportedSettings({ savedFeeds: {} }); + + expect(Object.keys(plugin.settings.savedFeeds).sort()).toEqual(["Later", "__proto__"]); + expect(plugin.settings.savedFeeds.Later).toEqual(laterFeed); + expect(plugin.settings.savedFeeds["__proto__"]).toEqual(showFeed); + expect(plugin.settings.savedFeeds.toString).toBeUndefined(); + expect(({} as { url?: string }).url).toBeUndefined(); + }); + it("locks ordinary settings controls until an import and its lane drain finish", async () => { const firstSave = deferred(); const plugin = { diff --git a/src/utility/episodeListEntry.test.ts b/src/utility/episodeListEntry.test.ts index 96f093ca..35f54eb6 100644 --- a/src/utility/episodeListEntry.test.ts +++ b/src/utility/episodeListEntry.test.ts @@ -79,4 +79,39 @@ describe("episodeListEntry", () => { expect(entries.map((entry) => entry.episode.title)).toEqual(["Newer", "Older", "Missing"]); }); + describe("a legacy record keyed by its title", () => { + const showA = episode("Shared episode", "Show A", "2024-02-01T00:00:00.000Z"); + const showB = episode("Shared episode", "Show B", "2024-02-01T00:00:00.000Z"); + + test("resolves to its own podcast's episode when another podcast's same-titled one comes first", () => { + const entries = buildPlayedEpisodeListEntries( + { "Shared episode": played("Shared episode", "Show A") }, + [[showB, showA]], + ); + + expect(entries).toHaveLength(1); + expect(entries[0]).toMatchObject({ episode: showA, isAvailable: true }); + }); + + test("stays unavailable when only another podcast has its title", () => { + const entries = buildPlayedEpisodeListEntries( + { "Shared episode": played("Shared episode", "Show A") }, + [[showB]], + ); + + expect(entries[0]).toMatchObject({ + isAvailable: false, + episode: { title: "Shared episode", podcastName: "Show A" }, + }); + }); + + test("resolves by title when it names no podcast", () => { + const entries = buildPlayedEpisodeListEntries( + { "Shared episode": played("Shared episode", "") }, + [[showB]], + ); + + expect(entries[0]).toMatchObject({ episode: showB, isAvailable: true }); + }); + }); }); diff --git a/src/utility/episodeStatus.test.ts b/src/utility/episodeStatus.test.ts index a1ab26a0..bc3f7232 100644 --- a/src/utility/episodeStatus.test.ts +++ b/src/utility/episodeStatus.test.ts @@ -50,6 +50,34 @@ describe("episodeStatus", () => { expect(isEpisodeFinished(episode, playedEpisodes)).toBe(true); }); + test("does not take another podcast's legacy record for this episode", () => { + const playedEpisodes = { + [episode.title]: playedEpisode("Other Podcast", true), + }; + + expect(getPlayedEpisode(playedEpisodes, episode)).toBeUndefined(); + expect(isEpisodeFinished(episode, playedEpisodes)).toBe(false); + }); + + test("takes a legacy record that names no podcast", () => { + const playedEpisodes = { + [episode.title]: playedEpisode("", true), + }; + + expect(isEpisodeFinished(episode, playedEpisodes)).toBe(true); + }); + + test.each(["toString", "__proto__", "constructor", "hasOwnProperty"])( + "finds no progress for an unplayed episode titled %s", + (title) => { + for (const podcastName of ["Design Podcast", ""]) { + const unplayed = { ...episode, title, podcastName }; + expect(getPlayedEpisode({}, unplayed)).toBeUndefined(); + expect(isEpisodeFinished(unplayed, {})).toBe(false); + } + }, + ); + test("lists only finished played records", () => { const records = getFinishedPlayedEpisodeRecords({ finished: playedEpisode("Design Podcast", true), diff --git a/tests/e2e/journey-feeds.test.ts b/tests/e2e/journey-feeds.test.ts index a3bd96ca..8c34d688 100644 --- a/tests/e2e/journey-feeds.test.ts +++ b/tests/e2e/journey-feeds.test.ts @@ -462,4 +462,48 @@ describe("feed journeys", () => { expect(await listedEpisodes(obsidian)).toEqual(["Stale Two", "Stale One"]); expect(await notices(obsidian)).toEqual([]); }); + test("podcasts titled toString and __proto__ download, queue, play and keep their names after a reload", async () => { + const context = getContext(); + const { obsidian, plugin } = context; + const toStringShow = feed("toString", "to-string", ["To String Episode"]); + const protoShow = feed("__proto__", "proto", ["Proto Episode"]); + const server = await serve(toStringShow, protoShow); + await seedLibrary(context, server, { subscriptions: [toStringShow, protoShow] }); + + await showPodNotes(obsidian); + await click(obsidian, ui.podcastCard("toString")); + await waitUntilShown(obsidian, ui.episode("To String Episode")); + await click(obsidian, ui.episode("To String Episode"), { button: "right" }); + await click(obsidian, ui.menuItem("Download")); + await waitForLibrary( + plugin, + (library) => library.downloads.some((entry) => entry.podcast === "toString"), + "the toString episode downloaded", + ); + await click(obsidian, ui.episode("To String Episode"), { button: "right" }); + await click(obsidian, ui.menuItem("Add to Queue")); + await waitForLibrary( + plugin, + (library) => library.queue.includes("To String Episode"), + "the toString episode queued", + ); + await click(obsidian, ui.episode("To String Episode")); + await waitForPlayback( + obsidian, + (playback) => playback.episode?.podcast === "toString" && playback.seconds > 0, + "the toString episode playing", + ); + + await plugin.reload(RELOAD_OPTIONS); + const saved = await plugin.data<{ savedFeeds: Record }>().read(); + expect(Object.keys(saved.savedFeeds).sort()).toEqual(["__proto__", "toString"]); + expect( + await obsidian.dev.evalJson( + `Object.keys(app.plugins.plugins.podnotes.settings.savedFeeds).includes("__proto__") && ({}).url === undefined`, + ), + ).toBe(true); + await showPodNotes(obsidian); + await click(obsidian, ui.podcastGridTab); + await waitUntilShown(obsidian, ui.podcastCard("__proto__")); + }); }); diff --git a/tests/e2e/journey-links-local.test.ts b/tests/e2e/journey-links-local.test.ts index 4a384609..881fb4ed 100644 --- a/tests/e2e/journey-links-local.test.ts +++ b/tests/e2e/journey-links-local.test.ts @@ -3,7 +3,9 @@ import { createPodNotesE2EHarness, PLUGIN_ID, RELOAD_OPTIONS } from "./harness"; import { click, closePodNotes, + dismissNotices, listedEpisodes, + notices, openInReadingView, revealInFileExplorer, runCommand, @@ -21,6 +23,7 @@ import { } from "./journey/fixtures"; import { episodeRef, + observePlayback, playbackOver, playerLeafCount, readLibrary, @@ -258,4 +261,46 @@ describe("link and local file journeys", () => { }); expect(await listedEpisodes(obsidian)).toEqual(["Local Journey"]); }); + test("a timestamp link to a moved vault file plays the one file with its name, and refuses when two share it", async () => { + const context = getContext(); + const { obsidian, sandbox } = context; + await seedLibrary(context, null); + const title = `Twin-${sandbox.root.split("/").pop()}`; + const fileName = `${title}.wav`; + const movedPath = sandbox.path("Moved/A", fileName); + await writeVaultFile(obsidian, movedPath, wav(30)); + const notePath = sandbox.path("Moved link.md"); + const linkedPath = encodeURIComponent(sandbox.path("Missing", fileName)); + await writeVaultFile( + obsidian, + notePath, + `[Moved audio](obsidian://podnotes?episodeName=${encodeURIComponent(title)}&url=${linkedPath}&time=2)\n`, + ); + + await openInReadingView(obsidian, notePath); + await click(obsidian, ui.readingLink("Moved audio")); + const playing = await waitForPlayback( + obsidian, + (playback) => playback.episode?.title === title && (playback.media?.seconds ?? 0) > 0, + "the moved file playing", + ); + expect(decodeURIComponent(playing.media?.src ?? "")).toContain(movedPath); + expect(playing.seconds).toBeGreaterThanOrEqual(2); + + await runCommand(obsidian, `${PLUGIN_ID}:stop-playing`); + await writeVaultFile(obsidian, sandbox.path("Moved/B", fileName), wav(30)); + await dismissNotices(obsidian); + await openInReadingView(obsidian, notePath); + await click(obsidian, ui.readingLink("Moved audio")); + await obsidian.waitFor( + async () => + (await notices(obsidian)).includes( + `More than one vault file is named "${fileName}". Update the link with the correct path.`, + ), + { timeoutMs: 10_000, message: "the ambiguous file notice" }, + ); + const refused = await observePlayback(obsidian); + expect(refused.playing).toBe(false); + expect(decodeURIComponent(refused.media?.src ?? "")).toContain(movedPath); + }); }); From 7a125b0cf9054fa6d113fe7072586c76e75baed9 Mon Sep 17 00:00:00 2001 From: Christian Bager Bach Houmann Date: Mon, 12 Oct 2026 19:51:30 +0200 Subject: [PATCH 2/2] fix: make podcast name lookups exact and safe --- src/URIHandler.ts | 63 ++++++++++++++++++++------ src/constants.ts | 11 +++-- src/main.ts | 4 +- src/persistence/codecUtils.ts | 24 ++-------- src/persistence/collectionCodecs.ts | 32 ++++++------- src/persistence/merge.ts | 28 ++++++++---- src/player/PlayerController.ts | 14 +----- src/services/privateFeeds.ts | 10 ++-- src/settingsTransfer.ts | 4 +- src/store/downloads.ts | 5 +- src/store/feeds.ts | 3 +- src/store/index.ts | 5 +- src/store/recordStore.ts | 15 ++++++ src/ui/settings/PodNotesSettingsTab.ts | 16 +++++-- src/utility/episodeListEntry.ts | 7 ++- src/utility/episodeStatus.ts | 21 +++++---- src/utility/ownRecord.ts | 3 ++ 17 files changed, 159 insertions(+), 106 deletions(-) create mode 100644 src/store/recordStore.ts create mode 100644 src/utility/ownRecord.ts diff --git a/src/URIHandler.ts b/src/URIHandler.ts index 8a9ad83b..955e1356 100644 --- a/src/URIHandler.ts +++ b/src/URIHandler.ts @@ -9,6 +9,7 @@ import { resolveFeedUrl } from "./services/privateFeeds"; import { isPrivateFeedPlaceholder, parsePrivateFeedPlaceholder } from "./utility/privateFeedUrl"; import { findUniqueTitleMatch, titleTokenSimilarity } from "./utility/episodeTitleMatch"; import { episodeLinkTarget } from "./utility/episodeLinkTarget"; +import { isLocalFile } from "./utility/isLocalFile"; import { localEpisodeFromFile } from "./utility/localEpisodeFromFile"; import { getEpisodeMediaType, getUnambiguousMediaTypeFromPath } from "./utility/mediaType"; import { ViewState } from "./types/ViewState"; @@ -62,6 +63,23 @@ function findLocalEpisodeByCandidates(nameCandidates: string[]): Episode | undef ); } +function filesNamedLike( + files: readonly TFile[], + paths: readonly string[], +): { paths: string[]; name: string; files: TFile[] } { + const candidates = paths.map((path) => { + const name = path.slice(path.lastIndexOf("/") + 1); + return { path, name, files: files.filter((file) => file.name === name) }; + }); + const { name, files: named } = + candidates.find((candidate) => candidate.files.length > 0) ?? candidates[0]; + return { + paths: candidates.filter((candidate) => candidate.name === name).map(({ path }) => path), + name, + files: named, + }; +} + /** A number of seconds from a link, or NaN unless the whole value is one. */ function parseSeconds(value: string): number { return value.trim() === "" ? Number.NaN : Number(value); @@ -109,13 +127,17 @@ export default async function podNotesURIHandler( const player = get(plugin).player; const playOptions = hasExplicitTime ? { time: requestedTime, endTime: requestedEndTime } : {}; const loaded = get(player.state).episode; + const { app } = get(plugin); + const localFile = candidateValues(url) + .map((path) => app.vault.getAbstractFileByPath(path)) + .find((file) => file instanceof TFile); // A link to the loaded episode names its feed, or its file for a local file, so a // same-titled episode of another show is not taken for it. const loadedLinkTarget = loaded && episodeLinkTarget(loaded); // Membership (not ordered selection) is correct here: there is only one loaded episode, and // keeping the legacy candidate lets a legacy '+'-as-space link to the loaded episode resume // without a feed round-trip. The only cost is a rare false-positive - a current-format link to - // a "X+Y" episode while a distinct "X Y" twin is loaded resumes the loaded twin instead of + // a "X+Y" feed episode while a distinct "X Y" twin is loaded resumes the loaded twin instead of // switching - which we accept over regressing the far more common same-episode legacy-link case. // A retitled same-episode link (guest name moved) has the same token set. // Sub-1.0 overlap is not enough here: "… Part 1" vs the loaded "… Part 2" @@ -124,6 +146,7 @@ export default async function podNotesURIHandler( const episodeIsLoaded = !!loaded && !!loadedLinkTarget && + (!isLocalFile(loaded) || localFile?.path === loadedLinkTarget) && candidateValues(url).includes(loadedLinkTarget) && (nameCandidates.includes(loaded.title) || nameCandidates.some((name) => titleTokenSimilarity(name, loaded.title) === 1)); @@ -136,12 +159,6 @@ export default async function podNotesURIHandler( return; } - // The path probe decides local-vs-feed routing; the episode itself is resolved by name. - const { app } = get(plugin); - const localFile = candidateValues(url) - .map((path) => app.vault.getAbstractFileByPath(path)) - .find((file) => file !== null); - let episode: Episode | undefined; let play = player.play.bind(player); @@ -154,9 +171,7 @@ export default async function podNotesURIHandler( // folder has since taken over the name's registration. const named = findLocalEpisodeByCandidates(nameCandidates); episode = - named && - localFile instanceof TFile && - (named as LocalEpisode).filePath !== localFile.path + named && (named as LocalEpisode).filePath !== localFile.path ? localEpisodeFromFile( app, localFile, @@ -166,11 +181,29 @@ export default async function podNotesURIHandler( ) : named; } else if (!isPlaceholderLink && !candidateValues(url).some((u) => /^https?:\/\//i.test(u))) { - // The probe found no file, yet `url` has no http(s) scheme, so it can only be - // a vault path whose file was moved/renamed. Resolve the episode by name from - // the local-files store rather than fetching the bare path as a feed (which - // would fail and show a misleading "Could not load the podcast feed"). - episode = findLocalEpisodeByCandidates(nameCandidates); + const { paths, name, files } = filesNamedLike(app.vault.getFiles(), candidateValues(url)); + if (files.length !== 1) { + new Notice( + files.length === 0 + ? `No vault file is named "${name}".` + : `More than one vault file is named "${name}". Update the link with the correct path.`, + ); + return; + } + const [file] = files; + const { episodes: registered } = get(localFiles); + const original = paths.flatMap((path) => + [loaded, ...registered].filter( + (candidate) => (candidate as LocalEpisode | undefined)?.filePath === path, + ), + )[0]; + episode = + registered.find((candidate) => (candidate as LocalEpisode).filePath === file.path) ?? + localEpisodeFromFile( + app, + file, + getUnambiguousMediaTypeFromPath(file.path) ?? original?.mediaType ?? "audio", + ); } else { // Links for a private feed carry the non-fetchable placeholder, not the // secret URL. Resolution is local-only: the placeholder's feed name must diff --git a/src/constants.ts b/src/constants.ts index 7c89cd53..9ea421de 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1,6 +1,7 @@ import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; import type { Playlist } from "./types/Playlist"; import { DEFAULT_SPEAKER_TEMPLATE } from "./services/diarization/segments"; +import { ownRecord } from "./utility/ownRecord"; export const VIEW_TYPE = "podcast_player_view"; @@ -56,13 +57,13 @@ export const PLAYED_SETTINGS: PlaylistSettings = { }; export const DEFAULT_SETTINGS: IPodNotesSettings = { - savedFeeds: {}, - podNotes: {}, + savedFeeds: ownRecord(), + podNotes: ownRecord(), defaultPlaybackRate: 1, defaultVolume: 1, hidePlayedEpisodes: false, episodeListLimit: DEFAULT_EPISODE_LIST_LIMIT, - playedEpisodes: {}, + playedEpisodes: ownRecord(), favorites: { ...FAVORITES_SETTINGS, episodes: [], @@ -72,7 +73,7 @@ export const DEFAULT_SETTINGS: IPodNotesSettings = { episodes: [], }, autoQueue: true, - playlists: {}, + playlists: ownRecord(), skipBackwardLength: 15, skipForwardLength: 15, currentEpisode: undefined, @@ -149,7 +150,7 @@ export const DEFAULT_SETTINGS: IPodNotesSettings = { // download writes junk and the second throws "File already exists" (#183). path: "PodNotes/{{podcast}}/{{title}}", }, - downloadedEpisodes: {}, + downloadedEpisodes: ownRecord(), localFiles: { ...LOCAL_FILES_SETTINGS, episodes: [], diff --git a/src/main.ts b/src/main.ts index 20e0c511..6962e31a 100644 --- a/src/main.ts +++ b/src/main.ts @@ -15,7 +15,7 @@ import { registerCommands } from "src/commands"; import { normalizePath, Notice, Platform, Plugin, type Editor, type WorkspaceLeaf } from "obsidian"; import { API } from "src/API/API"; import type { IAPI } from "src/API/IAPI"; -import { DEFAULT_SETTINGS, VIEW_TYPE } from "src/constants"; +import { VIEW_TYPE } from "src/constants"; import { PodNotesSettingsTab } from "src/ui/settings/PodNotesSettingsTab"; import { MainView } from "src/ui/PodcastView"; import type { IPodNotesSettings } from "./types/IPodNotesSettings"; @@ -481,7 +481,7 @@ export default class PodNotes extends Plugin implements IPodNotes { // loadData resolves nullish both for a missing file and for one it could // not read or parse. if (data == null && !(await this.dataFileHoldsNothing())) { - this.settings = structuredClone(DEFAULT_SETTINGS); + this.settings = decodePodNotesData(undefined).settings; this.persistenceUnknownFields = { libraryId: newLibraryId() }; this.refuseSaves( "unreadable-data-file", diff --git a/src/persistence/codecUtils.ts b/src/persistence/codecUtils.ts index 11ad9e57..c67c1a19 100644 --- a/src/persistence/codecUtils.ts +++ b/src/persistence/codecUtils.ts @@ -1,3 +1,5 @@ +import { ownRecord } from "src/utility/ownRecord"; + export type UnknownRecord = Record; export const DANGEROUS_KEYS = new Set(["__proto__", "constructor", "prototype"]); @@ -155,30 +157,12 @@ export function copySafeObject(value: UnknownRecord): UnknownRecord { return copy; } -export function safeEntries( - value: UnknownRecord, - warnings: Set, - path: string, -): [string, unknown][] { - const entries: [string, unknown][] = []; - for (const [key, field] of Object.entries(value)) { - if (DANGEROUS_KEYS.has(key)) { - warn(warnings, `${path}.${key}`, "unsafe key was removed"); - continue; - } - entries.push([key, field]); - } - return entries; -} - export function mapRecord( value: Record, mapper: (entry: T) => R, ): Record { - const result: Record = {}; - for (const [key, entry] of Object.entries(value)) { - if (!DANGEROUS_KEYS.has(key)) result[key] = mapper(entry); - } + const result = ownRecord(); + for (const [key, entry] of Object.entries(value)) result[key] = mapper(entry); return result; } diff --git a/src/persistence/collectionCodecs.ts b/src/persistence/collectionCodecs.ts index 22f35386..04ed8ade 100644 --- a/src/persistence/collectionCodecs.ts +++ b/src/persistence/collectionCodecs.ts @@ -3,13 +3,13 @@ import type { Playlist } from "src/types/Playlist"; import type { PodcastFeed } from "src/types/PodcastFeed"; import type { PodNote } from "src/types/PodNotes"; import type { PlayedEpisode } from "src/types/PlayedEpisode"; +import { ownRecord } from "src/utility/ownRecord"; import { copySafeObject, isPlainObject, readBoolean, readNonNegativeNumber, readString, - safeEntries, setOptionalString, warn, } from "./codecUtils"; @@ -21,11 +21,11 @@ export function decodeSavedFeeds( ): Record { if (!isPlainObject(value)) { if (value !== undefined) warn(warnings, "savedFeeds", "expected an object"); - return {}; + return ownRecord(); } - const feeds: Record = {}; - for (const [key, candidate] of safeEntries(value, warnings, "savedFeeds")) { + const feeds = ownRecord(); + for (const [key, candidate] of Object.entries(value)) { if (!isPlainObject(candidate)) { warn(warnings, `savedFeeds.${key}`, "expected an object; feed was skipped"); continue; @@ -53,11 +53,11 @@ export function decodeSavedFeeds( export function decodePodNotes(value: unknown, warnings: Set): Record { if (!isPlainObject(value)) { if (value !== undefined) warn(warnings, "podNotes", "expected an object"); - return {}; + return ownRecord(); } - const notes: Record = {}; - for (const [key, candidate] of safeEntries(value, warnings, "podNotes")) { + const notes = ownRecord(); + for (const [key, candidate] of Object.entries(value)) { if (!isPlainObject(candidate)) { warn(warnings, `podNotes.${key}`, "expected an object; note mapping was skipped"); continue; @@ -84,11 +84,11 @@ export function decodePlayedEpisodes( ): Record { if (!isPlainObject(value)) { if (value !== undefined) warn(warnings, "playedEpisodes", "expected an object"); - return {}; + return ownRecord(); } - const played: Record = {}; - for (const [key, candidate] of safeEntries(value, warnings, "playedEpisodes")) { + const played = ownRecord(); + for (const [key, candidate] of Object.entries(value)) { if (!isPlainObject(candidate)) { warn(warnings, `playedEpisodes.${key}`, "expected an object; progress was skipped"); continue; @@ -120,11 +120,11 @@ export function decodePlayedEpisodes( export function decodePlaylists(value: unknown, warnings: Set): Record { if (!isPlainObject(value)) { if (value !== undefined) warn(warnings, "playlists", "expected an object"); - return {}; + return ownRecord(); } - const playlists: Record = {}; - for (const [key, candidate] of safeEntries(value, warnings, "playlists")) { + const playlists = ownRecord(); + for (const [key, candidate] of Object.entries(value)) { if (!isPlainObject(candidate)) { warn(warnings, `playlists.${key}`, "expected an object; playlist was skipped"); continue; @@ -151,11 +151,11 @@ export function decodeDownloadedEpisodes( ): Record { if (!isPlainObject(value)) { if (value !== undefined) warn(warnings, "downloadedEpisodes", "expected an object"); - return {}; + return ownRecord(); } - const downloads: Record = {}; - for (const [podcastName, candidate] of safeEntries(value, warnings, "downloadedEpisodes")) { + const downloads = ownRecord(); + for (const [podcastName, candidate] of Object.entries(value)) { if (!Array.isArray(candidate)) { warn( warnings, diff --git a/src/persistence/merge.ts b/src/persistence/merge.ts index 200b9949..1727712b 100644 --- a/src/persistence/merge.ts +++ b/src/persistence/merge.ts @@ -1,6 +1,7 @@ import { DEFAULT_SETTINGS } from "src/constants"; import type { Episode } from "src/types/Episode"; import { getEpisodeKey } from "src/utility/episodeKey"; +import { ownRecord } from "src/utility/ownRecord"; import { isPlainObject, type UnknownRecord } from "./codecUtils"; import { toDocument } from "./podNotesData"; @@ -35,6 +36,10 @@ export function jsonValuesEqual(a: unknown, b: unknown): boolean { return false; } +function ownValue(record: UnknownRecord, key: string): unknown { + return Object.prototype.hasOwnProperty.call(record, key) ? record[key] : undefined; +} + type EpisodeEntry = UnknownRecord & { title: string }; /** An entry that has an episode key: its title, and its podcast name if any, are text. */ @@ -188,12 +193,14 @@ export function threeWayMergeSettings( ...Object.keys(local), ...Object.keys(disk), ]); - const merged: UnknownRecord = {}; + const merged = ownRecord(); for (const key of keys) { - const value = threeWayMergeSettings(ancestor[key], local[key], disk[key], [ - ...path, - key, - ]); + const value = threeWayMergeSettings( + ownValue(ancestor, key), + ownValue(local, key), + ownValue(disk, key), + [...path, key], + ); // JSON has no undefined: a merged-away key must be absent, or the // written document never equals the same document read back. if (value !== undefined) merged[key] = value; @@ -243,11 +250,14 @@ export function restoreDropped(base: T, disk: T, raw: unknown, path: readonly if (isPlainObject(base) && isPlainObject(disk)) { // Not an object: the codec replaced the whole value, so every entry was dropped. const rawEntries = isPlainObject(raw) ? raw : {}; - const restored: UnknownRecord = { ...disk }; + const restored = ownRecord(disk); for (const [key, value] of Object.entries(base)) { - if (key in disk) { - restored[key] = restoreDropped(value, disk[key], rawEntries[key], [...path, key]); - } else if (!isPlainObject(raw) || key in raw) { + if (Object.prototype.hasOwnProperty.call(disk, key)) { + restored[key] = restoreDropped(value, disk[key], ownValue(rawEntries, key), [ + ...path, + key, + ]); + } else if (!isPlainObject(raw) || Object.prototype.hasOwnProperty.call(raw, key)) { restored[key] = structuredClone(value); } } diff --git a/src/player/PlayerController.ts b/src/player/PlayerController.ts index a4dd03ae..bd51e450 100644 --- a/src/player/PlayerController.ts +++ b/src/player/PlayerController.ts @@ -6,6 +6,7 @@ import type { Episode, EpisodeMediaType } from "src/types/Episode"; import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; import type { PlayedEpisode } from "src/types/PlayedEpisode"; import { getEpisodeKey, isSameStoredEpisode } from "src/utility/episodeKey"; +import { getPlayedEpisode } from "src/utility/episodeStatus"; import { getEpisodeMediaType } from "src/utility/mediaType"; import { normalizePlaybackRate } from "src/utility/playbackRate"; import { selectSource, type PlaybackSource } from "./source"; @@ -551,19 +552,8 @@ export class PlayerController { this.moved = false; } - /** The composite key's record, else a legacy title-only record that does not name another podcast. */ private savedProgress(episode: Episode): PlayedEpisode | undefined { - const played = get(playedEpisodes); - const legacy = played[episode.title]; - return ( - played[getEpisodeKey(episode)] ?? - (legacy && - (!legacy.podcastName || - !episode.podcastName || - legacy.podcastName === episode.podcastName) - ? legacy - : undefined) - ); + return getPlayedEpisode(get(playedEpisodes), episode); } /** diff --git a/src/services/privateFeeds.ts b/src/services/privateFeeds.ts index 8db34744..47cce388 100644 --- a/src/services/privateFeeds.ts +++ b/src/services/privateFeeds.ts @@ -1,6 +1,7 @@ import { Notice } from "obsidian"; import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; import type { PodcastFeed } from "src/types/PodcastFeed"; +import { ownRecord } from "src/utility/ownRecord"; import { isCredentialBearingUrl, isPrivateFeedPlaceholder, @@ -92,7 +93,7 @@ export function protectPrivateFeeds( feedUrls: FeedUrlRepository, ): PrivateFeedProtection { const failed: string[] = []; - const savedFeeds: Record = {}; + const savedFeeds = ownRecord(); // A private feed's real URL maps to the placeholder naming its saved key, // which links resolve by. const byUrl = new Map(); @@ -131,13 +132,12 @@ function scrubEpisodeUrls( ): T { if (typeof value !== "object" || value === null) return value; const isArray = Array.isArray(value); - // Decoded settings carry Date instances (episodeDate); recursing into one - // would flatten it into {}. Non-plain objects hold no episode URLs. - if (!isArray && Object.getPrototypeOf(value) !== Object.prototype) return value; + const prototype = Object.getPrototypeOf(value) as object | null; + if (!isArray && prototype !== Object.prototype && prototype !== null) return value; const record = value as Record; let changed = false; - const next = (isArray ? [] : {}) as Record; + const next = (isArray ? [] : Object.create(prototype)) as Record; for (const [key, entry] of Object.entries(record)) { const scrubbed = (key === "url" || key === "feedUrl") && typeof entry === "string" diff --git a/src/settingsTransfer.ts b/src/settingsTransfer.ts index e27a7d70..1a2bd452 100644 --- a/src/settingsTransfer.ts +++ b/src/settingsTransfer.ts @@ -10,6 +10,7 @@ import { import type { CredentialValues } from "./types/Credentials"; import type { Episode } from "./types/Episode"; import type { IPodNotesSettings } from "./types/IPodNotesSettings"; +import { ownRecord } from "./utility/ownRecord"; import { isLocalFile } from "./utility/isLocalFile"; /** @@ -385,10 +386,9 @@ function normalizeSecrets(values: CredentialValues): CredentialValues { /** Drop any playlist entry that isn't a plain object with an `episodes` array. */ function sanitizePlaylistMap(value: Record): Record { - const out: Record = {}; + const out = ownRecord(); for (const [name, playlist] of Object.entries(value)) { - if (DANGEROUS_KEYS.has(name)) continue; if (!isPlainObject(playlist)) continue; if (!Array.isArray(playlist.episodes)) continue; out[name] = playlist; diff --git a/src/store/downloads.ts b/src/store/downloads.ts index 0fd93a83..244e1204 100644 --- a/src/store/downloads.ts +++ b/src/store/downloads.ts @@ -1,6 +1,7 @@ -import { get, writable } from "svelte/store"; +import { get } from "svelte/store"; import type { Episode } from "src/types/Episode"; import type DownloadedEpisode from "src/types/DownloadedEpisode"; +import { recordStore } from "./recordStore"; /** * The authoritative set of offline-available episodes, keyed by podcast name. @@ -8,7 +9,7 @@ import type DownloadedEpisode from "src/types/DownloadedEpisode"; * Files playlist is a projection of this store (see `localFiles.syncWithDownloaded`). */ export const downloadedEpisodes = (() => { - const store = writable<{ [podcastName: string]: DownloadedEpisode[] }>({}); + const store = recordStore(); const { subscribe, update, set } = store; function isEpisodeDownloaded(episode: Episode): boolean { diff --git a/src/store/feeds.ts b/src/store/feeds.ts index dd5006a0..d75c6693 100644 --- a/src/store/feeds.ts +++ b/src/store/feeds.ts @@ -1,10 +1,11 @@ import { writable } from "svelte/store"; +import { recordStore } from "./recordStore"; import type { PodcastFeed } from "src/types/PodcastFeed"; import { DEFAULT_EPISODE_LIST_LIMIT } from "src/constants"; export { sanitizeEpisodeListLimit } from "src/utility/episodeListLimit"; -export const savedFeeds = writable<{ [podcastName: string]: PodcastFeed }>({}); +export const savedFeeds = recordStore(); /** * How many of each feed's most recent episodes the aggregated "Latest Episodes" diff --git a/src/store/index.ts b/src/store/index.ts index cd80e2ab..23133ddf 100644 --- a/src/store/index.ts +++ b/src/store/index.ts @@ -9,6 +9,7 @@ import type { LocalEpisode } from "src/types/LocalEpisode"; import { LOCAL_FILES_SETTINGS } from "src/constants"; import { getEpisodeKey, isSameEpisode } from "src/utility/episodeKey"; import { getPlayedEpisode, getPlayedEpisodeAliasKeys } from "src/utility/episodeStatus"; +import { recordStore } from "./recordStore"; // `src/store` is the single import surface for the store layer. The saved-feed // and offline downloads stores are self-contained leaf modules; re-export them @@ -20,7 +21,7 @@ export const plugin = writable(); export const hidePlayedEpisodes = writable(false); export const playedEpisodes = (() => { - const store = writable<{ [key: string]: PlayedEpisode }>({}); + const store = recordStore(); const { subscribe, update, set } = store; return { @@ -353,7 +354,7 @@ export const localFiles = (() => { }; })(); -export const playlists = writable<{ [name: string]: Playlist }>({}); +export const playlists = recordStore(); export const podcastView = writable(); export const viewState = (() => { diff --git a/src/store/recordStore.ts b/src/store/recordStore.ts new file mode 100644 index 00000000..37ba560b --- /dev/null +++ b/src/store/recordStore.ts @@ -0,0 +1,15 @@ +import { writable, type Writable } from "svelte/store"; +import { ownRecord } from "src/utility/ownRecord"; + +function own(value: Record): Record { + return Object.getPrototypeOf(value) === null ? value : ownRecord(value); +} + +export function recordStore(): Writable> { + const store = writable(ownRecord()); + return { + subscribe: store.subscribe, + set: (value) => store.set(own(value)), + update: (updater) => store.update((value) => own(updater(value))), + }; +} diff --git a/src/ui/settings/PodNotesSettingsTab.ts b/src/ui/settings/PodNotesSettingsTab.ts index 3e0d5f1a..07708ceb 100644 --- a/src/ui/settings/PodNotesSettingsTab.ts +++ b/src/ui/settings/PodNotesSettingsTab.ts @@ -36,6 +36,7 @@ import type { PodcastFeed } from "src/types/PodcastFeed"; import type { IPodNotesSettings } from "src/types/IPodNotesSettings"; import { get } from "svelte/store"; import { exportOPML, importOPML } from "src/opml"; +import { decodePodNotesData, PODNOTES_DATA_SCHEMA_VERSION } from "src/persistence/podNotesData"; import { protectPrivateFeeds } from "src/services/privateFeeds"; import { describeSecrets, @@ -169,8 +170,14 @@ function restoreFailedImportChanges( currentHasKey === candidateHasKey && (!currentHasKey || settingsValuesEqual(current[key], candidate[key])) ) { - if (previousHasKey) restored[key] = structuredClone(previous[key]); - else delete restored[key]; + if (previousHasKey) { + Object.defineProperty(restored, key, { + value: structuredClone(previous[key]), + enumerable: true, + writable: true, + configurable: true, + }); + } else delete restored[key]; continue; } @@ -960,7 +967,10 @@ export class PodNotesSettingsTab extends PluginSettingTab { settingsValuesEqual(restored, previous) && settingsValuesEqual(previousSettings, previous) ? previousSettings - : restored; + : decodePodNotesData({ + ...restored, + schemaVersion: PODNOTES_DATA_SCHEMA_VERSION, + }).settings; try { // A store event may have queued a newer merged snapshot while the first // write was pending. Queue the rollback after it and wait for durability diff --git a/src/utility/episodeListEntry.ts b/src/utility/episodeListEntry.ts index 73a324e5..5d3e2968 100644 --- a/src/utility/episodeListEntry.ts +++ b/src/utility/episodeListEntry.ts @@ -4,6 +4,7 @@ import { getEpisodeKey } from "src/utility/episodeKey"; import { getFinishedPlayedEpisodeRecords, getPlayedEpisodeRecordKey, + podcastNamesAgree, type PlayedEpisodeMap, } from "src/utility/episodeStatus"; @@ -70,11 +71,13 @@ function resolvePlayedEpisode( playedEpisode: PlayedEpisode, episodeLookup: Map, ): Episode | undefined { - const lookupKeys = [key, getPlayedEpisodeRecordKey(playedEpisode), playedEpisode.title]; + const lookupKeys = [getPlayedEpisodeRecordKey(playedEpisode), key, playedEpisode.title]; for (const lookupKey of lookupKeys) { const episode = episodeLookup.get(lookupKey); - if (episode) return episode; + if (episode && podcastNamesAgree(playedEpisode.podcastName, episode.podcastName)) { + return episode; + } } return undefined; diff --git a/src/utility/episodeStatus.ts b/src/utility/episodeStatus.ts index fdfcc9bd..19ece39b 100644 --- a/src/utility/episodeStatus.ts +++ b/src/utility/episodeStatus.ts @@ -9,22 +9,23 @@ export interface PlayedEpisodeRecord { episode: PlayedEpisode; } +export function podcastNamesAgree(podcastName: string, otherPodcastName: string): boolean { + return !podcastName || !otherPodcastName || podcastName === otherPodcastName; +} + export function getPlayedEpisode( playedEpisodes: PlayedEpisodeMap, episode: Episode | null | undefined, ): PlayedEpisode | undefined { if (!episode) return undefined; - const key = getEpisodeKey(episode); - if (key && playedEpisodes[key]) { - return playedEpisodes[key]; - } - - if (episode.title && playedEpisodes[episode.title]) { - return playedEpisodes[episode.title]; - } - - return undefined; + const own = (key: string) => + Object.prototype.hasOwnProperty.call(playedEpisodes, key) ? playedEpisodes[key] : undefined; + const legacy = own(episode.title); + return ( + own(getEpisodeKey(episode)) ?? + (legacy && podcastNamesAgree(legacy.podcastName, episode.podcastName) ? legacy : undefined) + ); } export function isEpisodeFinished( diff --git a/src/utility/ownRecord.ts b/src/utility/ownRecord.ts new file mode 100644 index 00000000..67e98571 --- /dev/null +++ b/src/utility/ownRecord.ts @@ -0,0 +1,3 @@ +export function ownRecord(entries?: Readonly>): Record { + return Object.assign(Object.create(null) as Record, entries); +}