mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-11 02:46:16 -08:00
fix(stalker): budget the four-request login flow and drain abandoned attempts
Two timing defects in the discovery/auth interaction: - The auth budget was written for "two sequential requests", but a status-2 portal costs FOUR — handshake, profile, do_auth, profile retry — and the Electron transport allows each 15 s. A valid but slow login-required portal was aborted before its final profile and reported as auth-rejected. - Aborting a timed-out attempt stops it from SENDING further requests, but a `get_profile` already on the wire is processed regardless, and discovery advanced immediately — so the abandoned attempt could adopt the MAC's token after the next candidate had negotiated its own, invalidating a portal that actually works. The run now drains the abandoned attempt (bounded by one request budget) instead of racing it. The second corrects a claim this branch made in the docs: the in-flight window was described as unclosable from the client. The request cannot be un-sent, but nothing forces us to have a competing session in flight while it lands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
2e5cc3d901
commit
ab9ad00748
3 files changed
+110
-12
No files matched your search
@@ -361,6 +361,17 @@ regardless of what the client does, so tearing the socket down would not
|
||||
prevent the adoption. Only not sending the request does, which is exactly what
|
||||
the between-calls check guarantees.
|
||||
|
||||
That leaves the window where the timer fires while a `get_profile` is already
|
||||
dispatched — which the check cannot cover, but sequencing can: discovery
|
||||
**drains** the abandoned attempt (bounded by one request budget) before probing
|
||||
the next candidate, instead of racing it. The request cannot be un-sent, but
|
||||
nothing forces us to have a competing session in flight while it lands.
|
||||
|
||||
The budget itself covers the longest real flow: a status-2 portal costs four
|
||||
sequential requests (handshake, profile, `do_auth`, profile retry) and the
|
||||
Electron transport allows each 15 s, so a two-request budget would have failed
|
||||
valid but slow login portals.
|
||||
|
||||
### Watchdog
|
||||
|
||||
The portal expects `get_events` every `watchdog_timeout` seconds — **120 by
|
||||
|
||||
@@ -379,12 +379,17 @@ describe('StalkerPortalDiscoveryService', () => {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
|
||||
jest.advanceTimersByTime(45_000);
|
||||
jest.advanceTimersByTime(65_000);
|
||||
await Promise.resolve();
|
||||
|
||||
// The abandoned attempt is cancelled, so its get_profile never
|
||||
// goes out to adopt the MAC's token behind a later candidate.
|
||||
expect(captured?.aborted).toBe(true);
|
||||
|
||||
// …and discovery does not RACE it: a request already dispatched
|
||||
// cannot be un-sent, so the run drains it (bounded) instead of
|
||||
// probing the next candidate while it may still land.
|
||||
jest.advanceTimersByTime(15_000);
|
||||
// Outcome shape is unchanged: a timed-out confirmation is still
|
||||
// reported as a refusal of that endpoint.
|
||||
await expect(discovery).resolves.toMatchObject({
|
||||
@@ -395,4 +400,57 @@ describe('StalkerPortalDiscoveryService', () => {
|
||||
jest.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it('drains an abandoned attempt instead of probing the next candidate', async () => {
|
||||
jest.useFakeTimers();
|
||||
try {
|
||||
// Two candidates both answer "auth required", so a resolved first
|
||||
// attempt would stop the run — only a timed-out one advances.
|
||||
mockProbes({
|
||||
'http://slow.example/portal.php': {
|
||||
resolve: 'Authorization failed.',
|
||||
},
|
||||
'http://slow.example/server/load.php': {
|
||||
resolve: 'Authorization failed.',
|
||||
},
|
||||
});
|
||||
|
||||
let settleFirst: (() => void) | undefined;
|
||||
authenticate
|
||||
.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((_resolve, reject) => {
|
||||
settleFirst = () => reject(new Error('late'));
|
||||
})
|
||||
)
|
||||
.mockResolvedValue({ token: 'SECOND' });
|
||||
|
||||
const discovery = service.discover('http://slow.example/c', MAC);
|
||||
for (let i = 0; i < 6; i += 1) {
|
||||
await Promise.resolve();
|
||||
}
|
||||
|
||||
jest.advanceTimersByTime(65_000);
|
||||
for (let i = 0; i < 6; i += 1) {
|
||||
await Promise.resolve();
|
||||
}
|
||||
|
||||
// The first attempt is still on the wire: the second candidate
|
||||
// must NOT have been authenticated yet, or its token could be
|
||||
// invalidated by the first one's late get_profile.
|
||||
expect(authenticate).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Once it settles, the run continues immediately.
|
||||
settleFirst?.();
|
||||
for (let i = 0; i < 10; i += 1) {
|
||||
await Promise.resolve();
|
||||
}
|
||||
expect(authenticate).toHaveBeenCalledTimes(2);
|
||||
await expect(discovery).resolves.toMatchObject({
|
||||
status: 'resolved',
|
||||
});
|
||||
} finally {
|
||||
jest.useRealTimers();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -59,8 +59,27 @@ export type StalkerPortalDiscoveryOutcome =
|
||||
|
||||
/** Per-request guard so a hanging host cannot stall discovery forever. */
|
||||
const PROBE_TIMEOUT_MS = 20_000;
|
||||
/** authenticate() is two sequential requests; give it a matching budget. */
|
||||
const AUTH_TIMEOUT_MS = 45_000;
|
||||
/**
|
||||
* `authenticate()` is up to FOUR sequential requests once a portal answers
|
||||
* `get_profile` with status 2 — handshake, profile, `do_auth`, profile retry —
|
||||
* and the Electron transport allows each non-`create_link` call 15 s. A budget
|
||||
* that only covered two would abort a valid but slow login-required portal
|
||||
* before its final profile and report it as `auth-rejected`.
|
||||
*/
|
||||
const AUTH_TIMEOUT_MS = 4 * 15_000 + 5_000;
|
||||
|
||||
/**
|
||||
* How long to wait for an abandoned attempt to settle before probing the next
|
||||
* candidate.
|
||||
*
|
||||
* Aborting cannot un-send a request: if its `get_profile` was already
|
||||
* dispatched, the portal adopts that token regardless of what the client does
|
||||
* to its socket. What we CAN do is refuse to race it — advancing while it is
|
||||
* still in flight is what lets it invalidate the token the next candidate
|
||||
* negotiates. Bounded by one request budget so a genuinely hung host cannot
|
||||
* stall discovery forever; past that the risk is accepted rather than hanging.
|
||||
*/
|
||||
const ABANDONED_DRAIN_MS = 15_000;
|
||||
|
||||
function withTimeout<T>(
|
||||
promise: Promise<T>,
|
||||
@@ -212,16 +231,16 @@ export class StalkerPortalDiscoveryService {
|
||||
// portal call, so a timed-out attempt never sends the `get_profile`
|
||||
// that would adopt the MAC's token behind the next candidate's back.
|
||||
const abandon = new AbortController();
|
||||
// Kept so a timed-out attempt can be drained rather than raced.
|
||||
const pending = this.stalkerSession.authenticate(
|
||||
candidate,
|
||||
macAddress,
|
||||
identity,
|
||||
{ credentials, signal: abandon.signal }
|
||||
);
|
||||
try {
|
||||
const auth = await withTimeout(
|
||||
this.stalkerSession.authenticate(
|
||||
candidate,
|
||||
macAddress,
|
||||
identity,
|
||||
{ credentials, signal: abandon.signal }
|
||||
),
|
||||
AUTH_TIMEOUT_MS,
|
||||
() => abandon.abort()
|
||||
const auth = await withTimeout(pending, AUTH_TIMEOUT_MS, () =>
|
||||
abandon.abort()
|
||||
);
|
||||
// A handshake can hand out a token whose `get_profile` still
|
||||
// answers a structured denial (`{js:{error:'Invalid token'}}`);
|
||||
@@ -245,6 +264,16 @@ export class StalkerPortalDiscoveryService {
|
||||
timeslotSeconds: auth.timeslotSeconds,
|
||||
};
|
||||
} catch (error) {
|
||||
// Do not advance while the abandoned attempt may still be on the
|
||||
// wire: its `get_profile` adopts the MAC's token portal-side, so
|
||||
// racing it is exactly what invalidates the next candidate's
|
||||
// freshly issued session.
|
||||
await Promise.race([
|
||||
pending.catch(() => undefined),
|
||||
new Promise((resolve) =>
|
||||
setTimeout(resolve, ABANDONED_DRAIN_MS)
|
||||
),
|
||||
]);
|
||||
return {
|
||||
status: 'auth-rejected',
|
||||
portalUrl: candidate,
|
||||
|
||||
Reference in new issue
Block a user