v1.3.9 - #71
Merged
Merged
Conversation
Array.concat returns a new array without mutating, so the prior loop discarded every per-packet TimedSendResult and always returned an empty array. SendResultFailedRatio short-circuits empty input to 0, meaning probe and query broadcasts silently treated total socket failure as success. Use push(...value) so callers see real failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The constructed Error included result.reason.name (always "Error") where result.interface was intended, so log lines never identified which socket actually failed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When a loopback interface appeared after startup, the new-interface branch wrote it to currentInterfaces twice instead of populating loopbackInterfaces. isLoopbackNetaddressV4 then never matched it, so the cross-interface packet filter on Linux failed for runtime-added loopbacks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
console.log/error calls in the Responder, Announcer and Prober send-failure paths passed a trailing `true` that the previous version of these helpers did not accept, so it was silently formatted into the log output. Pass only the formatted error string. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
handleServiceRecordUpdate wired up the success branch of the broadcast promise but had no rejection handler. Encode/assert failures inside sendResponseBroadcast would therefore surface as an unhandled promise rejection, matching the orphan-rejection failure mode addressed in #69. Forward the failure through the optional callback and log it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Math.ceil((type & 0xFF) / 8) underflowed by one for rrtypes that are exact multiples of 8 — for instance an NSEC containing only TXT (16) asked for a 2-byte bitmap but encoded into byte index 2. In current usage another type in the same window always saved it via Math.max, but the formula was latent. Use (low >> 3) + 1 so the bitmap always covers the byte holding the highest type. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Coverage Report for CI Build 26375547901Coverage increased (+7.2%) to 43.695%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Pull request overview
This PR prepares the v1.3.9 release by tightening mDNS/DNS-SD responder robustness (network/interface race handling, failure propagation), fixing encoding edge cases (NSEC bitmap sizing, local-compression pointer decoding), and applying small refactors/perf improvements with accompanying regression tests.
Changes:
- Fixes send/error-path behaviors (accumulate per-packet send results, remove stray log args, propagate broadcast rejections, convert sync send throws into rejected results).
- Corrects/optimizes low-level record/coder logic (NSEC bitmap sizing; local-compression pointer offset sign; Buffer.compare for buffer equality).
- Improves operational behavior and docs (unref truncated-query timer; clarify
advertiseIpv6default; dependency/workflow bumps).
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/util/domain-formatter.ts | Removes redundant assertion; replaces deprecated substr with slice. |
| src/responder/TruncatedQuery.ts | Uses Date.now() and unref()s the finalisation timer to avoid pinning the event loop. |
| src/responder/TruncatedQuery.spec.ts | Adds regression tests asserting the timer is unref’d (including after reset). |
| src/responder/QueuedResponse.ts | Uses Date.now() for timestamp math. |
| src/responder/Prober.ts | Removes stray boolean argument from probe send-failure logging. |
| src/responder/Announcer.ts | Uses Date.now(), fixes logging arg shape, and wraps sync send() throws into rejected TimedSendResult. |
| src/responder/Announcer.spec.ts | Adds tests ensuring sync send() throws don’t escape and become rejected results. |
| src/Responder.ts | Uses Date.now(), handles sendResponseBroadcast rejections in record-update path, uses Buffer.compare, replaces substr with slice. |
| src/Responder.spec.ts | Adds regression tests for record-update rejection handling and log-call argument shape. |
| src/NetworkManager.ts | Fixes loopback tracking for newly added interfaces; skips interfaces with no usable addresses instead of asserting. |
| src/NetworkManager.spec.ts | Adds regression tests for loopback tracking and address-less interface skipping (including excludeIpv6). |
| src/MDNSServer.ts | Fixes sendQueryBroadcast aggregation; improves sendResponse error message interface reporting; clarifies IPv6 advertise default. |
| src/MDNSServer.spec.ts | Adds regression tests for query broadcast aggregation and sendResponse error reporting. |
| src/coder/records/TXTRecord.ts | Uses Buffer.compare for buffer equality (perf/clarity). |
| src/coder/records/OPTRecord.ts | Uses Buffer.compare for option data equality (perf/clarity). |
| src/coder/records/NSECRecord.ts | Collapses identical branches; fixes rrtype bitmap sizing to avoid buffer under-allocation at byte boundaries. |
| src/coder/records/NSECRecord.spec.ts | Adds tests covering byte-aligned rrtype bitmap sizing and roundtrip encode/decode. |
| src/coder/DNSLabelCoder.ts | Fixes local-compression pointer offset sign; improves related assertion message location. |
| src/coder/DNSLabelCoder.spec.ts | Adds regression test for >=256 (RData-relative) local-compression pointer decoding. |
| package.json | Updates devDependencies (types, eslint tooling, jest, ts-jest). |
| package-lock.json | Lockfile updates for dependency bumps. |
| CHANGELOG.md | Adds v1.3.9 (Pending Release) entry summarizing changes. |
| .github/workflows/release.yml | Bumps action versions used in the release workflow. |
The "10" label-type branch used `localPointer -= -256`, which adds 256
where the comment ("subtract the offset 256") and the surrounding logic
require subtraction. Local compression draft pointers are extremely
rare in practice, but the typo would corrupt any decode that ever
exercised this branch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The function asserted net.isIPv4 unconditionally on the line below the guarded conditional, so the conditional path was dead. The function takes an IPv4 input regardless of mapped status, so collapse to the single assertion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
substr is a legacy feature flagged for removal. Use slice with the equivalent semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Avoids allocating a Date object on every timestamp read across Responder, Announcer, QueuedResponse and TruncatedQuery. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace per-byte hex stringification in TXTRecord.dataEquals, OPTRecord.optionsEquality and Responder's TXT conflict check with Buffer.compare, which compares raw bytes directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both arms of the legacyUnicastEncoding ternaries called the uncompressed helpers — the comment explains compression is intentionally disabled here for Ubiquiti compatibility — so the ternary was a no-op. Inline the uncompressed call. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The docstring claimed IPv6 advertising defaulted to true, but the constructor only enables it when advertiseIpv6 is explicitly truthy — so the actual default is false. Update the doc to match the code rather than flipping the default and changing observed behaviour. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… loop The 400-500ms timeout that finalises an incoming truncated-query sequence was created without unref. If the remaining packets never arrive while the host is otherwise idle, the timer keeps the Node event loop alive until it fires. Mirror the unref pattern used by the other timers in the responder. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MDNSServer.send throws InterfaceNotFoundError synchronously when the socket for the requested interface has been torn down. Inside Announcer.sendResponseAddingAddressRecords that throw escaped the Promise.race wrapper, propagated through sendAnnouncement (invoked via setTimeout) and would surface as an uncaughtException — crashing the host process when an interface vanished mid-announce. Catch the synchronous throw and surface it as a rejected SendResult so the existing failure-ratio handling can deal with it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
getCurrentNetworkInterfaces asserted that every listed interface exposed at least one usable IP address. The newer ip -o link show helper enumerates more virtual/down interfaces, and disableIpv6 can filter out the only addresses on an IPv6-only link — both legitimate inputs that previously crashed the entire interface scan via the assertion. Continue past such interfaces and log a debug line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The JSDoc on Responder.handleQuery read "needs ot be handled" — a typo present since 2020. Flagged by the Copilot review on #71; correcting opportunistically while the release branch is open. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NorthernMan54
approved these changes
May 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.