fix: blank PUBLIC_HOST_AUDIO_URL no longer breaks audio download links
A present-but-empty PUBLIC_HOST_AUDIO_URL (e.g. `PUBLIC_HOST_AUDIO_URL=` in a
compose file) bypassed the default and stayed blank, so the UI built
root-relative audio links like `app.url/song.mp3` that 404'd, while video links
kept working off PUBLIC_HOST_URL.
Backend: a blank PUBLIC_HOST_AUDIO_URL now inherits PUBLIC_HOST_URL, matching the
README ("same as PUBLIC_HOST_URL but for audio downloads"). Both blank still
stays blank, so intentional root serving is preserved.
Frontend: buildDownloadLink and buildChapterDownloadLink fall back to
PUBLIC_HOST_URL when the audio host is blank.
Added backend and UI regression tests, both verified red against the unfixed
code.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
5429200fba
commit
2598f80835
4 changed files with 90 additions and 2 deletions
4
app/main.py
Normal file → Executable file
4
app/main.py
Normal file → Executable file
|
|
@ -117,6 +117,10 @@ class Config:
|
|||
if val and not val.endswith('/'):
|
||||
setattr(self, attr, val + '/')
|
||||
|
||||
# A blank PUBLIC_HOST_AUDIO_URL inherits PUBLIC_HOST_URL so audio links aren't left root-relative; both blank stays blank.
|
||||
if not self.PUBLIC_HOST_AUDIO_URL:
|
||||
self.PUBLIC_HOST_AUDIO_URL = self.PUBLIC_HOST_URL
|
||||
|
||||
# Convert relative addresses to absolute addresses to prevent the failure of file address comparison
|
||||
if self.YTDL_OPTIONS_FILE and self.YTDL_OPTIONS_FILE.startswith('.'):
|
||||
self.YTDL_OPTIONS_FILE = str(Path(self.YTDL_OPTIONS_FILE).resolve())
|
||||
|
|
|
|||
|
|
@ -51,6 +51,43 @@ class ConfigTests(unittest.TestCase):
|
|||
self.assertEqual(c.PUBLIC_HOST_URL, "")
|
||||
self.assertEqual(c.PUBLIC_HOST_AUDIO_URL, "")
|
||||
|
||||
def test_blank_audio_host_inherits_public_host_url(self):
|
||||
# Regression: a blank PUBLIC_HOST_AUDIO_URL must inherit PUBLIC_HOST_URL, not stay root-relative.
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
_base_env(PUBLIC_HOST_URL="download/", PUBLIC_HOST_AUDIO_URL=""),
|
||||
clear=False,
|
||||
):
|
||||
c = Config()
|
||||
self.assertEqual(c.PUBLIC_HOST_URL, "download/")
|
||||
self.assertEqual(c.PUBLIC_HOST_AUDIO_URL, "download/")
|
||||
|
||||
def test_blank_audio_host_audio_link_is_not_root_relative(self):
|
||||
# Repro of the reported bug. buildDownloadLink uses configuration[PUBLIC_HOST_AUDIO_URL]
|
||||
# as the base for audio, so a blank value made the link "song.mp3" (root-relative, 404)
|
||||
# while video kept working off PUBLIC_HOST_URL. The base must be prefixed instead.
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
_base_env(PUBLIC_HOST_URL="download/", PUBLIC_HOST_AUDIO_URL=""),
|
||||
clear=False,
|
||||
):
|
||||
c = Config()
|
||||
audio_link = c.PUBLIC_HOST_AUDIO_URL + "song.mp3"
|
||||
video_link = c.PUBLIC_HOST_URL + "video.mp4"
|
||||
self.assertEqual(video_link, "download/video.mp4")
|
||||
self.assertNotEqual(audio_link, "song.mp3")
|
||||
self.assertEqual(audio_link, "download/song.mp3")
|
||||
|
||||
def test_unset_audio_host_keeps_default_route(self):
|
||||
# Truly unset (not blank) keeps the 'audio_download/' default route.
|
||||
env = _base_env()
|
||||
env.pop("PUBLIC_HOST_AUDIO_URL", None)
|
||||
env.pop("PUBLIC_HOST_URL", None)
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
c = Config()
|
||||
self.assertEqual(c.PUBLIC_HOST_URL, "download/")
|
||||
self.assertEqual(c.PUBLIC_HOST_AUDIO_URL, "audio_download/")
|
||||
|
||||
def test_public_host_url_already_slashed_unchanged(self):
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
|
|
|
|||
|
|
@ -262,6 +262,49 @@ describe('App', () => {
|
|||
expect(payload.clipEnd).toBe('1:20');
|
||||
});
|
||||
|
||||
function makeDownload(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
download_type: 'audio',
|
||||
filename: 'song.mp3',
|
||||
folder: '',
|
||||
title: 'Song',
|
||||
...overrides,
|
||||
} as unknown as Parameters<App['buildDownloadLink']>[0];
|
||||
}
|
||||
|
||||
it('builds audio link from PUBLIC_HOST_AUDIO_URL when set', () => {
|
||||
downloads.configuration['PUBLIC_HOST_URL'] = 'download/';
|
||||
downloads.configuration['PUBLIC_HOST_AUDIO_URL'] = 'audio_download/';
|
||||
const app = TestBed.createComponent(App).componentInstance;
|
||||
expect(app.buildDownloadLink(makeDownload())).toBe('audio_download/song.mp3');
|
||||
});
|
||||
|
||||
it('builds video link from PUBLIC_HOST_URL', () => {
|
||||
downloads.configuration['PUBLIC_HOST_URL'] = 'download/';
|
||||
downloads.configuration['PUBLIC_HOST_AUDIO_URL'] = 'audio_download/';
|
||||
const app = TestBed.createComponent(App).componentInstance;
|
||||
const link = app.buildDownloadLink(makeDownload({ download_type: 'video', filename: 'video.mp4' }));
|
||||
expect(link).toBe('download/video.mp4');
|
||||
});
|
||||
|
||||
it('audio link falls back to PUBLIC_HOST_URL when audio host is blank (regression)', () => {
|
||||
// The reported bug: a blank PUBLIC_HOST_AUDIO_URL produced a root-relative
|
||||
// "song.mp3" that 404'd while video kept working. It must not be root-relative.
|
||||
downloads.configuration['PUBLIC_HOST_URL'] = 'download/';
|
||||
downloads.configuration['PUBLIC_HOST_AUDIO_URL'] = '';
|
||||
const app = TestBed.createComponent(App).componentInstance;
|
||||
const link = app.buildDownloadLink(makeDownload());
|
||||
expect(link).not.toBe('song.mp3');
|
||||
expect(link).toBe('download/song.mp3');
|
||||
});
|
||||
|
||||
it('audio link stays root-relative when both hosts are blank', () => {
|
||||
downloads.configuration['PUBLIC_HOST_URL'] = '';
|
||||
downloads.configuration['PUBLIC_HOST_AUDIO_URL'] = '';
|
||||
const app = TestBed.createComponent(App).componentInstance;
|
||||
expect(app.buildDownloadLink(makeDownload())).toBe('song.mp3');
|
||||
});
|
||||
|
||||
it('blocks subscribe with invalid title regex', () => {
|
||||
const alertSpy = vi.spyOn(window, 'alert').mockImplementation(() => undefined);
|
||||
const fixture = TestBed.createComponent(App);
|
||||
|
|
|
|||
|
|
@ -1203,7 +1203,9 @@ export class App implements AfterViewInit, OnInit, OnDestroy {
|
|||
buildDownloadLink(download: Download) {
|
||||
let baseDir = this.downloads.configuration["PUBLIC_HOST_URL"];
|
||||
if (download.download_type === 'audio' || download.filename.endsWith('.mp3')) {
|
||||
baseDir = this.downloads.configuration["PUBLIC_HOST_AUDIO_URL"];
|
||||
// Fall back to PUBLIC_HOST_URL when the audio host is blank, else the link goes root-relative and 404s.
|
||||
baseDir = this.downloads.configuration["PUBLIC_HOST_AUDIO_URL"]
|
||||
|| this.downloads.configuration["PUBLIC_HOST_URL"];
|
||||
}
|
||||
|
||||
if (download.folder) {
|
||||
|
|
@ -1299,7 +1301,9 @@ export class App implements AfterViewInit, OnInit, OnDestroy {
|
|||
buildChapterDownloadLink(download: Download, chapterFilename: string) {
|
||||
let baseDir = this.downloads.configuration["PUBLIC_HOST_URL"];
|
||||
if (download.download_type === 'audio' || chapterFilename.endsWith('.mp3')) {
|
||||
baseDir = this.downloads.configuration["PUBLIC_HOST_AUDIO_URL"];
|
||||
// Fall back to PUBLIC_HOST_URL when the audio host is blank (see buildDownloadLink).
|
||||
baseDir = this.downloads.configuration["PUBLIC_HOST_AUDIO_URL"]
|
||||
|| this.downloads.configuration["PUBLIC_HOST_URL"];
|
||||
}
|
||||
|
||||
if (download.folder) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue