fix(sources): clean URL-shaped fragments; one registry for busy sources

- A fragment is cleaned whatever its shape: `#https://joe:pw@…` loses its
  userinfo, an unclassifiable secret is dropped, and the name check reads
  a URL in the fragment as well as a parameter list.
- Every refresh holds its source in the root SourceActivityService, which
  deletes and the startup auto-refresh already use: Sources-row refreshes,
  the switcher's M3U refresh and the shared Xtream flow. The switcher no
  longer offers Refresh or Remove for a source the page is refreshing, the
  removal service ignores a held source, a second refresh from either
  entry point is skipped, and the page disables Remove while it is held.
- The long-input timing spec allows 3 s: the inputs take about 0.25 s,
  a backtracking pattern takes minutes, and a loaded runner took 1.6 s.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 5.5 committed 2026-10-13 23:18:58 +02:00
1 parent d2b0325138
commit a861ef94ac
19 files changed
+188 -40

No files matched your search

+5 -4
View File
@@ -1391,10 +1391,11 @@ in `@iptvnator/playlist/shared/util` (`source-display`, `source-format`,
switcher closes the popover first, since its focus trap would keep the
keyboard from Undo: Undo takes the focus and returns it to the trigger
(`SourceRemovalOptions.returnFocusTo`). A source cannot be removed while
a refresh clears and refills it: Remove is disabled in both lists and the
service ignores it (`PlaylistRefreshActionService.isSourceBusy`), and the
Sources page shows a switcher refresh as busy. Bulk cleanup keeps its
confirmation dialog.
a refresh clears and refills it: every refresh (a Sources row, the
switcher, the Xtream flow, the startup auto-refresh) and every delete
holds the source in the root `SourceActivityService`, which both lists,
the removal service and a second refresh consult. The Sources page shows
a switcher refresh as busy. Bulk cleanup keeps its confirmation dialog.
- **Side list.** Type and, on desktop, Status (Any / Responding / Not
responding) combine and persist (`SourcesViewStateService`); each group
counts within the other's choice. The page checks every source, not only
@@ -349,13 +349,17 @@ describe('PlaylistRefreshActionService', () => {
resolve = r;
})
);
const activity = TestBed.inject(SourceActivityService);
service.refresh(playlist);
expect(service.isSourceBusy('busy')).toBe(true);
expect(service.isSourceBusy('other')).toBe(false);
// The Sources page and the removal guard see it too.
expect(activity.isBusy('busy')).toBe(true);
resolve({ _id: 'busy', playlist: { items: [] } } as Playlist);
await Promise.resolve();
await Promise.resolve();
expect(service.isSourceBusy('busy')).toBe(false);
expect(activity.isBusy('busy')).toBe(false);
});
it('passes trusted TLS hosts to URL-backed M3U refreshes', async () => {
@@ -148,6 +148,8 @@ export class PlaylistRefreshActionService {
this.m3uRefreshId.set(item._id);
this.isRefreshing.set(true);
// Seen by the Sources page and the removal guard as well.
const release = this.activity.begin([item._id]);
if (isActiveM3uRoute) {
this.store.dispatch(
ChannelActions.setChannelsLoading({ loading: true })
@@ -214,6 +216,7 @@ export class PlaylistRefreshActionService {
}
this.m3uRefreshId.set(null);
this.isRefreshing.set(false);
release();
}
}
@@ -157,7 +157,7 @@
[current]="option.playlist._id === currentId()"
[highlighted]="index === highlight()"
[canRefresh]="canRefresh(option.playlist)"
[removable]="!isRefreshing(option.playlist)"
[removable]="!isBusy(option.playlist)"
(activated)="select(option.playlist)"
(refreshRequested)="refresh(option.playlist)"
(playlistInfoRequested)="openInfo(option.playlist)"
@@ -15,6 +15,7 @@ import {
SortOptions,
SortOrder,
SortService,
SourceActivityService,
} from '@iptvnator/services';
import { PlaylistMeta } from '@iptvnator/shared/interfaces';
import { PlaylistRefreshActionService } from '../playlist-refresh-action.service';
@@ -51,7 +52,6 @@ describe('PlaylistSwitcherPanelComponent', () => {
let selectPlaylist: jest.Mock;
let remove: jest.Mock;
let navigate: jest.Mock;
let refreshing: ReturnType<typeof signal<string | null>>;
const input = () =>
(fixture.nativeElement as HTMLElement).querySelector(
@@ -83,7 +83,6 @@ describe('PlaylistSwitcherPanelComponent', () => {
selectPlaylist = jest.fn();
remove = jest.fn();
navigate = jest.fn();
refreshing = signal<string | null>(null);
TestBed.configureTestingModule({
imports: [
PlaylistSwitcherPanelComponent,
@@ -138,7 +137,6 @@ describe('PlaylistSwitcherPanelComponent', () => {
useValue: {
canRefresh: () => true,
refresh: jest.fn(),
isSourceBusy: (id: string) => refreshing() === id,
},
},
{ provide: MatDialog, useValue: { open: jest.fn() } },
@@ -288,7 +286,8 @@ describe('PlaylistSwitcherPanelComponent', () => {
});
it('offers no Remove while a refresh clears and refills the source', async () => {
refreshing.set('bravo');
// Held by a refresh from any entry point, the Sources page included.
const release = TestBed.inject(SourceActivityService).begin(['bravo']);
fixture.detectChanges();
const menuRemove = async (id: string) => {
(fixture.nativeElement as HTMLElement)
@@ -318,6 +317,7 @@ describe('PlaylistSwitcherPanelComponent', () => {
)
?.click();
expect(selectPlaylist).toHaveBeenCalled();
release();
});
it('leaves out a source whose delete is pending or running', () => {
@@ -32,7 +32,11 @@ import {
sourceSortName,
SourcesTypeFilter,
} from '@iptvnator/playlist/shared/util';
import { SortBy, SortService } from '@iptvnator/services';
import {
SortBy,
SortService,
SourceActivityService,
} from '@iptvnator/services';
import {
PlaylistMeta,
PlaylistSourceKind,
@@ -76,6 +80,7 @@ const KINDS: readonly PlaylistSourceKind[] = ['m3u', 'xtream', 'stalker'];
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class PlaylistSwitcherPanelComponent {
private readonly activity = inject(SourceActivityService);
private readonly context = inject(PlaylistContextFacade);
private readonly document = inject(DOCUMENT);
private readonly dialog = inject(MatDialog);
@@ -284,8 +289,8 @@ export class PlaylistSwitcherPanelComponent {
}
/** A source cannot be removed while a refresh clears and refills it. */
isRefreshing(playlist: PlaylistMeta): boolean {
return this.refreshAction.isSourceBusy(playlist._id);
isBusy(playlist: PlaylistMeta): boolean {
return this.activity.isBusy(playlist._id);
}
refresh(playlist: PlaylistMeta): void {
@@ -37,6 +37,7 @@
[draggable]="isCustomSort()"
[canRefresh]="refresh.canRefresh(item)"
[refreshing]="isRefreshing(item)"
[removable]="isRemovable(item)"
[deleting]="removal.progress.isDeleting(item._id)"
[busyMessage]="busyMessage(item)"
[busyProgress]="busyProgress(item)"
@@ -1,5 +1,6 @@
import { signal } from '@angular/core';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { By } from '@angular/platform-browser';
import { MatDialog } from '@angular/material/dialog';
import { Router } from '@angular/router';
import { MockStore, provideMockStore } from '@ngrx/store/testing';
@@ -21,10 +22,12 @@ import {
SortOptions,
SortOrder,
SortService,
SourceActivityService,
} from '@iptvnator/services';
import { PlaylistMeta, SourceStatus } from '@iptvnator/shared/interfaces';
import { PlaylistRefreshActionService } from '../playlist-refresh-action.service';
import { SourceRemovalService } from '../source-removal/source-removal.service';
import { SourceRowComponent } from '../source-row/source-row.component';
import { RecentPlaylistsComponent } from './recent-playlists.component';
import { SourceListRefreshService } from './source-list-refresh.service';
@@ -285,6 +288,21 @@ describe('RecentPlaylistsComponent', () => {
).toBe(true);
});
it('offers no Remove while something else holds a source', () => {
const removable = () =>
fixture.debugElement
.queryAll(By.directive(SourceRowComponent))
.map((row) =>
(row.componentInstance as SourceRowComponent).removable()
);
expect(removable()).toEqual([true, true, true]);
// The startup auto-refresh, or a refresh from the switcher.
const release = TestBed.inject(SourceActivityService).begin(['m3u']);
fixture.detectChanges();
expect(removable()).toEqual([false, true, true]);
release();
});
it('hides sources whose delete is pending', () => {
pending.set(new Set(['m3u']));
expect(ids()).toEqual(['xtream', 'file']);
@@ -40,6 +40,7 @@ import {
RuntimeCapabilitiesService,
SortBy,
SortService,
SourceActivityService,
} from '@iptvnator/services';
import { PlaylistMeta, sourceStatusBucket } from '@iptvnator/shared/interfaces';
import type { PlaylistType } from '../add-playlist-menu/playlist-type';
@@ -82,6 +83,7 @@ export class RecentPlaylistsComponent {
readonly removal = inject(SourceRemovalService);
readonly refresh = inject(SourceListRefreshService);
private readonly refreshAction = inject(PlaylistRefreshActionService);
private readonly activity = inject(SourceActivityService);
readonly searchQueryInput = input<string>('');
readonly playlistClicked = output<string>();
@@ -228,6 +230,11 @@ export class RecentPlaylistsComponent {
);
}
/** Nothing else (a refresh, the startup auto-refresh) holds it. */
isRemovable(item: PlaylistMeta): boolean {
return !this.activity.isBusy(item._id);
}
busyMessage(item: PlaylistMeta): string {
return (
this.removal.progress.message(item._id) ||
@@ -24,6 +24,7 @@ import {
SortBy,
SortOrder,
SortService,
SourceActivityService,
} from '@iptvnator/services';
import {
CONNECTIVITY_GUARD_RESET,
@@ -502,6 +503,36 @@ describe('SourceListRefreshService', () => {
expect(playlistRefreshService.refreshPlaylist).not.toHaveBeenCalled();
});
it('holds a row refresh in the shared registry, and leaves a held source alone', async () => {
const activity = TestBed.inject(SourceActivityService);
const item = createPlaylistMeta({
_id: 'playlist-m3u',
serverUrl: undefined,
username: undefined,
password: undefined,
url: 'https://source.test/list.m3u',
});
let resolve!: (value: unknown) => void;
playlistRefreshService.refreshPlaylist.mockReturnValueOnce(
new Promise((r) => {
resolve = r;
})
);
component.refresh(item);
// The switcher and the removal guard see the row's refresh.
expect(activity.isBusy(item._id)).toBe(true);
resolve({ id: item._id, items: [] });
await Promise.resolve();
await Promise.resolve();
expect(activity.isBusy(item._id)).toBe(false);
// A refresh or delete started elsewhere holds it: no second run.
const release = activity.begin([item._id]);
component.refresh(item);
expect(playlistRefreshService.refreshPlaylist).toHaveBeenCalledTimes(1);
release();
});
it('re-evaluates refresh bridge availability when refreshing local M3U playlists', async () => {
runtime.supportsPlaylistRefresh = false;
const lateComponent = TestBed.inject(SourceListRefreshService);
@@ -11,6 +11,7 @@ import {
isDbAbortError,
PlaylistRefreshService,
RuntimeCapabilitiesService,
SourceActivityService,
} from '@iptvnator/services';
import {
PLAYLIST_UPDATE,
@@ -57,6 +58,10 @@ export class SourceListRefreshService {
private readonly dataService = inject(DataService);
private readonly playlistRefreshService = inject(PlaylistRefreshService);
private readonly runtime = inject(RuntimeCapabilitiesService);
// Root registry: the switcher, the removal guard and cleanup consult it,
// so a row refresh must not be visible only to this page.
private readonly activity = inject(SourceActivityService);
private readonly releases = new Map<string, () => void>();
private readonly snackBar = inject(MatSnackBar);
private readonly store = inject(Store);
private readonly translate = inject(TranslateService);
@@ -83,7 +88,7 @@ export class SourceListRefreshService {
}
refresh(item: PlaylistMeta): void {
if (this.isRefreshing(item._id)) {
if (this.isRefreshing(item._id) || this.activity.isBusy(item._id)) {
return;
}
if (item.serverUrl && this.runtime.supportsXtreamSqliteDataSource) {
@@ -166,7 +171,8 @@ export class SourceListRefreshService {
/** The shared Xtream re-import reports into this list's row state. */
private readonly xtreamReporter: XtreamRefreshProgressReporter = {
isBusy: (playlistId) => this.isRefreshing(playlistId),
isBusy: (playlistId) =>
this.isRefreshing(playlistId) || this.activity.isBusy(playlistId),
begin: ({ playlistId }) => this.setRefreshing(playlistId, true),
report: ({ playlistId }, event) => this.track(playlistId, event),
end: ({ playlistId }) => {
@@ -283,6 +289,12 @@ export class SourceListRefreshService {
}
private setRefreshing(id: string, refreshing: boolean): void {
if (refreshing && !this.releases.has(id)) {
this.releases.set(id, this.activity.begin([id]));
} else if (!refreshing) {
this.releases.get(id)?.();
this.releases.delete(id);
}
this.refreshingIds.update((current) => {
const next = new Set(current);
if (refreshing) {
@@ -11,13 +11,13 @@ import {
DatabaseService,
PlaylistDeleteActionService,
RuntimeCapabilitiesService,
SourceActivityService,
} from '@iptvnator/services';
import { PlaylistMeta } from '@iptvnator/shared/interfaces';
import {
RENDERER_PERFORMANCE_PHASE_HOOK_KEY,
type RendererPerformancePhaseEvent,
} from '@iptvnator/shared/logging';
import { PlaylistRefreshActionService } from '../playlist-refresh-action.service';
import {
SOURCE_REMOVAL_UNDO_MS,
SourceRemovalService,
@@ -52,7 +52,6 @@ describe('SourceRemovalService', () => {
};
let router: { url: string; navigate: jest.Mock; navigateByUrl: jest.Mock };
let service: SourceRemovalService;
let refreshing: Set<string>;
const toast = (message: string, data: { commitNow?: () => void } = {}) => {
const dismissed = new Subject<void>();
@@ -85,7 +84,6 @@ describe('SourceRemovalService', () => {
deletePlaylist = jest.fn().mockResolvedValue(true);
cancelOperation = jest.fn().mockResolvedValue(true);
dispatch = jest.fn();
refreshing = new Set();
resolvedPlaylistId = signal<string | null>(null);
routePlaylistId = signal<string | null>(null);
context = {
@@ -134,12 +132,6 @@ describe('SourceRemovalService', () => {
provide: RuntimeCapabilitiesService,
useValue: { supportsXtreamSqliteDataSource: true },
},
{
provide: PlaylistRefreshActionService,
useValue: {
isSourceBusy: (id: string) => refreshing.has(id),
},
},
],
});
service = TestBed.inject(SourceRemovalService);
@@ -165,13 +157,15 @@ describe('SourceRemovalService', () => {
});
it('does not remove a source while a refresh clears and refills it', async () => {
refreshing.add('a');
// Any entry point's refresh, or the startup auto-refresh.
const release = TestBed.inject(SourceActivityService).begin(['a']);
service.remove(playlist('a'));
expireUndo();
await flush();
expect(service.isPending('a')).toBe(false);
expect(toasts).toHaveLength(0);
expect(deletePlaylist).not.toHaveBeenCalled();
release();
});
it('restores the source on Undo without deleting anything', async () => {
@@ -11,9 +11,9 @@ import {
import {
PlaylistDeleteActionService,
RuntimeCapabilitiesService,
SourceActivityService,
} from '@iptvnator/services';
import { PlaylistMeta } from '@iptvnator/shared/interfaces';
import { PlaylistRefreshActionService } from '../playlist-refresh-action.service';
import { SourceDeleteProgressService } from './source-delete-progress.service';
import { SourceRemovalToastComponent } from './source-removal-toast.component';
import {
@@ -67,7 +67,7 @@ interface PendingRemoval {
export class SourceRemovalService {
private readonly deleteAction = inject(PlaylistDeleteActionService);
private readonly playlistContext = inject(PlaylistContextFacade);
private readonly refreshAction = inject(PlaylistRefreshActionService);
private readonly activity = inject(SourceActivityService);
private readonly router = inject(Router);
private readonly runtime = inject(RuntimeCapabilitiesService);
private readonly snackBar = inject(MatSnackBar);
@@ -94,11 +94,11 @@ export class SourceRemovalService {
next: PlaylistMeta | null = null,
options: SourceRemovalOptions = {}
): void {
// A refresh started from the switcher is destructive too (it clears
// the catalog, then re-imports it): the two must not overlap.
// A refresh from any entry point (or the startup auto-refresh) is
// destructive too: it clears the catalog, then re-imports it.
if (
this.isPending(playlist._id) ||
this.refreshAction.isSourceBusy(playlist._id)
this.activity.isBusy(playlist._id)
) {
return;
}
@@ -9,6 +9,7 @@ import {
DatabaseService,
DbOperationEvent,
PlaybackPositionService,
SourceActivityService,
} from '@iptvnator/services';
import {
CONNECTIVITY_GUARD_RESET,
@@ -168,6 +169,38 @@ describe('XtreamRefreshFlowService', () => {
expect(dataService.sendIpcEvent).not.toHaveBeenCalled();
});
it('holds the source in the shared registry for the run, and skips a held one', async () => {
const activity = TestBed.inject(SourceActivityService);
const item = createPlaylistMeta();
let heldDuringDelete = false;
databaseService.deleteXtreamPlaylistContent.mockImplementationOnce(
() => {
heldDuringDelete = activity.isBusy(item._id);
return Promise.resolve({
success: true,
favorites: [],
recentlyViewed: [],
hiddenCategories: [],
});
}
);
service.confirmAndRefresh(item, createReporter().reporter);
await confirmPromise;
expect(heldDuringDelete).toBe(true);
expect(activity.isBusy(item._id)).toBe(false);
// A delete or another refresh already holds it: nothing runs.
const release = activity.begin([item._id]);
const second = createReporter();
service.confirmAndRefresh(item, second.reporter);
await confirmPromise;
expect(second.calls).toEqual([]);
expect(
databaseService.deleteXtreamPlaylistContent
).toHaveBeenCalledTimes(1);
release();
});
it('serializes destructive runs for one playlist across entry points', async () => {
const item = createPlaylistMeta();
const confirms: Array<Promise<void> | undefined> = [];
@@ -11,6 +11,7 @@ import {
isDbAbortError,
PlaybackPositionService,
resetHostConnectivityGuard,
SourceActivityService,
XtreamPendingRestoreService,
} from '@iptvnator/services';
import { PlaylistActions } from '@iptvnator/m3u-state';
@@ -79,6 +80,7 @@ export class XtreamRefreshFlowService {
private readonly pendingRestoreService = inject(
XtreamPendingRestoreService
);
private readonly activity = inject(SourceActivityService);
/**
* Playlists with a destructive run in flight, across every entry point.
@@ -119,8 +121,11 @@ export class XtreamRefreshFlowService {
item: PlaylistMeta,
reporter: XtreamRefreshProgressReporter
): Promise<void> {
// The shared registry also holds deletes, the startup auto-refresh
// and M3U refreshes from either entry point.
if (
this.activeRefreshPlaylistIds.has(item._id) ||
this.activity.isBusy(item._id) ||
reporter.isBusy(item._id)
) {
return;
@@ -131,6 +136,7 @@ export class XtreamRefreshFlowService {
const run: XtreamRefreshRun = { playlistId: item._id, operationId };
reporter.begin(run);
this.activeRefreshPlaylistIds.add(item._id);
const release = this.activity.begin([item._id]);
try {
// Before anything destructive: this deletes the cached catalog and
@@ -205,6 +211,7 @@ export class XtreamRefreshFlowService {
}
} finally {
this.activeRefreshPlaylistIds.delete(item._id);
release();
reporter.end(run);
}
}
@@ -110,6 +110,14 @@ describe('sourceDisplayName', () => {
).toBe('host.test');
});
it('drops a name holding a secret of a URL in the fragment', () => {
const url = 'https://proxy.test/list#https://joe:hunter2@origin.test/a';
for (const title of ['Mine hunter2', 'Mine joe']) {
expect(sourceDisplayName({ title, url })).toBe('proxy.test');
}
expect(sourceDisplayName({ title: 'Mine', url })).toBe('Mine');
});
it('keeps a playlist file name from a path that names no account', () => {
const url = 'https://cdn.example/live/news/list.m3u';
expect(sourceDisplayName({ title: 'list.m3u', url })).toBe('list.m3u');
@@ -132,10 +132,8 @@ function collectUrlSecrets(
visitNested(segment);
});
visit(url.searchParams);
const fragment = url.hash.slice(1);
if (fragment.includes('=')) {
visit(new URLSearchParams(fragment));
}
// A fragment may hold a parameter list or a whole URL.
visitNested(url.hash.slice(1));
}
/**
@@ -83,6 +83,28 @@ describe('stripUrlCredentials', () => {
expect(stripUrlCredentials(nested)).not.toMatch(/hunter2|joe/);
});
it('cleans a fragment that holds a URL rather than parameters', () => {
expect(
stripUrlCredentials(
'https://proxy.test/list#https://joe:hunter2@origin.test/a.m3u'
)
).toBe('https://proxy.test/list#https://origin.test/a.m3u');
expect(
stripUrlCredentials(
`https://proxy.test/list#${encodeURIComponent(
'https://joe:hunter2@origin.test/a.m3u'
)}`
)
).not.toMatch(/hunter2|joe/);
expect(
stripUrlCredentials('https://proxy.test/list#joe:hunter2@origin')
).toBe('https://proxy.test/list');
// A plain anchor stays as written.
expect(stripUrlCredentials('https://host.test/list.m3u#part-2')).toBe(
'https://host.test/list.m3u#part-2'
);
});
it('keeps a playlist path that only starts like an Xtream stream', () => {
// No stream follows `/news/list.m3u`: it names no account.
expect(stripUrlCredentials('https://cdn.test/live/news/list.m3u')).toBe(
@@ -163,7 +185,9 @@ describe('stripUrlCredentials', () => {
`https://host.test/list?note=${encodeURIComponent(value)}`
);
}
expect(performance.now() - started).toBeLessThan(1_000);
// About 0.25 s in total when idle; a backtracking pattern takes
// minutes. The margin absorbs a loaded CI runner.
expect(performance.now() - started).toBeLessThan(3_000);
});
it('returns null for text that is not an absolute URL', () => {
@@ -169,13 +169,15 @@ function stripUrl(value: string, depth: number): string | null {
const search = withoutSensitiveParams(url.searchParams, depth);
url.search = search ? `?${search}` : '';
// Any fragment, not only a parameter list: `#https://joe:pw@…` is a URL.
const fragment = url.hash.slice(1);
if (looksLikeSearchParams(fragment)) {
const hash = withoutSensitiveParams(
new URLSearchParams(fragment),
depth
);
url.hash = hash ? `#${hash}` : '';
if (fragment) {
const clean = sanitizeNested(fragment, depth);
if (!clean) {
url.hash = '';
} else if (clean !== fragment) {
url.hash = clean;
}
}
return url.toString();