Skip to content

Commit ecd09d5

Browse files
authored
fix(download): prevent Android crash and create missing folders on download (#178)
Downloading an episode buffered the whole file via requestUrl, wrapped it in a Blob, then round-tripped back to an ArrayBuffer before writing — holding ~3x the episode size in the JS heap. On Android's constrained WebView this OOM- crashed Obsidian mid-download with no file created (#113). Thread a single ArrayBuffer from requestUrl straight into vault.createBinary, dropping the Blob and the Blob->ArrayBuffer round-trip, and read audio-signature bytes from a zero-copy Uint8Array view instead of Blob.slice + FileReader. This removes every full-file copy PodNotes controls (peak ~3x -> ~1x). The remaining peak is inside requestUrl's native base64 decode, which no plugin can reduce, so this is a mitigation rather than a guaranteed cure for the very largest files. Also create missing parent folders before createBinary (mirroring createPodcastNote), so templated paths like podcast/{{podcast}}/{{title}} no longer fail to write (#86), and harden the file-name sanitizer to preserve dots (e.g. "Episode 1.5"), strip control characters, and collapse whitespace globally. Closes #113 Closes #86
1 parent d994d61 commit ecd09d5

6 files changed

Lines changed: 377 additions & 78 deletions

File tree

src/TemplateEngine.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { describe, expect, it } from "vitest";
2+
import { DownloadPathTemplateEngine } from "./TemplateEngine";
3+
import type { Episode } from "./types/Episode";
4+
5+
// The illegal-character sanitizer is private; exercise it through
6+
// DownloadPathTemplateEngine, which applies it to {{title}} and {{podcast}}.
7+
function sanitizeTitle(title: string): string {
8+
const episode = {
9+
title,
10+
streamUrl: "https://example.com/a.mp3",
11+
url: "https://example.com",
12+
description: "",
13+
content: "",
14+
podcastName: "Pod",
15+
episodeDate: undefined,
16+
artworkUrl: "",
17+
} as unknown as Episode;
18+
19+
return DownloadPathTemplateEngine("{{title}}", episode);
20+
}
21+
22+
describe("replaceIllegalFileNameCharactersInString (via DownloadPathTemplateEngine)", () => {
23+
it("preserves dots in titles (no more 'Episode 1.5' -> 'Episode 15')", () => {
24+
expect(sanitizeTitle("Episode 1.5")).toBe("Episode 1.5");
25+
});
26+
27+
it("still strips filesystem-illegal and wikilink-hostile characters", () => {
28+
expect(sanitizeTitle('a/b\\c:d*e?f"g<h>i|j')).toBe("abcdefghij");
29+
expect(sanitizeTitle("a#b%c&d{e}f")).toBe("abcdef");
30+
});
31+
32+
it("replaces every control character with a space (global), not just the first", () => {
33+
expect(sanitizeTitle("a\nb\nc")).toBe("a b c");
34+
expect(sanitizeTitle("a\tb\rc")).toBe("a b c");
35+
});
36+
37+
it("collapses every run of whitespace, not just the first", () => {
38+
expect(sanitizeTitle("a b c")).toBe("a b c");
39+
});
40+
41+
it("drops leading and trailing dots", () => {
42+
expect(sanitizeTitle("...hidden")).toBe("hidden");
43+
expect(sanitizeTitle("trailing...")).toBe("trailing");
44+
});
45+
46+
it("does not strip hyphens or ordinary letters", () => {
47+
expect(sanitizeTitle("Part 1 - The Beginning")).toBe(
48+
"Part 1 - The Beginning",
49+
);
50+
});
51+
});

src/TemplateEngine.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,24 @@ export function TranscriptTemplateEngine(
243243
}
244244

245245
function replaceIllegalFileNameCharactersInString(string: string) {
246-
return string
247-
.replace(/[\\,#%&{}/*<>$'":@\u2023|\\.?]/g, "") // Replace illegal file name characters with empty string
248-
.replace(/\n/, " ") // replace newlines with spaces
249-
.replace(" ", " "); // replace multiple spaces with single space to make sure we don't have double spaces in the file name
246+
return (
247+
string
248+
// Strip characters that are illegal in file names on major platforms,
249+
// plus a few the plugin removes for clean paths/wikilinks. Dots are
250+
// intentionally preserved so titles like "Episode 1.5" are not mangled
251+
// into "Episode 15".
252+
.replace(/[\\,#%&{}/*<>$'":@\u2023|?]/g, "")
253+
// Replace any control characters (newlines, tabs, carriage returns)
254+
// with spaces so they can never end up in a file name.
255+
// eslint-disable-next-line no-control-regex
256+
.replace(/[\u0000-\u001f]/g, " ")
257+
// Collapse every run of whitespace into a single space.
258+
.replace(/\s+/g, " ")
259+
.trim()
260+
// Avoid leading/trailing dots, which create hidden files or "."/".."
261+
// segments and are rejected on Windows/Android.
262+
.replace(/^\.+/, "")
263+
.replace(/\.+$/, "")
264+
.trim()
265+
);
250266
}

src/downloadEpisode.test.ts

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
2+
import { get } from "svelte/store";
3+
import { requestUrl, TFile } from "obsidian";
4+
import {
5+
detectAudioFileExtension,
6+
downloadEpisode,
7+
} from "./downloadEpisode";
8+
import { downloadedEpisodes } from "./store";
9+
import type { Episode } from "./types/Episode";
10+
11+
vi.mock("obsidian", async (importOriginal) => {
12+
const actual = await importOriginal<typeof import("obsidian")>();
13+
return { ...actual, requestUrl: vi.fn() };
14+
});
15+
16+
const requestUrlMock = vi.mocked(requestUrl);
17+
18+
function bytes(...values: number[]): ArrayBuffer {
19+
return new Uint8Array(values).buffer;
20+
}
21+
22+
function setupVault() {
23+
const present = new Set<string>();
24+
const createdFolders: string[] = [];
25+
26+
const createBinary = vi.fn(async (path: string, _data: ArrayBuffer) => {
27+
present.add(path);
28+
return new TFile();
29+
});
30+
const createFolder = vi.fn(async (path: string) => {
31+
present.add(path);
32+
createdFolders.push(path);
33+
});
34+
35+
(globalThis as { app?: unknown }).app = {
36+
vault: {
37+
getAbstractFileByPath: (path: string) =>
38+
present.has(path) ? new TFile() : null,
39+
createBinary,
40+
createFolder,
41+
},
42+
};
43+
44+
return { present, createdFolders, createBinary, createFolder };
45+
}
46+
47+
function makeEpisode(overrides: Partial<Episode> = {}): Episode {
48+
return {
49+
title: "My Title",
50+
streamUrl: "https://example.com/ep.mp3",
51+
url: "https://example.com/ep",
52+
description: "",
53+
content: "",
54+
podcastName: "Pod",
55+
episodeDate: undefined,
56+
artworkUrl: "",
57+
...overrides,
58+
} as unknown as Episode;
59+
}
60+
61+
beforeEach(() => {
62+
requestUrlMock.mockReset();
63+
downloadedEpisodes.set({});
64+
});
65+
66+
afterEach(() => {
67+
(globalThis as { app?: unknown }).app = undefined;
68+
});
69+
70+
describe("detectAudioFileExtension", () => {
71+
it("matches exact signatures (ID3 -> mp3, RIFF -> wav, m4a)", () => {
72+
expect(detectAudioFileExtension(bytes(0x49, 0x44, 0x33, 0x04))).toBe("mp3");
73+
expect(detectAudioFileExtension(bytes(0x52, 0x49, 0x46, 0x46))).toBe("wav");
74+
expect(detectAudioFileExtension(bytes(0x4d, 0x34, 0x41, 0x20))).toBe("m4a");
75+
});
76+
77+
it("applies the masked MPEG frame-sync signature", () => {
78+
expect(detectAudioFileExtension(bytes(0xff, 0xfb, 0x90, 0x00))).toBe("mp3");
79+
});
80+
81+
it("returns null for unknown content", () => {
82+
expect(detectAudioFileExtension(bytes(0x00, 0x01, 0x02, 0x03))).toBeNull();
83+
});
84+
85+
it("does not crash on a buffer shorter than the longest signature", () => {
86+
expect(detectAudioFileExtension(bytes(0xff))).toBeNull();
87+
expect(detectAudioFileExtension(new ArrayBuffer(0))).toBeNull();
88+
});
89+
});
90+
91+
describe("downloadEpisode (API path)", () => {
92+
it("threads a single ArrayBuffer straight to createBinary (no Blob copy) and creates folders", async () => {
93+
const { createBinary, createdFolders } = setupVault();
94+
const buffer = bytes(0x49, 0x44, 0x33, 0x01, 0x02, 0x03);
95+
requestUrlMock.mockResolvedValue({
96+
status: 200,
97+
headers: { "content-type": "audio/mpeg" },
98+
arrayBuffer: buffer,
99+
} as unknown as Awaited<ReturnType<typeof requestUrl>>);
100+
101+
const episode = makeEpisode();
102+
const path = await downloadEpisode(episode, "podcast/{{podcast}}/{{title}}");
103+
104+
expect(path).toBe("podcast/Pod/My Title.mp3");
105+
expect(createdFolders).toEqual(["podcast", "podcast/Pod"]);
106+
expect(createBinary).toHaveBeenCalledTimes(1);
107+
108+
const [writtenPath, writtenData] = createBinary.mock.calls[0];
109+
expect(writtenPath).toBe("podcast/Pod/My Title.mp3");
110+
// The exact buffer returned by requestUrl must reach createBinary — proving
111+
// no Blob round-trip / extra full-file copy is made (issue #113).
112+
expect(writtenData).toBe(buffer);
113+
114+
const recorded = get(downloadedEpisodes)["Pod"]?.[0];
115+
expect(recorded?.filePath).toBe("podcast/Pod/My Title.mp3");
116+
expect(recorded?.size).toBe(buffer.byteLength);
117+
});
118+
119+
it("returns the existing path without downloading when the file already exists", async () => {
120+
const { present, createBinary } = setupVault();
121+
present.add("My Title.mp3");
122+
123+
const path = await downloadEpisode(makeEpisode(), "{{title}}");
124+
125+
expect(path).toBe("My Title.mp3");
126+
expect(requestUrlMock).not.toHaveBeenCalled();
127+
expect(createBinary).not.toHaveBeenCalled();
128+
});
129+
});

0 commit comments

Comments
 (0)