diff --git a/README.md b/README.md index 2d13c78..4e975a7 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,8 @@ await client.removeMapping({ public: 8080 }); `getGateway()` discovers the UPnP gateway via SSDP and returns an `UpnpInfo` object. The SSDP socket is created for discovery and automatically closed when done — no cleanup needed. +The search goes out again 250, 750, 1750 and 3750 ms after the first until a gateway answers, since UDP loses datagrams and some routers drop searches for a second or more at a time; it stops at the first answer, or when `timeout` (5 s by default) runs out. Only an answer whose location is an IPv4 address is taken: some routers answer twice, with an IPv6 location first, and both the mappings and the host's address are IPv4. + Device info and capabilities are fetched lazily on first access and cached; the local address is resolved on every call. All three come at once via `getAll()`: ```typescript @@ -63,7 +65,7 @@ const addr = await info.getLocalAddress(); With `cacheGateway: true` the gateway found by the first discovery is kept for later calls. A request to it that fails with anything but a UPnP fault - a connection refused or reset, no answer within the request timeout, an answer that is not a SOAP reply - drops it and searches again, since a router whose UPnP service restarts answers on a new port. If the router now answers at a different location, the request is sent there once more; at the same location the original error stands. A UPnP fault, an action the router does not support, and an abort by the caller keep the cached gateway. -A request sent again may find that the first one landed before the router went quiet. `createMapping` then gets 718 for its own mapping, and resolves if the router holds exactly that mapping (same internal client, internal port and description). `removeMapping` gets 714, and `removeMappingRange` 714 or 730, and they resolve. On a first request each of these codes throws as usual. `createAnyMapping` is never sent twice, because the router picks its port. +A request sent again may find that the first one landed before the router went quiet. `createMapping` then gets 718 for its own mapping, and resolves if the router holds exactly that mapping (same internal client, internal port and description, compared as `canonicalDescription` gives it). `removeMapping` gets 714, and `removeMappingRange` 714 or 730, and they resolve. On a first request each of these codes throws as usual. `createAnyMapping` is never sent twice, because the router picks its port. ### Device Info @@ -119,6 +121,11 @@ const mapping = await client.getMapping({ public: 8080, protocol: "TCP" }); const all = await client.getMappings(); const local = await client.getMappings({ local: true }); const filtered = await client.getMappings({ description: /^Flux_/ }); + +// A string filter is matched as routers store descriptions: FRITZ!Box keeps +// "Flux_App_web" as "Flux.App.web", and both match "Flux_App_". Compare any +// stored description the same way with canonicalDescription(): +canonicalDescription("Flux.App.web") === canonicalDescription("Flux_App_web"); // true ``` ## Network Info @@ -213,7 +220,7 @@ The gateway, its description and its capabilities are fetched once and shared be | Option | Type | Default | Description | |--------|------|---------|-------------| -| `timeout` | `number` | `1800` | SSDP discovery timeout (ms) | +| `timeout` | `number` | `5000` | SSDP discovery timeout (ms) | | `cacheGateway` | `boolean` | `false` | Keep the discovered gateway between calls; see [Caching the Gateway](#caching-the-gateway) | | `url` | `string` | — | Bypass SSDP, connect directly | | `localAddress` | `string` | — | Required when using `url` | diff --git a/package-lock.json b/package-lock.json index deb04a1..9c3cae1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@runonflux/nat-upnp", - "version": "2.0.3", + "version": "2.0.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@runonflux/nat-upnp", - "version": "2.0.3", + "version": "2.0.4", "license": "BlueOak-1.0.0", "dependencies": { "axios": "^1.6.5", diff --git a/package.json b/package.json index a4b33df..bb186f7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@runonflux/nat-upnp", - "version": "2.0.3", + "version": "2.0.4", "main": "build/src/index.js", "types": "build/src/index.d.ts", "files": [ diff --git a/src/index.ts b/src/index.ts index 9a7d320..44c0436 100644 --- a/src/index.ts +++ b/src/index.ts @@ -19,7 +19,7 @@ export type { export { Ssdp } from "./nat-upnp/ssdp"; export type { SearchCallback, ISsdp, SsdpEmitter } from "./nat-upnp/ssdp"; -export { Client, UpnpInfo } from "./nat-upnp/client"; +export { Client, UpnpInfo, canonicalDescription } from "./nat-upnp/client"; export type { GetMappingOpts, GetMappingRangeOpts, diff --git a/src/nat-upnp/client.ts b/src/nat-upnp/client.ts index 9c1d403..ed9b116 100644 --- a/src/nat-upnp/client.ts +++ b/src/nat-upnp/client.ts @@ -1,3 +1,4 @@ +import net from "net"; import { RawResponse } from "../index"; import Device, { GatewayDevice, @@ -113,7 +114,7 @@ export class Client implements IClient { throw new Error("`localAddress` must be supplied if using `url`"); } - this.timeout = options.timeout || 1800; + this.timeout = options.timeout || 5000; this.url = options.url || null; this.localAddress = options.localAddress || null; this.cacheGateway = options.cacheGateway || false; @@ -170,7 +171,7 @@ export class Client implements IClient { held !== null && held.mapping.private.host === internalClient && held.mapping.private.port === ports.internal.port && - held.mapping.description === description; + canonicalDescription(held.mapping.description) === canonicalDescription(description); if (!isThisMapping) throw err; return held.raw; } @@ -632,6 +633,11 @@ export class Client implements IClient { p.on("device", (headers) => { if (resolved) return; + // Mappings are IPv4 and the node's address is read from its route to + // the router, so only an IPv4 location will do. Some routers answer + // an IPv4 search twice, with an IPv6 location first; the IPv4 answer + // that follows is the one taken. + if (!isIPv4Location(headers.location)) return; resolved = true; p.emit("end"); clearTimeout(timeout); @@ -745,6 +751,14 @@ function validPort(value: unknown, what: string, lowest: 0 | 1 = 1): number { * from strict firmware, or a mapping to nothing reported as success from * lenient firmware. The absence is an error before the wire, not an element. */ +function isIPv4Location(location: string): boolean { + try { + return net.isIPv4(new URL(location).hostname); + } catch { + return false; + } +} + function internalClientFor(host: string | undefined, localAddress: string): string { const client = host || localAddress; if (!client) { @@ -837,10 +851,22 @@ function findResponseKey(data: RawResponse, prefix: string): any | null { return key ? data[key] : null; } +/** + * A mapping description as routers keep it, whatever they did to its + * punctuation: every run of characters other than letters and digits becomes + * one underscore. FRITZ!Box stores "Flux_App_web" as "Flux.App.web" and + * "FluxOS Reserved" as "FluxOS.Reserved"; all three forms of each compare + * equal here. Compare descriptions a router has stored this way, never as + * they were sent. + */ +export function canonicalDescription(description: string): string { + return String(description ?? "").replace(/[^A-Za-z0-9]+/g, "_"); +} + function matchesDescription(desc: string, filter: RegExp | string): boolean { if (typeof desc !== "string") return false; if (filter instanceof RegExp) return filter.test(desc); - return desc.indexOf(filter) !== -1; + return canonicalDescription(desc).includes(canonicalDescription(filter)); } export default Client; diff --git a/src/nat-upnp/device.ts b/src/nat-upnp/device.ts index 0a84ccf..61c33e7 100644 --- a/src/nat-upnp/device.ts +++ b/src/nat-upnp/device.ts @@ -107,11 +107,25 @@ function text(value: unknown): string { * mapping table does within 50 ms. * * A kept connection is closed from this side after KEPT_CONNECTION_IDLE_MS - * without a request: sooner than those routers close one themselves (4 to 5 s), - * so a request never crosses the router's close, and the connection does not - * hold one of the router's few slots between unrelated calls. + * unused in the pool: sooner than those routers close one themselves (4 to + * 5 s), so a request never crosses the router's close, and the connection does + * not hold one of the router's few slots between unrelated calls. The limit is + * the pool's alone. Given to the agent as its `timeout` it would apply to every + * socket from creation, connecting included, and a router slow to accept - a + * busy miniupnpd, one loop behind a backlog of five - would have requests cut + * off at the idle limit instead of at their own timeout. */ class RouterAgent extends http.Agent { + constructor() { + super({ keepAlive: true }); + // After the agent's own "free" handler, which sets a pooled socket's + // timeout to the agent's - none. + this.on("free", (socket: Socket) => { + const pooled = Object.values(this.freeSockets).some((sockets) => sockets?.includes(socket)); + if (pooled && !socket.destroyed) socket.setTimeout(KEPT_CONNECTION_IDLE_MS); + }); + } + // Node returns whether the socket was kept; @types/node declares void. keepSocketAlive(socket: Socket): void { // The request the socket has just carried, and the router's answer to it. @@ -124,7 +138,8 @@ class RouterAgent extends http.Agent { } } export const KEPT_CONNECTION_IDLE_MS = 3000; -const upnpAgent = new RouterAgent({ keepAlive: true, timeout: KEPT_CONNECTION_IDLE_MS }); +/** Exported for the tests, which slow a connection's connect through it. */ +export const upnpAgent = new RouterAgent(); // Prevent OOM from malicious/broken router responses const MAX_RESPONSE_BYTES = 2 * 1024 * 1024; diff --git a/src/nat-upnp/ssdp.ts b/src/nat-upnp/ssdp.ts index 9f93f5b..719bcad 100644 --- a/src/nat-upnp/ssdp.ts +++ b/src/nat-upnp/ssdp.ts @@ -17,6 +17,14 @@ export class Ssdp implements ISsdp { */ private static readonly INTERNET_ROUTE_PROBE = "8.8.8.8"; + /** + * After the first M-SEARCH, the same search goes out again at these offsets + * (ms) until it ends: UDP loses some, and a router can drop searches for a + * second or more at a time. The gaps double, as DHCP's retransmissions do, + * so the last copies still reach a router that was silent for the first. + */ + static readonly RESEND_AFTER_MS: readonly number[] = [250, 750, 1750, 3750]; + private readonly sourcePort: number; private readonly multicast = "239.255.255.250"; private readonly port = 1900; @@ -26,6 +34,7 @@ export class Ssdp implements ISsdp { private bound = false; private closed = false; private readonly pendingSearches: [string, SsdpEmitter][] = []; + private readonly resendTimers = new Set(); constructor(options?: { sourcePort?: number }) { this.sourcePort = options?.sourcePort || 0; @@ -138,6 +147,17 @@ export class Ssdp implements ISsdp { let ended = false; + const resends = Ssdp.RESEND_AFTER_MS.map((after) => { + const timer = setTimeout(() => { + this.resendTimers.delete(timer); + // a socket that died and was not replaced has no one to send for + if (!this.socket) return; + this.socket.send(query, 0, query.length, this.port, this.multicast); + }, after); + this.resendTimers.add(timer); + return timer; + }); + const ondevice: SearchCallback = (headers) => { if (ended || headers.st !== device) return; emitter!.emit("device", headers); @@ -155,6 +175,10 @@ export class Ssdp implements ISsdp { this.ssdpEmitter.removeListener("device", ondevice); this.ssdpEmitter.removeListener("error", onerror); ended = true; + for (const timer of resends) { + clearTimeout(timer); + this.resendTimers.delete(timer); + } }); return emitter; @@ -166,6 +190,8 @@ export class Ssdp implements ISsdp { this.bound = false; this.pendingSearches.length = 0; this.ssdpEmitter.removeAllListeners(); + for (const timer of this.resendTimers) clearTimeout(timer); + this.resendTimers.clear(); if (this.socket) { try { diff --git a/test/unit.test.ts b/test/unit.test.ts index 38f6ceb..427162c 100644 --- a/test/unit.test.ts +++ b/test/unit.test.ts @@ -13,8 +13,8 @@ import { decodeXmlEntities, untilAborted, } from "../src/nat-upnp/device"; -import { Device, KEPT_CONNECTION_IDLE_MS } from "../src/nat-upnp/device"; -import { Client } from "../src/nat-upnp/client"; +import { Device, KEPT_CONNECTION_IDLE_MS, upnpAgent } from "../src/nat-upnp/device"; +import { Client, canonicalDescription } from "../src/nat-upnp/client"; import { parseMimeHeader, Ssdp, type SsdpEmitter } from "../src/nat-upnp/ssdp"; import { surveyedRouters } from "./router-data"; import { installFakeDgram, FakeSocket, ssdpResponse, settle } from "./fake-dgram"; @@ -2674,6 +2674,58 @@ function getSoapFaultCode(xml: string): number | null { }); }); + const IGD_SEARCH = "urn:schemas-upnp-org:device:InternetGatewayDevice:1"; + + await test("a router that answers only the last search is found within the default timeout", async () => { + const fake = installFakeDgram(); + const restore = installFakeRouter("opnsense"); + const realSend = FakeSocket.prototype.send; + let searches = 0; + FakeSocket.prototype.send = function (this: FakeSocket, ...args: Parameters) { + realSend.apply(this, args); + searches++; + if (searches === Ssdp.RESEND_AFTER_MS.length + 1) { + setImmediate(() => this.deliver(ssdpResponse(IGD_SEARCH, DESCRIPTION_URL))); + } + }; + const client = new Client({}); + try { + const info = await client.getGateway(); + assertEqual(info.gateway.description, DESCRIPTION_URL, "found"); + assertEqual(searches, Ssdp.RESEND_AFTER_MS.length + 1, "on the last search"); + } finally { + client.close(); + FakeSocket.prototype.send = realSend; + restore(); + fake.restore(); + } + }); + + await test("discovery passes over an IPv6 location and takes the IPv4 answer after it", async () => { + await withDiscovery({}, async (client, sockets) => { + const pending = client.getGateway(); + await settle(); + for (const socket of sockets) { + socket.deliver(ssdpResponse(IGD_SEARCH, "http://[2604:3d08:1c7f:dd05::1]:5000/rootDesc.xml")); + socket.deliver(ssdpResponse(IGD_SEARCH, DESCRIPTION_URL)); + } + const info = await pending; + assertEqual(info.gateway.description, DESCRIPTION_URL, "the IPv4 location"); + }); + }); + + await test("an IPv6 location alone is never taken", async () => { + await withDiscovery({ timeout: 300 }, async (client, sockets) => { + const pending = expectThrow(() => client.getGateway(), "only an IPv6 answer"); + await settle(); + for (const socket of sockets) { + socket.deliver(ssdpResponse(IGD_SEARCH, "http://[2604:3d08:1c7f:dd05::1]:5000/rootDesc.xml")); + } + const err = await pending; + assert(/timed out/i.test((err as Error).message), `got: ${(err as Error).message}`); + }); + }); + await test("discovery that nothing answers times out", async () => { await withDiscovery({ timeout: 200 }, async (client) => { const err = await expectThrow(() => client.getGateway(), "unanswered discovery"); @@ -3176,6 +3228,32 @@ function getSoapFaultCode(xml: string): number | null { }); } + await test("a re-sent mapping the router holds with its description stored FRITZ!Box-style counts as made", async () => { + // FRITZ!Box keeps "Flux_App_x" as "Flux.App.x". + await withMovingGateway(async (g) => { + await g.client.getPublicIp(); + g.intercept("AddPortMapping", (origin) => { + if (origin === FIRST_ORIGIN) { + g.moveTo(SECOND_LOCATION); + throw Object.assign(new Error("socket hang up"), { code: "ECONNRESET" }); + } + throw faultResponse(718, "ConflictInMappingEntry"); + }); + g.intercept("GetSpecificPortMappingEntry", () => ({ + data: + '' + + '' + + "36666192.168.1.220" + + `1${MAPPED_DESCRIPTION.replace(/_/g, ".")}` + + "0", + })); + + const answer = await g.client.createMapping({ ...MAPPED, description: MAPPED_DESCRIPTION }); + + assert("GetSpecificPortMappingEntryResponse" in answer, "the lookup that found it is the answer"); + }); + }); + await test("a re-sent mapping refused when the router holds no such entry stays refused", async () => { await withMovingGateway(async (g) => { await g.client.getPublicIp(); @@ -3392,6 +3470,34 @@ function getSoapFaultCode(xml: string): number | null { assertEqual(missed.length, 0, "an unrelated fragment does not"); }); + await test("canonicalDescription reads a description as routers store it", async () => { + for (const [stored, sent] of [ + ["Flux.App.Fluxtracker", "Flux_App_Fluxtracker"], + ["FluxOS.Reserved", "FluxOS Reserved"], + ["Flux.Prelaunch.App.7001", "Flux_Prelaunch_App_7001"], + ["Flux.Backend.API.SSL", "Flux_Backend_API_SSL"], + ["Flux - Home UI", "Flux_Home_UI"], + ]) { + assertEqual(canonicalDescription(stored), canonicalDescription(sent), `${stored} is ${sent}`); + } + assert(canonicalDescription("Flux.App.web") !== canonicalDescription("Flux_App_webdelta"), "a different name stays different"); + assert(canonicalDescription("Flux_App_web").startsWith(canonicalDescription("Flux_App_")), "a prefix stays a prefix"); + }); + + await test("getMappings matches a string description as the router stored it", async () => { + const fritzStored = () => ({ + data: + '' + + '' + + "31000TCP" + + "31000172.16.32.1431" + + "Flux.App.web0" + + "", + }); + const found = await withWalkIndexAnswering(0, fritzStored, (c) => c.getMappings({ description: "Flux_App_" })); + assertEqual(found.map((m) => m.description).join(","), "Flux.App.web", "the FRITZ!Box form matches"); + }); + await test("getMappings accepts a regular expression description", async () => { const found = await withRouter("opnsense", (c) => c.getMappings({ description: /^FluxOS/ }) @@ -3610,6 +3716,40 @@ function getSoapFaultCode(xml: string): number | null { }); }); + const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + + await test("a search goes out again at the resend offsets until it ends", async () => { + await withSsdp(async (ssdp, sockets) => { + const search = ssdp.search(IGD); + await settle(); + await sleep(Ssdp.RESEND_AFTER_MS[1] + 100); + assertEqual(sockets[0].sent.length, 3, "the first search and the first two resends"); + assert( + sockets[0].sent.every((sent) => sent.query === sockets[0].sent[0].query), + "every copy is the same search" + ); + search.emit("end"); + await sleep(Ssdp.RESEND_AFTER_MS[2] - Ssdp.RESEND_AFTER_MS[1] + 100); + assertEqual(sockets[0].sent.length, 3, "nothing more once the search has ended"); + }); + }); + + await test("closing the instance stops its resends and holds no timer", async () => { + const timers = () => process.getActiveResourcesInfo().filter((kind) => kind === "Timeout").length; + await withSsdp(async (ssdp, sockets) => { + const before = timers(); + ssdp.search(IGD); + await settle(); + await sleep(Ssdp.RESEND_AFTER_MS[0] + 100); + assertEqual(timers(), before + Ssdp.RESEND_AFTER_MS.length - 1, "the resends still to come"); + const sent = sockets[0].sent.length; + ssdp.close(); + assertEqual(timers(), before, "none left to keep the process alive"); + await sleep(Ssdp.RESEND_AFTER_MS[1]); + assertEqual(sockets[0].sent.length, sent, "no send after close"); + }); + }); + await test("the search leaves via the internet-facing interface", async () => { await withSsdp(async (ssdp, sockets) => { // The kernel routes multicast by its own table, which an unrelated @@ -4886,6 +5026,43 @@ function getSoapFaultCode(xml: string): number | null { }); }); + await test("a new connection carries no timeout of the agent's, so connecting is held to the request's own", async () => { + // A busy router accepts late, and a socket's own timeout runs while it + // connects; the request's timeout takes over only once it has. The idle + // limit is the pool's alone. + await withRawRouter("keep-alive", 1, async (router) => { + const timeouts = await new Promise<{ atCreation: number | undefined; pooled: number | undefined }>((resolve, reject) => { + const req = http.request(router.url, { agent: upnpAgent }, (res) => { + res.resume(); + res.on("end", () => setImmediate(() => resolve({ atCreation, pooled: (req.socket as any)?.timeout }))); + }); + let atCreation: number | undefined; + req.on("socket", (socket) => { atCreation = (socket as any).timeout; }); + req.on("error", reject); + req.end(); + }); + assert(!timeouts.atCreation, `no timeout on a new connection, got ${timeouts.atCreation}`); + assertEqual(timeouts.pooled, KEPT_CONNECTION_IDLE_MS, "the idle limit once pooled"); + }); + }); + + await test("an answer on a new connection slower than the kept connection's idle limit still arrives", async () => { + // Every request here is on a connection of its own: the router answers close. + await withRawRouter( + "close", + 1, + async (router) => { + const client = rawClient(router); + try { + assertEqual((await client.getMappings()).length, 1, "the table, answered slowly"); + } finally { + client.close(); + } + }, + KEPT_CONNECTION_IDLE_MS + 500 + ); + }); + await test("an answer slower than the kept connection's idle limit still arrives", async () => { await withRawRouter( "keep-alive",