diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index 2aa3b8f..3e84f5a 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -24,7 +24,7 @@ jobs: uses: actions/cache@v5 with: path: '**/node_modules' - key: ${{ runner.os }}-modules-${{ hashFiles('**/yarn.lock') }}-${{ matrix.node-version }} + key: ${{ runner.os }}-modules-${{ hashFiles('**/package.json') }}-${{ matrix.node-version }} - name: Install dependencies if: steps.yarn-cache.outputs.cache-hit != 'true' run: yarn install @@ -37,6 +37,28 @@ jobs: - name: Run examples run: node examples/example.js + verify-package: + runs-on: ubuntu-latest + needs: build + steps: + - uses: actions/checkout@v5 + - name: Use Node.js 22 (LTS) + uses: actions/setup-node@v5 + with: + node-version: '22' + check-latest: true + - name: Cache Node.js modules + id: yarn-cache + uses: actions/cache@v5 + with: + path: '**/node_modules' + key: ${{ runner.os }}-modules-${{ hashFiles('**/package.json') }}-22 + - name: Install dependencies + if: steps.yarn-cache.outputs.cache-hit != 'true' + run: yarn install + - name: Verify published artifact (pack, publint, attw) + run: yarn verify:package + deno: runs-on: ubuntu-latest strategy: @@ -54,7 +76,7 @@ jobs: uses: actions/cache@v5 with: path: '**/node_modules' - key: ${{ runner.os }}-modules-${{ hashFiles('**/yarn.lock') }}-${{ matrix.node-version }}-deno + key: ${{ runner.os }}-modules-${{ hashFiles('**/package.json') }}-${{ matrix.node-version }}-deno - name: Install dependencies if: steps.yarn-cache.outputs.cache-hit != 'true' run: yarn install diff --git a/package.json b/package.json index 944de65..91da806 100644 --- a/package.json +++ b/package.json @@ -24,8 +24,10 @@ "ntp-packet-parser": "^0.6.0" }, "devDependencies": { + "@arethetypeswrong/cli": "0.18.2", "@types/node": "^20 || ^22 || ^24 || ^25", "prettier": "3.8.3", + "publint": "0.3.18", "ts-node": "10.9.2", "typescript": "6.0.3" }, @@ -39,8 +41,13 @@ "test:integration:esm": "node test/integration/esm.mjs", "test:integration:deno": "deno run --allow-read test/integration/deno.ts && deno run --allow-read test/integration/deno-esm.ts", "test:integration": "yarn test:integration:cjs && yarn test:integration:esm", + "test:unit": "node scripts/run-unit-tests.mjs", "prettier": "prettier --write src/**/*.ts", - "prettier:lint": "prettier --list-different src/**/*.ts" + "prettier:lint": "prettier --list-different src/**/*.ts", + "verify:pack": "npm pack --dry-run", + "verify:publint": "publint", + "verify:attw": "attw --pack .", + "verify:package": "yarn build && yarn verify:pack && yarn verify:publint && yarn verify:attw" }, "keywords": [ "ntp", @@ -53,7 +60,7 @@ "author": "Laurens Stötzel", "repository": { "type": "git", - "url": "https://github.com/buffcode/ntp-time-sync.git" + "url": "git+https://github.com/buffcode/ntp-time-sync.git" }, "files": [ "dist" diff --git a/scripts/run-unit-tests.mjs b/scripts/run-unit-tests.mjs new file mode 100644 index 0000000..5ec6db5 --- /dev/null +++ b/scripts/run-unit-tests.mjs @@ -0,0 +1,53 @@ +// Runner for the unit test suite. +// +// Discovers every test/unit/**/*.test.ts file and spawns `node --test` with +// ts-node's transpile-only CJS hook registered. Uses TS_NODE_COMPILER_OPTIONS +// to force CommonJS compilation with bundler resolution so that the source's +// `.js`-suffixed ESM-style imports resolve correctly under Yarn PnP without +// pulling in a PnP ESM loader. + +import { readdirSync, statSync } from "node:fs"; +import { join, relative } from "node:path"; +import { spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +const projectRoot = fileURLToPath(new URL("..", import.meta.url)); +const testRoot = join(projectRoot, "test", "unit"); + +function collectTestFiles(dir) { + const out = []; + for (const entry of readdirSync(dir)) { + const full = join(dir, entry); + const st = statSync(full); + if (st.isDirectory()) { + out.push(...collectTestFiles(full)); + } else if (st.isFile() && entry.endsWith(".test.ts")) { + out.push(relative(projectRoot, full)); + } + } + return out; +} + +const testFiles = collectTestFiles(testRoot).sort(); +if (testFiles.length === 0) { + console.error("No test files found under test/unit/"); + process.exit(1); +} + +const result = spawnSync( + process.execPath, + ["--test", "--require", "ts-node/register/transpile-only", ...testFiles], + { + stdio: "inherit", + cwd: projectRoot, + env: { + ...process.env, + TS_NODE_COMPILER_OPTIONS: JSON.stringify({ + module: "commonjs", + moduleResolution: "bundler", + }), + }, + } +); + +process.exit(result.status ?? 1); diff --git a/src/NtpTimeSync.ts b/src/NtpTimeSync.ts index 159693c..768242e 100644 --- a/src/NtpTimeSync.ts +++ b/src/NtpTimeSync.ts @@ -5,15 +5,6 @@ import { NtpPacket, NtpPacketParser } from "ntp-packet-parser"; import { NtpTimeResult } from "./NtpTimeResult.js"; import { RecursivePartial } from "./RecursivePartial.js"; -let singleton: NtpTimeSync | undefined; -let lastPoll: number | undefined; -let lastResult: - | undefined - | { - offset: number; - precision: number; - }; - export interface NtpTimeSyncConstructorOptions { servers: string[]; sampleCount: number; @@ -40,7 +31,27 @@ export interface NtpTimeSyncOptions extends Omit; } -export const NtpTimeSyncDefaultOptions = { +// Recursively freeze an object tree so consumers cannot mutate shared defaults. +// Arrays are frozen along with each of their entries. +const deepFreeze = (value: T): T => { + if (value === null || typeof value !== "object" || Object.isFrozen(value)) { + return value; + } + + if (Array.isArray(value)) { + for (const entry of value) { + deepFreeze(entry); + } + } else { + for (const key of Object.keys(value as object)) { + deepFreeze((value as Record)[key]); + } + } + + return Object.freeze(value); +}; + +export const NtpTimeSyncDefaultOptions = deepFreeze({ // list of NTP time servers, optionally including a port (defaults to options.ntpDefaults.port = 123) servers: ["0.pool.ntp.org", "1.pool.ntp.org", "2.pool.ntp.org", "3.pool.ntp.org"], @@ -64,7 +75,7 @@ export const NtpTimeSyncDefaultOptions = { precision: -18, referenceDate: new Date("Jan 01 1900 GMT"), }, -}; +}); interface NtpReceivedPacket extends Partial { destinationTimestamp: Date; @@ -78,8 +89,17 @@ interface SampleData { } export class NtpTimeSync { + private static singleton: NtpTimeSync | undefined; + private options: NtpTimeSyncOptions; private samples: SampleData[] = []; + private lastPoll: number | undefined; + private lastResult: + | undefined + | { + offset: number; + precision: number; + }; constructor(options: RecursivePartial = {}) { const serverConfig = options.servers || NtpTimeSyncDefaultOptions.servers; @@ -143,11 +163,11 @@ export class NtpTimeSync { * Returns a singleton */ static getInstance(options: RecursivePartial = {}): NtpTimeSync { - if (!singleton) { - singleton = new NtpTimeSync(options); + if (!NtpTimeSync.singleton) { + NtpTimeSync.singleton = new NtpTimeSync(options); } - return singleton; + return NtpTimeSync.singleton; } private async collectSamples(numSamples: number) { @@ -167,6 +187,8 @@ export class NtpTimeSync { ); }); + const prevResultCount = ntpResults.length; + // wait for NTP responses to arrive ntpResults = ntpResults .concat(await Promise.all(timePromises.map((p) => p.catch((e) => e)))) @@ -174,7 +196,9 @@ export class NtpTimeSync { return !(result instanceof Error); }); - if (ntpResults.length === 0) { + // count a retry whenever a full round produced no new usable samples; + // otherwise partial progress could loop forever against a slow server set + if (ntpResults.length === prevResultCount) { retry++; } } while (ntpResults.length < numSamples && retry < 3); @@ -186,10 +210,20 @@ export class NtpTimeSync { // filter erroneous responses, use valid ones as samples let samples: SampleData[] = []; ntpResults.forEach((data) => { - const transmitTimestamp = data.transmitTimestamp!; - const receiveTimestamp = data.receiveTimestamp!; - const originTimestamp = data.originTimestamp!; - const precision = data.precision!; + const transmitTimestamp = data.transmitTimestamp; + const receiveTimestamp = data.receiveTimestamp; + const originTimestamp = data.originTimestamp; + const precision = data.precision; + + // acceptResponse has already validated these fields; narrow for the type system + if ( + transmitTimestamp === undefined || + receiveTimestamp === undefined || + originTimestamp === undefined || + precision === undefined + ) { + return; + } const offsetSign = transmitTimestamp.getTime() > data.destinationTimestamp.getTime() ? 1 : -1; @@ -234,17 +268,17 @@ export class NtpTimeSync { async getTime(force = false): Promise { if ( !force && - lastPoll && - lastResult && - Date.now() - lastPoll < Math.pow(2, this.options.ntpDefaults.minPoll) * 1000 + this.lastPoll && + this.lastResult && + Date.now() - this.lastPoll < Math.pow(2, this.options.ntpDefaults.minPoll) * 1000 ) { let date = new Date(); - date.setUTCMilliseconds(date.getUTCMilliseconds() + lastResult.offset); + date.setUTCMilliseconds(date.getUTCMilliseconds() + this.lastResult.offset); return { now: date, - offset: lastResult.offset, - precision: lastResult.precision, + offset: this.lastResult.offset, + precision: this.lastResult.precision, }; } @@ -259,11 +293,11 @@ export class NtpTimeSync { const precision = NtpTimeSync.stdDev(this.samples.map((sample) => sample.offset)); - lastResult = { + this.lastResult = { offset: offset, precision: precision, }; - lastPoll = Date.now(); + this.lastPoll = Date.now(); let date = new Date(); date.setUTCMilliseconds(date.getUTCMilliseconds() + offset); @@ -286,14 +320,6 @@ export class NtpTimeSync { return now; } - private static pad(string: string, length: number, char = "0", side: "left" | "right" = "left") { - if (side === "left") { - return char.repeat(length).substring(0, length - string.length) + string; - } - - return string + char.repeat(length).substring(0, length - string.length); - } - /** * @param {Integer} leapIndicator, defaults to 3 (unsynchronized) * @param {Integer} ntpVersion, defaults to `options.ntpDefaults.version` @@ -303,49 +329,37 @@ export class NtpTimeSync { private createPacket(leapIndicator = 3, ntpVersion: number | undefined = undefined, mode = 3): Buffer { ntpVersion = ntpVersion || this.options.ntpDefaults.version; - // generate NTP packet - let ntpData = new Array(48).fill(0); - - ntpData[0] = - // Leap indicator (= 3, unsynchronized) - NtpTimeSync.pad((leapIndicator >>> 0).toString(2), 2) + - // NTP version (= 4) - NtpTimeSync.pad((ntpVersion >>> 0).toString(2), 3) + - // client mode (= 3) - NtpTimeSync.pad((mode >>> 0).toString(2), 3); + const buf = Buffer.alloc(48); - ntpData[0] = parseInt(ntpData[0], 2); + // Leap indicator (2 bits) | NTP version (3 bits) | mode (3 bits) + buf[0] = ((leapIndicator & 0x3) << 6) | ((ntpVersion & 0x7) << 3) | (mode & 0x7); - // origin timestamp - const baseTime = new Date().getTime() - this.options.ntpDefaults.referenceDate.getTime(); - const seconds = baseTime / 1000; - let ntpTimestamp = (seconds * Math.pow(2, 32)).toString(2); - ntpTimestamp = NtpTimeSync.pad(ntpTimestamp, 64); + // origin timestamp: seconds since 1900 epoch in upper 32 bits, + // fractional seconds (scaled by 2^32) in lower 32 bits + const baseTimeMs = new Date().getTime() - this.options.ntpDefaults.referenceDate.getTime(); + const seconds = Math.trunc(baseTimeMs / 1000); + const fractional = Math.trunc(((baseTimeMs % 1000) / 1000) * 2 ** 32); + const mask32 = BigInt("0xffffffff"); + const shift32 = BigInt(32); + const ntpTimestamp = ((BigInt(seconds) & mask32) << shift32) | (BigInt(fractional) & mask32); // origin timestamp - ntpData[24] = parseInt(ntpTimestamp.substr(0, 8), 2); - ntpData[25] = parseInt(ntpTimestamp.substr(8, 8), 2); - ntpData[26] = parseInt(ntpTimestamp.substr(16, 8), 2); - ntpData[27] = parseInt(ntpTimestamp.substr(24, 8), 2); - ntpData[28] = parseInt(ntpTimestamp.substr(32, 8), 2); - ntpData[29] = parseInt(ntpTimestamp.substr(40, 8), 2); - ntpData[30] = parseInt(ntpTimestamp.substr(48, 8), 2); - ntpData[31] = parseInt(ntpTimestamp.substr(56, 8), 2); - + buf.writeBigUInt64BE(ntpTimestamp, 24); // transmit timestamp - ntpData[40] = parseInt(ntpTimestamp.substr(0, 8), 2); - ntpData[41] = parseInt(ntpTimestamp.substr(8, 8), 2); - ntpData[42] = parseInt(ntpTimestamp.substr(16, 8), 2); - ntpData[43] = parseInt(ntpTimestamp.substr(24, 8), 2); - ntpData[44] = parseInt(ntpTimestamp.substr(32, 8), 2); - ntpData[45] = parseInt(ntpTimestamp.substr(40, 8), 2); - ntpData[46] = parseInt(ntpTimestamp.substr(48, 8), 2); - ntpData[47] = parseInt(ntpTimestamp.substr(56, 8), 2); - - return Buffer.from(ntpData); + buf.writeBigUInt64BE(ntpTimestamp, 40); + + return buf; } private static cleanup(client: dgram.Socket) { + try { + // Drop all listeners first so late-arriving error/message events on an + // already-abandoned socket cannot trigger resolve/reject a second time + // or keep the event loop alive. + client.removeAllListeners(); + } catch (e) { + // ignore, as we just want to cleanup + } try { client.close(); } catch (e) { @@ -381,43 +395,61 @@ export class NtpTimeSync { errorCallback(new Error("Timeout waiting for NTP response.")); }, this.options.replyTimeout); - client.send(this.createPacket(), port, server, (err: Error | null) => { + // Register the message listener BEFORE sending the packet so we never + // miss an unusually fast reply that arrives between send() completing + // and the send callback firing. + client.once("message", (msg: Buffer) => { if (hasFinished) { return; } - if (err) { - errorCallback(err); + clearTimeout(timeoutHandler); + timeoutHandler = undefined; + client.close(); + + let parsed: Partial; + try { + parsed = NtpPacketParser.parse(msg); + } catch (err) { + hasFinished = true; + reject(err); return; } - client.once("message", function (msg: Buffer) { + const result: NtpReceivedPacket = { + ...parsed, + destinationTimestamp: new Date(), + }; + + hasFinished = true; + resolve(result); + }); + + try { + client.send(this.createPacket(), port, server, (err: Error | null) => { if (hasFinished) { return; } - clearTimeout(timeoutHandler); - timeoutHandler = undefined; - client.close(); - - let parsed: Partial; - try { - parsed = NtpPacketParser.parse(msg); - } catch (err) { - hasFinished = true; - reject(err); + if (err) { + errorCallback(err); return; } - - const result: NtpReceivedPacket = { - ...parsed, - destinationTimestamp: new Date(), - }; - - hasFinished = true; - resolve(result); }); - }); + } catch (err) { + // dgram.send can throw synchronously (e.g. when the packet cannot be + // constructed or the socket is already in an unusable state) - make + // sure we still tear the socket down and reject the pending promise. + if (timeoutHandler !== undefined) { + clearTimeout(timeoutHandler); + timeoutHandler = undefined; + } + NtpTimeSync.cleanup(client); + if (!hasFinished) { + hasFinished = true; + reject(err); + } + } }); } @@ -458,6 +490,19 @@ export class NtpTimeSync { if (data.originTimestamp === undefined || data.originTimestamp.getTime() > new Date().getTime()) { throw new Error("Format error: Origin timestamp is from the future"); } + + /* + * Verify remaining fields required for sample computation + */ + if (data.transmitTimestamp === undefined) { + throw new Error("Format error: Missing transmit timestamp"); + } + if (data.receiveTimestamp === undefined) { + throw new Error("Format error: Missing receive timestamp"); + } + if (data.precision === undefined) { + throw new Error("Format error: Missing precision"); + } } /** diff --git a/src/index.ts b/src/index.ts index 3fe158d..f6e02db 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,2 +1,7 @@ -export * from "./NtpTimeResult.js"; -export * from "./NtpTimeSync.js"; +export type { NtpTimeResult } from "./NtpTimeResult.js"; +export { + NtpTimeSync, + NtpTimeSyncDefaultOptions, + type NtpTimeSyncConstructorOptions, + type NtpTimeSyncOptions, +} from "./NtpTimeSync.js"; diff --git a/test/unit/acceptResponse.test.ts b/test/unit/acceptResponse.test.ts new file mode 100644 index 0000000..af8deb8 --- /dev/null +++ b/test/unit/acceptResponse.test.ts @@ -0,0 +1,87 @@ +// Regression tests for acceptResponse's field validation. +// The method is private; we call it through a TypeScript cast to keep the +// test decoupled from the src/ visibility model (no production change). + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { NtpTimeSync } from "../../src/NtpTimeSync"; + +type AcceptResponseFn = (data: Record) => void; + +function getAcceptResponse(sync: NtpTimeSync): AcceptResponseFn { + const fn = (sync as unknown as { acceptResponse: AcceptResponseFn }).acceptResponse; + assert.equal(typeof fn, "function", "acceptResponse is present on the instance"); + return fn.bind(sync) as AcceptResponseFn; +} + +function baseValidPacket(): Record { + const now = new Date(); + // rootDelay / rootDispersion are Dates relative to the 1900 NTP epoch once + // parsed; use the epoch itself so (rootDelay - epoch)/1000 = 0. + const epoch1900 = new Date("Jan 01 1900 GMT"); + return { + version: 4, + leapIndicator: 0, + stratum: 2, + rootDelay: epoch1900, + rootDispersion: epoch1900, + originTimestamp: new Date(now.getTime() - 10), + receiveTimestamp: now, + transmitTimestamp: now, + precision: -18, + }; +} + +test("acceptResponse accepts a schema-valid packet with all required fields", () => { + const sync = new NtpTimeSync(); + const accept = getAcceptResponse(sync); + assert.doesNotThrow(() => accept(baseValidPacket())); +}); + +test("acceptResponse rejects packet missing originTimestamp", () => { + const sync = new NtpTimeSync(); + const accept = getAcceptResponse(sync); + const pkt = baseValidPacket(); + delete pkt.originTimestamp; + assert.throws(() => accept(pkt), /origin timestamp/i); +}); + +test("acceptResponse rejects packet missing transmitTimestamp", () => { + const sync = new NtpTimeSync(); + const accept = getAcceptResponse(sync); + const pkt = baseValidPacket(); + delete pkt.transmitTimestamp; + assert.throws(() => accept(pkt), /missing transmit timestamp/i); +}); + +test("acceptResponse rejects packet missing receiveTimestamp", () => { + const sync = new NtpTimeSync(); + const accept = getAcceptResponse(sync); + const pkt = baseValidPacket(); + delete pkt.receiveTimestamp; + assert.throws(() => accept(pkt), /missing receive timestamp/i); +}); + +test("acceptResponse rejects packet missing precision", () => { + const sync = new NtpTimeSync(); + const accept = getAcceptResponse(sync); + const pkt = baseValidPacket(); + delete pkt.precision; + assert.throws(() => accept(pkt), /missing precision/i); +}); + +test("acceptResponse rejects packet with originTimestamp in the future", () => { + const sync = new NtpTimeSync(); + const accept = getAcceptResponse(sync); + const pkt = baseValidPacket(); + pkt.originTimestamp = new Date(Date.now() + 60_000); + assert.throws(() => accept(pkt), /from the future/i); +}); + +test("acceptResponse rejects unsynchronized stratum (leapIndicator=3)", () => { + const sync = new NtpTimeSync(); + const accept = getAcceptResponse(sync); + const pkt = baseValidPacket(); + pkt.leapIndicator = 3; + assert.throws(() => accept(pkt), /unsynchronized/i); +}); diff --git a/test/unit/collectSamples-retry.test.ts b/test/unit/collectSamples-retry.test.ts new file mode 100644 index 0000000..8c525f5 --- /dev/null +++ b/test/unit/collectSamples-retry.test.ts @@ -0,0 +1,97 @@ +// Regression tests for collectSamples' retry accounting. +// The loop must increment `retry` on no-progress rounds and bail after 3 +// such rounds, preventing an infinite loop against a server set that never +// produces enough valid samples. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { NtpTimeSync } from "../../src/NtpTimeSync"; +import { buildNtpResponse, startFakeNtpServer } from "./helpers/ntpFixture"; + +test("collectSamples bails out after 3 no-progress retry rounds when no samples arrive", async () => { + // Server never replies; every round produces zero new samples → retry++ + // on each iteration. With replyTimeout=200ms and retry limit=3, total + // wall time is bounded near 4 * 200ms = ~800ms. + const server = await startFakeNtpServer(() => null); + try { + const sync = new NtpTimeSync({ + servers: [`127.0.0.1:${server.port}`], + sampleCount: 4, + replyTimeout: 150, + }); + + const start = Date.now(); + await assert.rejects( + () => sync.getTime(true), + /Unable to get any NTP response/i, + "getTime must reject with a connection error after retries exhaust" + ); + const elapsed = Date.now() - start; + + // 4 rounds * 150ms timeout = 600ms base. Allow generous safety margin + // but ensure we didn't loop unbounded. + assert.ok(elapsed < 3000, `must bail out quickly, took ${elapsed}ms`); + assert.ok(elapsed >= 400, `should observe at least a few timeouts, took ${elapsed}ms`); + } finally { + await server.close(); + } +}); + +test("collectSamples terminates when partial samples never reach target count", async () => { + // Server responds to every request, but we ask for more samples than we + // can produce in a single round. Because every round produces the SAME + // new sample count, ntpResults grows → retry stays at 0 for progress + // rounds and increments when the round produces duplicates / no growth. + // With 1 server + sampleCount=5, each round adds exactly 1 sample → no + // progress after round 1 because ntpResults.length keeps increasing... + // actually it IS progress. So to force termination we use sampleCount + // HIGHER than achievable and verify the implementation doesn't spin + // forever. + let requestCount = 0; + const server = await startFakeNtpServer(() => { + requestCount++; + // Return a malformed packet (wrong length) so the parser throws and + // acceptResponse never gets a valid sample. This keeps ntpResults + // length at 0, triggering the no-progress retry path. + return Buffer.alloc(10); // invalid length, parser will reject + }); + try { + const sync = new NtpTimeSync({ + servers: [`127.0.0.1:${server.port}`], + sampleCount: 3, + replyTimeout: 150, + }); + + const start = Date.now(); + await assert.rejects( + () => sync.getTime(true), + /Unable to get any NTP response/i, + "must reject when parser rejects every response" + ); + const elapsed = Date.now() - start; + + // 4 rounds should have occurred before bailing. + assert.ok(requestCount >= 3, `server should have received ≥3 requests, got ${requestCount}`); + assert.ok(elapsed < 3000, `bounded termination time, got ${elapsed}ms`); + } finally { + await server.close(); + } +}); + +test("collectSamples produces a result when sampleCount is satisfied in one round", async () => { + const server = await startFakeNtpServer(() => buildNtpResponse()); + try { + const sync = new NtpTimeSync({ + servers: [ + `127.0.0.1:${server.port}`, + `127.0.0.1:${server.port}`, + ], + sampleCount: 2, + replyTimeout: 500, + }); + const result = await sync.getTime(true); + assert.ok(Number.isFinite(result.offset), "offset produced when targets met in one round"); + } finally { + await server.close(); + } +}); diff --git a/test/unit/createPacket.test.ts b/test/unit/createPacket.test.ts new file mode 100644 index 0000000..ec673e0 --- /dev/null +++ b/test/unit/createPacket.test.ts @@ -0,0 +1,105 @@ +// Regression tests for the NTP v4 packet layout produced by NtpTimeSync. +// The packet builder is private; we observe it indirectly by sending a real +// getTime() call against an in-process UDP server that captures the datagram. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { NtpTimeSync } from "../../src/NtpTimeSync"; +import { buildNtpResponse, startFakeNtpServer } from "./helpers/ntpFixture"; + +const NTP_EPOCH_OFFSET_SECONDS = 2208988800; + +function readNtpTimestampAsDate(buf: Buffer, offset: number): Date { + const value = buf.readBigUInt64BE(offset); + const mask32 = BigInt("0xffffffff"); + const shift32 = BigInt(32); + const seconds = Number(value >> shift32); + const fractional = Number(value & mask32); + const unixMs = (seconds - NTP_EPOCH_OFFSET_SECONDS) * 1000 + (fractional / 2 ** 32) * 1000; + return new Date(unixMs); +} + +test("createPacket produces a 48-byte NTP v4 client packet", async () => { + const server = await startFakeNtpServer(() => buildNtpResponse()); + try { + const sync = new NtpTimeSync({ + servers: [`127.0.0.1:${server.port}`], + sampleCount: 1, + replyTimeout: 500, + }); + await sync.getTime(true); + + assert.ok(server.receivedPackets.length >= 1, "server received at least one packet"); + const packet = server.receivedPackets[0]; + assert.equal(packet.length, 48, "packet length is 48 bytes"); + + // Byte 0: LI (2) | VN (3) | Mode (3) + // Default leapIndicator = 3 (unsynchronized), version = 4, mode = 3 (client) + const byte0 = packet[0]; + const leapIndicator = (byte0 >> 6) & 0x3; + const version = (byte0 >> 3) & 0x7; + const mode = byte0 & 0x7; + assert.equal(leapIndicator, 3, "leap indicator bits 6-7 encode default value 3"); + assert.equal(version, 4, "version bits 3-5 encode NTP v4"); + assert.equal(mode, 3, "mode bits 0-2 encode client (3)"); + } finally { + await server.close(); + } +}); + +test("createPacket writes a BE 64-bit timestamp at offset 24 that matches offset 40", async () => { + const server = await startFakeNtpServer(() => buildNtpResponse()); + try { + const sync = new NtpTimeSync({ + servers: [`127.0.0.1:${server.port}`], + sampleCount: 1, + replyTimeout: 500, + }); + const sentAt = Date.now(); + await sync.getTime(true); + const receivedAt = Date.now(); + + const packet = server.receivedPackets[0]; + + const originRaw = packet.readBigUInt64BE(24); + const transmitRaw = packet.readBigUInt64BE(40); + assert.equal(originRaw, transmitRaw, "origin (24) and transmit (40) timestamps are written with the same value"); + + const decoded = readNtpTimestampAsDate(packet, 24); + const decodedMs = decoded.getTime(); + // Allow generous envelope for wall-clock jitter; timestamps must fall + // between the pre-call time and post-call time plus safety margin. + assert.ok( + decodedMs >= sentAt - 5 && decodedMs <= receivedAt + 50, + `decoded timestamp ${decodedMs} should be between ${sentAt - 5} and ${receivedAt + 50}` + ); + } finally { + await server.close(); + } +}); + +test("createPacket round-trips an NTP timestamp through big-endian write/read", async () => { + // Independently verify the big-endian encoding logic against a known Date. + // This re-implements the src-side math to lock the byte layout in place. + const knownDate = new Date("2024-06-15T12:34:56.789Z"); + const baseMs = knownDate.getTime() - new Date("Jan 01 1900 GMT").getTime(); + const seconds = Math.trunc(baseMs / 1000); + const fractional = Math.trunc(((baseMs % 1000) / 1000) * 2 ** 32); + const mask32 = BigInt("0xffffffff"); + const shift32 = BigInt(32); + const ntpTimestamp = ((BigInt(seconds) & mask32) << shift32) | (BigInt(fractional) & mask32); + + const buf = Buffer.alloc(48); + buf.writeBigUInt64BE(ntpTimestamp, 24); + buf.writeBigUInt64BE(ntpTimestamp, 40); + + // High 32 bits stored at offset 24 (big-endian) should decode back to seconds. + const decodedSeconds = buf.readUInt32BE(24); + const decodedFraction = buf.readUInt32BE(28); + assert.equal(decodedSeconds, seconds, "seconds word stored big-endian at offset 24"); + assert.equal(decodedFraction, fractional, "fraction word stored big-endian at offset 28"); + + // Same for offset 40. + assert.equal(buf.readUInt32BE(40), seconds); + assert.equal(buf.readUInt32BE(44), fractional); +}); diff --git a/test/unit/helpers/ntpFixture.ts b/test/unit/helpers/ntpFixture.ts new file mode 100644 index 0000000..ac4936a --- /dev/null +++ b/test/unit/helpers/ntpFixture.ts @@ -0,0 +1,146 @@ +// Helpers for building synthetic NTP response packets and running an in-process +// UDP server. Kept deterministic and offline so the unit suite can complete +// in well under a second without any external network access. + +import * as dgram from "node:dgram"; +import { AddressInfo } from "node:net"; + +const NTP_EPOCH_OFFSET_SECONDS = 2208988800; // 1970-01-01 in NTP seconds-since-1900 + +export interface BuildPacketOptions { + leapIndicator?: number; + version?: number; + mode?: number; + stratum?: number; + poll?: number; + /** Raw byte value (0-255). -18 would be written as 0xee (two's complement). */ + precisionByte?: number; + /** Root delay in seconds (added to NTP epoch when written). */ + rootDelaySeconds?: number; + rootDispersionSeconds?: number; + referenceId?: number; + referenceTimestamp?: Date; + originTimestamp?: Date; + receiveTimestamp?: Date; + transmitTimestamp?: Date; + /** + * When true, zero out the 64-bit slot for the field. This lets tests + * produce packets whose parsed timestamps come back as 1900-01-01 to + * validate acceptResponse missing-field handling while still keeping a + * schema-correct 48-byte buffer. + */ + blankReceiveTimestamp?: boolean; + blankTransmitTimestamp?: boolean; + blankOriginTimestamp?: boolean; +} + +function writeNtpTimestamp(buf: Buffer, offset: number, date: Date): void { + // Convert a JS Date to an NTP 64-bit timestamp and write big-endian. + const unixMs = date.getTime(); + const totalSeconds = unixMs / 1000 + NTP_EPOCH_OFFSET_SECONDS; + const seconds = Math.trunc(totalSeconds); + const fractional = Math.trunc((totalSeconds - seconds) * 2 ** 32); + const mask32 = BigInt("0xffffffff"); + const shift32 = BigInt(32); + const value = ((BigInt(seconds) & mask32) << shift32) | (BigInt(fractional) & mask32); + buf.writeBigUInt64BE(value, offset); +} + +function writeSecondsSinceEpoch(buf: Buffer, offset: number, seconds: number): void { + // rootDelay/rootDispersion use a 32-bit "seconds since NTP epoch" field + // that ntp-packet-parser converts into a Date relative to 1900-01-01. + const scaled = Math.trunc(seconds); + buf.writeUInt32BE(scaled, offset); +} + +/** + * Build a 48-byte NTP response packet. Defaults produce a schema-valid, sane + * packet that acceptResponse should accept. + */ +export function buildNtpResponse(opts: BuildPacketOptions = {}): Buffer { + const now = new Date(); + const buf = Buffer.alloc(48); + + const leapIndicator = opts.leapIndicator ?? 0; + const version = opts.version ?? 4; + const mode = opts.mode ?? 4; // server + buf[0] = ((leapIndicator & 0x3) << 6) | ((version & 0x7) << 3) | (mode & 0x7); + + buf[1] = opts.stratum ?? 2; + buf[2] = opts.poll ?? 4; + // precision byte: default 0xee == -18 two's complement, interpreted by + // ntp-packet-parser as the raw unsigned byte 238. acceptResponse only + // checks "!== undefined", so any non-missing byte is accepted. + buf[3] = opts.precisionByte ?? 0xee; + + writeSecondsSinceEpoch(buf, 4, opts.rootDelaySeconds ?? 0); + writeSecondsSinceEpoch(buf, 8, opts.rootDispersionSeconds ?? 0); + buf.writeUInt32BE(opts.referenceId ?? 0, 12); + + // reference timestamp (offset 16) + writeNtpTimestamp(buf, 16, opts.referenceTimestamp ?? now); + // origin timestamp (offset 24) + if (!opts.blankOriginTimestamp) { + writeNtpTimestamp(buf, 24, opts.originTimestamp ?? new Date(now.getTime() - 1)); + } + // receive timestamp (offset 32) + if (!opts.blankReceiveTimestamp) { + writeNtpTimestamp(buf, 32, opts.receiveTimestamp ?? now); + } + // transmit timestamp (offset 40) + if (!opts.blankTransmitTimestamp) { + writeNtpTimestamp(buf, 40, opts.transmitTimestamp ?? now); + } + + return buf; +} + +export interface FakeNtpServer { + port: number; + receivedPackets: Buffer[]; + close(): Promise; +} + +/** + * Start a UDP server on 127.0.0.1:0 that echoes a synthetic reply for every + * inbound NTP request. The reply is produced by `buildReply(requestBuffer)`. + * Returning `null` suppresses the reply (tests can simulate drop / timeout). + */ +export async function startFakeNtpServer( + buildReply: (request: Buffer) => Buffer | null +): Promise { + const socket = dgram.createSocket("udp4"); + const received: Buffer[] = []; + + socket.on("message", (msg: Buffer, rinfo) => { + received.push(Buffer.from(msg)); + const reply = buildReply(msg); + if (reply) { + socket.send(reply, 0, reply.length, rinfo.port, rinfo.address); + } + }); + + await new Promise((resolve, reject) => { + socket.once("error", reject); + socket.bind(0, "127.0.0.1", () => { + socket.removeListener("error", reject); + resolve(); + }); + }); + + const addr = socket.address() as AddressInfo; + + return { + port: addr.port, + receivedPackets: received, + close: () => + new Promise((resolve) => { + socket.removeAllListeners(); + try { + socket.close(() => resolve()); + } catch { + resolve(); + } + }), + }; +} diff --git a/test/unit/instance-isolation.test.ts b/test/unit/instance-isolation.test.ts new file mode 100644 index 0000000..fb3743c --- /dev/null +++ b/test/unit/instance-isolation.test.ts @@ -0,0 +1,83 @@ +// Regression test for the module-level-state → instance-state refactor. +// Two independent NtpTimeSync instances with different server lists must +// maintain independent caches. This test fails against the old code where +// lastResult / samples lived at module scope. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { NtpTimeSync } from "../../src/NtpTimeSync"; +import { buildNtpResponse, startFakeNtpServer } from "./helpers/ntpFixture"; + +test("two NtpTimeSync instances with different servers keep independent caches", async () => { + // Server A always reports a time 1 second BEHIND wall clock. + const serverA = await startFakeNtpServer(() => { + const skewed = new Date(Date.now() - 1000); + return buildNtpResponse({ + receiveTimestamp: skewed, + transmitTimestamp: skewed, + originTimestamp: new Date(Date.now() - 5), + }); + }); + // Server B always reports a time 1 second AHEAD of wall clock. + const serverB = await startFakeNtpServer(() => { + const skewed = new Date(Date.now() + 1000); + return buildNtpResponse({ + receiveTimestamp: skewed, + transmitTimestamp: skewed, + originTimestamp: new Date(Date.now() - 5), + }); + }); + + try { + const syncA = new NtpTimeSync({ + servers: [`127.0.0.1:${serverA.port}`], + sampleCount: 1, + replyTimeout: 500, + }); + const syncB = new NtpTimeSync({ + servers: [`127.0.0.1:${serverB.port}`], + sampleCount: 1, + replyTimeout: 500, + }); + + const resultA = await syncA.getTime(true); + const resultB = await syncB.getTime(true); + + // Offsets must reflect each server's intentional skew, not be cross- + // contaminated by the other instance. + assert.ok(resultA.offset < -500, `A should report negative offset (~-1000ms), got ${resultA.offset}`); + assert.ok(resultB.offset > 500, `B should report positive offset (~+1000ms), got ${resultB.offset}`); + + // Re-read cache; non-forced calls must return each instance's own value. + const cachedA = await syncA.getTime(false); + const cachedB = await syncB.getTime(false); + assert.equal(cachedA.offset, resultA.offset, "A's cache survives B's invocation"); + assert.equal(cachedB.offset, resultB.offset, "B's cache survives A's invocation"); + + // And they are distinct. + assert.notEqual(cachedA.offset, cachedB.offset, "A and B have different cached offsets"); + + // Internal state inspection (via TS cast): samples arrays must be + // separate Array instances, not a shared module-level variable. + const samplesA = (syncA as unknown as { samples: unknown[] }).samples; + const samplesB = (syncB as unknown as { samples: unknown[] }).samples; + assert.notEqual(samplesA, samplesB, "samples arrays are separate objects per instance"); + } finally { + await serverA.close(); + await serverB.close(); + } +}); + +test("instance options are not shared between constructions", () => { + const a = new NtpTimeSync({ servers: ["a.example"], sampleCount: 2 }); + const b = new NtpTimeSync({ servers: ["b.example"], sampleCount: 7 }); + + const optsA = (a as unknown as { options: { servers: Array<{ host: string }>; sampleCount: number } }).options; + const optsB = (b as unknown as { options: { servers: Array<{ host: string }>; sampleCount: number } }).options; + + assert.equal(optsA.servers[0].host, "a.example"); + assert.equal(optsB.servers[0].host, "b.example"); + assert.equal(optsA.sampleCount, 2); + assert.equal(optsB.sampleCount, 7); + assert.notEqual(optsA, optsB, "options objects are distinct"); +}); diff --git a/test/unit/samples-math.test.ts b/test/unit/samples-math.test.ts new file mode 100644 index 0000000..90c664a --- /dev/null +++ b/test/unit/samples-math.test.ts @@ -0,0 +1,111 @@ +// Regression tests for the offset/delay/dispersion math in collectSamples. +// The math lives in a private method; we validate it through the public +// getTime() path against a deterministic fake server. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { NtpTimeSync } from "../../src/NtpTimeSync"; +import { buildNtpResponse, startFakeNtpServer } from "./helpers/ntpFixture"; + +test("getTime returns a result with offset and precision fields", async () => { + const server = await startFakeNtpServer(() => buildNtpResponse()); + try { + const sync = new NtpTimeSync({ + servers: [`127.0.0.1:${server.port}`], + sampleCount: 1, + replyTimeout: 500, + }); + const result = await sync.getTime(true); + assert.ok(result.now instanceof Date, "result.now is a Date"); + assert.equal(typeof result.offset, "number", "result.offset is numeric"); + assert.equal(typeof result.precision, "number", "result.precision is numeric"); + assert.ok(Number.isFinite(result.offset), "offset is finite"); + assert.ok(result.precision >= 0, "precision (stddev) is non-negative"); + } finally { + await server.close(); + } +}); + +test("getTime with receive=transmit=now from fake server yields near-zero offset", async () => { + // When a server's receive and transmit timestamps match the client's + // destination time, the offset formula ((|T2-T1| + |T3-T4|) / 2) must + // be small — bounded by RTT and wall-clock jitter. + const server = await startFakeNtpServer((req) => { + // Echo the request timestamp back as origin; use current time for + // receive/transmit to minimize apparent offset. + const now = new Date(); + return buildNtpResponse({ + receiveTimestamp: now, + transmitTimestamp: now, + originTimestamp: new Date(Date.now() - 1), + }); + }); + try { + const sync = new NtpTimeSync({ + servers: [`127.0.0.1:${server.port}`], + sampleCount: 1, + replyTimeout: 500, + }); + const result = await sync.getTime(true); + // Loopback RTT should be <100ms; offset magnitude must stay bounded. + assert.ok( + Math.abs(result.offset) < 500, + `|offset| should be under 500ms against a local fake, got ${result.offset}` + ); + } finally { + await server.close(); + } +}); + +test("getTime caches result across invocations within minPoll window", async () => { + const server = await startFakeNtpServer(() => buildNtpResponse()); + try { + const sync = new NtpTimeSync({ + servers: [`127.0.0.1:${server.port}`], + sampleCount: 1, + replyTimeout: 500, + }); + await sync.getTime(true); + const firstPacketCount = server.receivedPackets.length; + + // Non-forced call within minPoll window should hit the cache. + await sync.getTime(false); + const secondPacketCount = server.receivedPackets.length; + + assert.equal( + secondPacketCount, + firstPacketCount, + "cached getTime must not send another packet" + ); + + // Forced call bypasses the cache. + await sync.getTime(true); + assert.ok( + server.receivedPackets.length > secondPacketCount, + "force=true must trigger a new network round" + ); + } finally { + await server.close(); + } +}); + +test("getTime with 3 samples averages offsets across responses", async () => { + // Three back-to-back identical responses must produce a stable offset. + const server = await startFakeNtpServer(() => buildNtpResponse()); + try { + const sync = new NtpTimeSync({ + servers: [ + `127.0.0.1:${server.port}`, + `127.0.0.1:${server.port}`, + `127.0.0.1:${server.port}`, + ], + sampleCount: 3, + replyTimeout: 500, + }); + const result = await sync.getTime(true); + assert.ok(Number.isFinite(result.offset)); + assert.ok(result.precision >= 0); + } finally { + await server.close(); + } +}); diff --git a/test/unit/socket-cleanup.test.ts b/test/unit/socket-cleanup.test.ts new file mode 100644 index 0000000..70ca906 --- /dev/null +++ b/test/unit/socket-cleanup.test.ts @@ -0,0 +1,102 @@ +// Regression tests for socket lifecycle: cleanup uses removeAllListeners + +// close, and a synchronous throw from createPacket/send must not leak a +// dgram socket. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import * as dgram from "node:dgram"; +import { NtpTimeSync } from "../../src/NtpTimeSync"; +import { startFakeNtpServer, buildNtpResponse } from "./helpers/ntpFixture"; + +interface ActiveHandle { + constructor?: { name?: string }; +} + +function countUdpHandles(): number { + // Use Node's debug API to count active UDP socket handles. This is + // deprecated/private but stable across LTS versions and suits a + // regression check that cleanup() actually releases the handle. + const getActiveHandles = (process as unknown as { _getActiveHandles?: () => ActiveHandle[] })._getActiveHandles; + if (typeof getActiveHandles !== "function") return -1; + return getActiveHandles + .call(process) + .filter((h) => h?.constructor?.name === "UDP" || h?.constructor?.name === "Socket") + .length; +} + +test("socket handles do not accumulate across repeated getNetworkTime calls", async () => { + // Fix #6 guards against socket leaks. A leak would show up as a steadily + // growing active-handle count across many iterations. Run enough rounds + // to notice any retained-per-call handle. + const server = await startFakeNtpServer(() => buildNtpResponse()); + try { + const sync = new NtpTimeSync({ + servers: [`127.0.0.1:${server.port}`], + sampleCount: 1, + replyTimeout: 500, + }); + + // Warm-up to stabilize the baseline + await sync.getTime(true); + // Give Node one tick to close sockets from the warm-up round + await new Promise((r) => setImmediate(r)); + const baseline = countUdpHandles(); + + for (let i = 0; i < 10; i++) { + await sync.getTime(true); + } + await new Promise((r) => setImmediate(r)); + + const after = countUdpHandles(); + // Tolerate +1 for the fake server's own socket variations; any larger + // delta implies a leaked client socket per iteration. + assert.ok( + after - baseline <= 1, + `UDP handle count grew from ${baseline} to ${after}, suggesting a socket leak` + ); + } finally { + await server.close(); + } +}); + +test("getNetworkTime rejects and cleans up when sending to an unreachable port", async () => { + // Closed loopback port → kernel sends ICMP unreachable → socket errors. + // This exercises the error-callback path where cleanup must remove all + // listeners and close the socket before rejecting. + const sync = new NtpTimeSync({ + servers: ["127.0.0.1:1"], + sampleCount: 1, + replyTimeout: 300, + }); + + await assert.rejects(() => sync.getNetworkTime("127.0.0.1", 1)); + // If cleanup failed, a UDP handle would linger past a tick. + await new Promise((r) => setImmediate(r)); + // No assertion on count because other tests may have warmed handles up; + // we just assert the rejection path completes without hanging. +}); + +test("reply that arrives before send-callback completes is still captured", async () => { + // This indirectly verifies fix #4: the message listener must be wired + // up BEFORE client.send() is called. Our fake server responds + // synchronously as soon as it receives a packet, which stresses that + // ordering — if the message listener were registered after send, a + // very fast reply could slip through the gap. Against a loopback UDP + // server the round-trip is microseconds, so this tightens the race + // window. + const server = await startFakeNtpServer(() => buildNtpResponse()); + try { + const sync = new NtpTimeSync({ + servers: [`127.0.0.1:${server.port}`], + sampleCount: 1, + replyTimeout: 500, + }); + // Run several iterations to exercise the race window repeatedly. + for (let i = 0; i < 5; i++) { + const result = await sync.getTime(true); + assert.ok(result.now instanceof Date, `iteration ${i}: message was captured`); + } + } finally { + await server.close(); + } +});