fix(sources): hand over only to an idle source; hide names that are a short secret

- Removing the active source could hand over to a neighbour a refresh
  was still clearing, opening it and starting its import early.
  `SourceRemovalService.canTakeOver` (not leaving, not deleting, not held
  in SourceActivityService) now picks the replacement in both lists and
  gates it in the service.
- Secrets under three characters were dropped from the check entirely,
  so a name that was just a two-letter password showed it. Short secrets
  now count as the whole text (trimmed, any case); longer ones still
  match inside a text.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 5.5 committed 2026-10-14 01:45:55 +02:00
1 parent ecddc518ba
commit ea3dacb052
8 files changed
+95 -27

No files matched your search

@@ -133,6 +133,9 @@ describe('PlaylistSwitcherPanelComponent', () => {
progress: {
isDeleting: (id: string) => deleting().has(id),
},
canTakeOver: (id: string) =>
!deleting().has(id) &&
!TestBed.inject(SourceActivityService).isBusy(id),
},
},
{
@@ -348,6 +351,16 @@ describe('PlaylistSwitcherPanelComponent', () => {
expect(selectPlaylist).toHaveBeenCalledWith(ALPHA);
});
it('hands over past a source a refresh holds when removing the current one', () => {
// Bravo is current; Charlie would follow it but is refreshing.
const release = TestBed.inject(SourceActivityService).begin([
'charlie',
]);
component.remove(BRAVO);
expect(remove).toHaveBeenCalledWith(BRAVO, ALPHA, expect.anything());
release();
});
it('leaves out a source whose delete is pending or running', () => {
pending.set(new Set(['bravo']));
fixture.detectChanges();
@@ -334,7 +334,7 @@ export class PlaylistSwitcherPanelComponent {
const next = replacementAfterRemoval(
this.groups().all.map((option) => option.playlist),
playlist._id,
(row) => this.removal.progress.isDeleting(row._id)
(row) => !this.removal.canTakeOver(row._id)
);
this.close();
// Closing put the focus back on the trigger.
@@ -63,6 +63,7 @@ describe('RecentPlaylistsComponent', () => {
pendingIds: typeof pending;
remove: jest.Mock;
isPending: jest.Mock;
canTakeOver: jest.Mock;
progress: object;
};
let viewState: SourcesViewStateService;
@@ -84,6 +85,15 @@ describe('RecentPlaylistsComponent', () => {
pendingIds: pending,
remove: jest.fn(),
isPending: jest.fn((id: string) => pending().has(id)),
canTakeOver: jest.fn(
(id: string) =>
!(
removal.progress as {
isDeleting: (id: string) => boolean;
}
).isDeleting(id) &&
!TestBed.inject(SourceActivityService).isBusy(id)
),
progress: {
isDeleting: () => false,
message: () => '',
@@ -227,6 +237,13 @@ describe('RecentPlaylistsComponent', () => {
expect(removal.remove).toHaveBeenCalledWith(M3U, FILE);
});
it('hands over past a source a refresh holds', () => {
const release = TestBed.inject(SourceActivityService).begin(['xtream']);
component.remove(M3U);
expect(removal.remove).toHaveBeenCalledWith(M3U, FILE);
release();
});
it('keeps a running delete on screen whatever the filters', () => {
(
removal.progress as { isDeleting: (id: string) => boolean }
@@ -217,7 +217,7 @@ export class RecentPlaylistsComponent {
const next = replacementAfterRemoval(
this.visiblePlaylists(),
playlist._id,
(row) => this.removal.progress.isDeleting(row._id)
(row) => !this.removal.canTakeOver(row._id)
);
this.removal.remove(playlist, next);
}
@@ -233,6 +233,15 @@ describe('SourceRemovalService', () => {
expect(toasts[0].message).toBe('WORKSPACE.SOURCES.REMOVED');
});
it('does not hand over to a source a refresh or import holds', () => {
resolvedPlaylistId.set('a');
const release = TestBed.inject(SourceActivityService).begin(['b']);
service.remove(playlist('a'), playlist('b'));
expect(context.selectPlaylist).not.toHaveBeenCalled();
expect(toasts[0].message).toBe('WORKSPACE.SOURCES.REMOVED');
release();
});
it('leaves a switch the user made after the delete alone on Undo', () => {
resolvedPlaylistId.set('a');
service.remove(playlist('a'), playlist('b'));
@@ -85,6 +85,19 @@ export class SourceRemovalService {
return this.pendingState().has(id);
}
/**
* Whether a source can become active in place of a removed one: not
* leaving itself, and not held by a refresh, import or delete, since
* opening it would start an import against a catalog being rewritten.
*/
canTakeOver(id: string): boolean {
return (
!this.isPending(id) &&
!this.progress.isDeleting(id) &&
!this.activity.isBusy(id)
);
}
/**
* @param next The source to switch to when `playlist` is the active one:
* the caller's next row, so the switch follows the list on screen.
@@ -112,14 +125,8 @@ export class SourceRemovalService {
const wasOnItsRoute =
this.playlistContext.routePlaylistId() === playlist._id;
const previousUrl = this.router.url;
// A source that is leaving too cannot take over.
const replacement =
wasActive &&
next &&
!this.isPending(next._id) &&
!this.progress.isDeleting(next._id)
? next
: null;
wasActive && next && this.canTakeOver(next._id) ? next : null;
this.setPending(playlist._id, true);
if (replacement) {
@@ -139,6 +139,27 @@ describe('sourceDisplayName', () => {
);
});
it('drops a name that is nothing but a short password', () => {
const xtream = {
serverUrl: 'http://xtream.tv:8080',
username: 'joe',
password: 'pw',
};
expect(sourceDisplayName({ ...xtream, title: 'pw' })).toBe(
'xtream.tv:8080'
);
expect(sourceDisplayName({ ...xtream, title: ' PW ' })).toBe(
'xtream.tv:8080'
);
// Inside a longer name a short password is just letters.
expect(sourceDisplayName({ ...xtream, title: 'Pwnova' })).toBe(
'Pwnova'
);
expect(sourceDisplayUrl({ ...xtream, username: 'pw' })).toBe(
'http://xtream.tv:8080'
);
});
it('keeps a name that only shares a letter with a decoded password', () => {
// `%2542` is the password as written; decoded it is just `B`.
const url = 'https://host.test/list?password=%2542';
@@ -27,7 +27,10 @@ export type SourceDisplayFields = Partial<
/** The account name stays on screen by design; every other secret goes. */
const SHOWN_ACCOUNT_KEYS = new Set(['username', 'login', 'user', 'usr']);
/** Shorter values match ordinary words, not secrets. */
/**
* Shorter secrets match ordinary words inside a text, so they count only
* as the whole text: a name that is just a two-letter password is hidden.
*/
const MIN_SECRET_LENGTH = 3;
const ABSOLUTE_URL = /^[a-z][a-z0-9+.-]*:\/\//i;
@@ -185,24 +188,14 @@ function sourceSecrets(playlist: SourceDisplayFields): string[] {
if (url) {
collectUrlSecrets(url, secrets, 0);
}
const shown: string[] = [];
const forms: string[] = [];
for (const secret of secrets) {
if (!secret) {
continue;
}
// A value encoded more than once reads as itself in a name. Each
// form must be long enough on its own: `%2542` decodes to `B`.
for (const form of [
secret,
encodeURIComponent(secret),
decodeFully(secret),
]) {
if (form.length >= MIN_SECRET_LENGTH) {
shown.push(form);
}
if (secret) {
// A value encoded more than once reads as itself in a name.
forms.push(secret, encodeURIComponent(secret), decodeFully(secret));
}
}
return shown;
return forms;
}
/**
@@ -215,10 +208,18 @@ export function containsSourceSecret(
text: string,
playlist: SourceDisplayFields
): boolean {
const forms = [text, decodeFully(text)].map((form) => form.toLowerCase());
const forms = [text, decodeFully(text)].map((form) =>
form.trim().toLowerCase()
);
return sourceSecrets(playlist).some((secret) => {
const needle = secret.toLowerCase();
return forms.some((form) => form.includes(needle));
// Each form on its own: `%2542` decodes to `B`, which may only hide
// a text that is nothing but `b`.
return forms.some(
(form) =>
form === needle ||
(needle.length >= MIN_SECRET_LENGTH && form.includes(needle))
);
});
}