dns: consistency; performance on malicious input #20445

davisjam · 2018-05-01T01:38:28Z

This PR addresses two issues in lib/dns.js.

Inconsistent behavior on some invalid servers, dns: setServers throws inconsistent TypeError #20441
Quadratic regex, a potential performance issue, dns: setServers has super-linear regex #20443

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Behavior: dns.setServers throws a null pointer dereference on some inputs. Expected behavior was the more pleasant TypeError [ERR_INVALID_IP_ADDRESS] ... Root cause(s?): - Dereferencing the result of a regex match without confirming that there was a match. - assuming the capture of an optional group (?) Solution: Confirm the match, and handle a missing port cleanly. Tests: I added tests for various unusual inputs. Fixes: nodejs#20441

davisjam · 2018-05-01T01:40:30Z

test/parallel/test-dns.js

+    '['.repeat(100000) // REDOS
+  ];
+  invalidServers.forEach((serv) => {
+    assert.throws(() => dns.setServers([serv]), /TypeError.*ERR_INVALID_IP_ADDRESS/, `Unexpected error thrown for ${serv}`);


This line is too long, but I'm not sure how best to format it. I tried the obvious indentation but eslint complained that the TypeError ought to be indented 18 spaces (??).

davisjam · 2018-05-01T01:52:00Z

test/parallel/test-dns.js

+    '['.repeat(100000) // REDOS
+  ];
+  invalidServers.forEach((serv) => {
+    assert.throws(() => dns.setServers([serv]), /TypeError.*ERR_INVALID_IP_ADDRESS/, `Unexpected error thrown for ${serv}`);


The correct error type is Error, not TypeError.

davisjam · 2018-05-01T01:55:25Z

test/parallel/test-dns.js

+  ];
+  invalidServers.forEach((serv) => {
+    // The error type is undocumented.
+    assert.throws(() => dns.setServers([serv]), /Error.*ERR_INVALID_IP_ADDRESS/, `Unexpected error thrown for ${serv}`);


This line is too long, but linter complains if I try to break it up?

It depends on how you break it up. The following will definitely work.

assert.throws( () => foo, /abc/, 'bar' );

BridgeAR

In general this is LG. Just left a few comments.

BridgeAR · 2018-05-01T02:01:18Z

lib/dns.js

+    // IPv6: /\[(.*)\]/ but O(n) complexity
+    const openBracketIx = serv.indexOf('[');
+    const nextCloseBracketIx = serv.indexOf(']', openBracketIx);
+    if (0 <= openBracketIx && openBracketIx < nextCloseBracketIx) {


Suggestion: change it to e.g.,

const openBracketIx = serv.indexOf('['); if (openBracketIx !== -1) { const nextCloseBracketIx = serv.indexOf(']', openBracketIx); if (nextCloseBracketIx > openBracketIx) { const withinBrackets = ... ... } }

As written I believe the code is correct: if openBracketIx is -1 then indexOf treats it as 0.

Is your suggestion about correctness or readability?

Neither. It is just faster because it will not check for the closing bracket in case the first one does not exist.

It also looks more Obviously Correct.

Apropos the names, can you use fooIndex instead of fooIx?

BridgeAR · 2018-05-01T02:03:38Z

test/parallel/test-dns.js

+  ];
+  invalidServers.forEach((serv) => {
+    // The error type is undocumented.
+    assert.throws(() => dns.setServers([serv]), /Error.*ERR_INVALID_IP_ADDRESS/, `Unexpected error thrown for ${serv}`);


Please use the object notation as in:

assert.throws( () => fn, { name: 'ErrorName', code: 'ERR_INVALID_IP_ADDRESS' }, `Unexpected error thrown for ${serv}` );

Great, thanks

BridgeAR · 2018-05-01T02:05:23Z

lib/dns.js

  const addrSplitRE = /(^.+?)(?::(\d+))?$/;

  servers.forEach((serv) => {
+    // Easily recognized?


I am not sure what the comment stands for.

This is the case "we can easily recognize what this server is"

I would personally prefer a comment that's a descriptive statement. The question mark makes it pretty unclear.

BridgeAR · 2018-05-01T02:05:58Z

lib/dns.js

+    const addrSplitMatch = serv.match(addrSplitRE);
+    if (addrSplitMatch) {
+      const hostIP = addrSplitMatch[1];
+      const port = addrSplitMatch[2] || IANA_DNS_PORT;


Can you explain the default value here? That is something unexpected for me.

addrSplitMatch[2] might be undefined (see #20441), in which case I figured the port should be 53 (DNS port).

I think this is consistent with existing cases where a default port is selected, e.g. here and here.

@bnoordhuis @apapirovski PTAL

I think it should be fine. parseInt('') works out to 0 when coerced to int32 and c-ares uses the default port in that case. IOW, there should be no observable change in behavior, it'll use port 53 either way.

BridgeAR · 2018-05-18T11:03:51Z

@davisjam would you be so kind and address the comments? :-)

bnoordhuis · 2018-05-18T11:15:34Z

lib/dns.js

+    // IPv6: /\[(.*)\]/ but O(n) complexity
+    const openBracketIx = serv.indexOf('[');
+    const nextCloseBracketIx = serv.indexOf(']', openBracketIx);
+    if (0 <= openBracketIx && openBracketIx < nextCloseBracketIx) {


It also looks more Obviously Correct.

Apropos the names, can you use fooIndex instead of fooIx?

bnoordhuis · 2018-05-18T11:19:22Z

lib/dns.js

+    const nextCloseBracketIx = serv.indexOf(']', openBracketIx);
+    if (0 <= openBracketIx && openBracketIx < nextCloseBracketIx) {
+      const withinBrackets = serv.slice(openBracketIx + 1, nextCloseBracketIx);
+      ipVersion = isIP(withinBrackets);


Can't you match using /\[([^\]]+)\]/? Succinct and has linear complexity... although it does look as if the cat sat on your keyboard.

Actually, /\[[^\]]+\]/ is still quadratic under input like '['.repeat(N) + 'a'. Lacking a leading ^ anchor, the regex engine will try each of the [s as a possible start point and will take linear time to reject each one.

Something like /\[([^\[\]]+)\]/ would be safe but:

The cat's revenge is complete, and

It couldn't handle nested braces like [[]] (not sure if that's relevant in the DNS spec).

See also: vuln-regex-detector #49.

Right, I mean the anchored version. Nested square brackets shouldn't be an issue, an IPv6 address only has the two.

bnoordhuis · 2018-05-18T11:27:15Z

lib/dns.js

+    const addrSplitMatch = serv.match(addrSplitRE);
+    if (addrSplitMatch) {
+      const hostIP = addrSplitMatch[1];
+      const port = addrSplitMatch[2] || IANA_DNS_PORT;


I think it should be fine. parseInt('') works out to 0 when coerced to int32 and c-ares uses the default port in that case. IOW, there should be no observable change in behavior, it'll use port 53 either way.

davisjam · 2018-05-18T13:34:50Z

@BridgeAR On vacation at the moment.

BridgeAR · 2018-05-29T20:06:42Z

Ping @davisjam

davisjam · 2018-05-31T03:53:37Z

@BridgeAR Good timing, let's see...

davisjam · 2018-05-31T04:34:24Z

I've decided to KISS, using a safe regex rather than the more complex logic needed to replace it. The proposed regex is slightly pickier (e.g. won't allow leading characters, like whitespace). Let me know if you want a trim for somewhat more backwards compatibility.

Problem: The IPv6 regex was quadratic. On long malicious input the event loop could block. The security team did not deem it a security risk, but said a PR was welcome. Solution: Revise the regex to a linear-complexity version. Tests: I added REDOS tests to the "oddities" section. Fixes: nodejs#20443

yorkie

The consistency looks good, but as for performance, I think some benchmark should be added as well :)

yorkie · 2018-05-31T05:48:41Z

test/parallel/test-dns.js

+    '['.repeat(100000) + ']'.repeat(100000) + 'a'
+  ];
+  invalidServers.forEach((serv) => {
+    assert.throws(() => dns.setServers([serv]), /TypeError.*ERR_INVALID_IP_ADDRESS/, `Unexpected error thrown for ${serv}`);


This should exceed the max line number :(

Ah yes I recall a comment about that earlier. I need to fix this.

davisjam · 2018-05-31T20:47:37Z

@yorkie Can you be more specific about your performance concern?

yorkie · 2018-06-01T02:30:46Z

@davisjam just suggest if a benchmark provided, we can get where the performance is optimized :)

davisjam · 2018-06-01T03:42:34Z

@yorkie See the test cases here. These run slowly without the patch.

davisjam · 2018-06-01T03:57:30Z

@yorkie See changes in 03451b4 to address your comments (and those of @BridgeAR about test case format).

yorkie · 2018-06-01T04:11:39Z

@davisjam What I mean is in https://github.com/nodejs/node/tree/master/benchmark/dns, it consumes the cost precisely :)

davisjam · 2018-06-01T04:18:04Z

@yorkie

There should be no change in common case performance.
The only observable difference should be for deliberately malicious input.

I don't think a benchmark targeting malicious input to this API would be of interest to those monitoring Node.js performance characteristics. Do you?

yorkie · 2018-06-01T04:27:53Z

Your patch looks good to me, just ensure the "performance" in your title, I think if anything performance gets improved, actually they could be proven from the benchmark.

These run slowly without the patch.

It'd be better if you provide a performance diffs by benchmark :)

davisjam · 2018-06-01T04:39:08Z

@yorkie I have adjusted the title.

Here's an example of the change in performance on malicious input.
I think adding an explicit benchmark would be "benchmark bloat". This is not a critical use case and I don't think it merits a benchmark.

(00:38:12) jamie@woody ~/Desktop/floss/node $ time node /tmp/setServers.js  > /dev/null 2>&1

real	0m6.431s
user	0m6.415s
sys	0m0.004s
(00:38:26) jamie@woody ~/Desktop/floss/node $ time ./node /tmp/setServers.js  > /dev/null 2>&1

real	0m0.045s
user	0m0.041s
sys	0m0.004s
(00:38:29) jamie@woody ~/Desktop/floss/node $ cat /tmp/setServers.js 
const dns = require('dns');

dns.setServers(['['.repeat(100000)]);

BridgeAR · 2018-06-02T12:27:12Z

CI https://ci.nodejs.org/job/node-test-pull-request/15229/

Trott

LGTM with or without my nit addressed.

addaleax · 2018-06-07T23:31:09Z

@Trott I’d suggest you land this and address your own nit, there hasn’t been any movement in 5 days and it doesn’t seem like something that should hold up this PR?

Trott · 2018-06-07T23:54:31Z

Addressed my own nit.

CI: https://ci.nodejs.org/job/node-test-pull-request/15329/

davisjam · 2018-06-08T00:40:19Z

@Trott Thanks!

Trott · 2018-06-08T23:44:02Z

CI: https://ci.nodejs.org/job/node-test-pull-request/15349/

jasnell

Should be fine. Nothing obviously incorrect here :-)

Trott · 2018-06-09T03:55:31Z

arm-fanned re-run: https://ci.nodejs.org/job/node-test-commit-arm-fanned/1823/

Trott · 2018-06-09T03:55:55Z

smartos rerun: https://ci.nodejs.org/job/node-test-commit-smartos/18245/

Issue 1: make invalid setServers yield uniform error Behavior: dns.setServers throws a null pointer dereference on some inputs. Expected behavior was the more pleasant TypeError [ERR_INVALID_IP_ADDRESS] ... Root cause(s?): - Dereferencing the result of a regex match without confirming that there was a match. - assuming the capture of an optional group (?) Solution: Confirm the match, and handle a missing port cleanly. Tests: I added tests for various unusual inputs. Issue 2: revise quadratic regex in setServers Problem: The IPv6 regex was quadratic. On long malicious input the event loop could block. The security team did not deem it a security risk, but said a PR was welcome. Solution: Revise the regex to a linear-complexity version. Tests: I added REDOS tests to the "oddities" section. Fixes: nodejs#20441 Fixes: nodejs#20443 PR-URL: nodejs#20445 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Trott · 2018-06-09T04:36:21Z

Landed in 872c331. Thanks!

Issue 1: make invalid setServers yield uniform error Behavior: dns.setServers throws a null pointer dereference on some inputs. Expected behavior was the more pleasant TypeError [ERR_INVALID_IP_ADDRESS] ... Root cause(s?): - Dereferencing the result of a regex match without confirming that there was a match. - assuming the capture of an optional group (?) Solution: Confirm the match, and handle a missing port cleanly. Tests: I added tests for various unusual inputs. Issue 2: revise quadratic regex in setServers Problem: The IPv6 regex was quadratic. On long malicious input the event loop could block. The security team did not deem it a security risk, but said a PR was welcome. Solution: Revise the regex to a linear-complexity version. Tests: I added REDOS tests to the "oddities" section. Fixes: #20441 Fixes: #20443 PR-URL: #20445 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

This Pull Request updates dependency [node](https://github.com/nodejs/node) from `v10.4.1` to `v10.5.0` <details> <summary>Release Notes</summary> ### [`v10.5.0`](https://github.com/nodejs/node/releases/v10.5.0) [Compare Source](nodejs/node@v10.4.1...v10.5.0) ##### Notable Changes * **crypto**: * Support for `crypto.scrypt()` has been added. [#20816](`nodejs/node#20816) * **fs**: * BigInt support has been added to `fs.stat` and `fs.watchFile`. [#20220](`nodejs/node#20220) * APIs that take `mode` as arguments no longer throw on values larger than `0o777`. [#20636](`nodejs/node#20636) [#20975](`nodejs/node#20975) (Fixes: [#20498](`nodejs/node#20498)) * Fix crashes in closed event watchers. [#20985](`nodejs/node#20985) (Fixes: [#20297](`nodejs/node#20297)) * **Worker Threads**: * Support for multi-threading has been added behind the `--experimental-worker` flag in the `worker_threads` module. This feature is *experimental* and may receive breaking changes at any time. [#20876](`nodejs/node#20876) ##### Commits * [[`a6986fe8b6`](nodejs/node@a6986fe8b6)] - **async_hooks**: remove deprecated example (Mathias Buus) [#20998](`nodejs/node#20998) * [[`4b9817bf1e`](nodejs/node@4b9817bf1e)] - **benchmark**: disable only the ESLint rule needing it (Rich Trott) [#21133](`nodejs/node#21133) * [[`ecba1c57b1`](nodejs/node@ecba1c57b1)] - **(SEMVER-MINOR)** **benchmark**: port cluster/echo to worker (Timothy Gu) [#20876](`nodejs/node#20876) * [[`02adb2d62c`](nodejs/node@02adb2d62c)] - **(SEMVER-MINOR)** **build**: expose openssl scrypt functions to addons (Ben Noordhuis) [#20816](`nodejs/node#20816) * [[`c3fbac432f`](nodejs/node@c3fbac432f)] - **build**: install markdown linter for travis (Richard Lau) [#21215](`nodejs/node#21215) * [[`896017b134`](nodejs/node@896017b134)] - **build**: build addon tests in parallel (Anna Henningsen) [#21155](`nodejs/node#21155) * [[`76927fc734`](nodejs/node@76927fc734)] - **build**: stop distclean from deleting v8 files (Ujjwal Sharma) [#21164](`nodejs/node#21164) * [[`b044256f2a`](nodejs/node@b044256f2a)] - **build**: use LC\_ALL of C for maximum compatibility (Rich Trott) [#21222](`nodejs/node#21222) * [[`78c7d666fb`](nodejs/node@78c7d666fb)] - **build**: don't change locale on smartos (Refael Ackermann) [#21220](`nodejs/node#21220) * [[`c688a00a6d`](nodejs/node@c688a00a6d)] - **build**: fix 'gas\_version' check on localized environments (Evandro Oliveira) [#20394](`nodejs/node#20394) * [[`79b3423fb5`](nodejs/node@79b3423fb5)] - **build**: initial .travis.yml implementation (Anna Henningsen) [#21059](`nodejs/node#21059) * [[`ea4be72f22`](nodejs/node@ea4be72f22)] - **child_process**: swallow errors in internal communication (Anatoli Papirovski) [#21108](`nodejs/node#21108) * [[`9981220e2a`](nodejs/node@9981220e2a)] - **crypto**: fix behavior of createCipher in wrap mode (Tobias Nießen) [#21287](`nodejs/node#21287) * [[`d0cb9cbb35`](nodejs/node@d0cb9cbb35)] - **(SEMVER-MINOR)** **crypto**: drop Math.pow(), use static exponentation (Ben Noordhuis) [#20816](`nodejs/node#20816) * [[`2d9c3cc89d`](nodejs/node@2d9c3cc89d)] - **(SEMVER-MINOR)** **crypto**: refactor randomBytes() (Ben Noordhuis) [#20816](`nodejs/node#20816) * [[`6262fa44d6`](nodejs/node@6262fa44d6)] - **(SEMVER-MINOR)** **crypto**: refactor pbkdf2() and pbkdf2Sync() methods (Ben Noordhuis) [#20816](`nodejs/node#20816) * [[`c9b4592dbf`](nodejs/node@c9b4592dbf)] - **(SEMVER-MINOR)** **crypto**: add scrypt() and scryptSync() methods (Ben Noordhuis) [#20816](`nodejs/node#20816) * [[`495756264a`](nodejs/node@495756264a)] - **(SEMVER-MINOR)** **crypto**: DRY type checking (Ben Noordhuis) [#20816](`nodejs/node#20816) * [[`e4a7e0d28b`](nodejs/node@e4a7e0d28b)] - **deps**: float ea7abee from openssl / CVE-2018-0732 (Rod Vagg) [#21282](`nodejs/node#21282) * [[`0b90b071c4`](nodejs/node@0b90b071c4)] - **deps**: Upgrade node-inspect to 1.11.5 (Jan Krems) [#21055](`nodejs/node#21055) * [[`ffc29c12da`](nodejs/node@ffc29c12da)] - **deps**: patch V8 to 6.7.288.46 (Myles Borins) [#21260](`nodejs/node#21260) * [[`14bb905d18`](nodejs/node@14bb905d18)] - **deps**: V8: cherry-pick a440efb27f from upstream (Yang Guo) [#21022](`nodejs/node#21022) * [[`65b9c427ac`](nodejs/node@65b9c427ac)] - **dns**: improve setServers() errors and performance (Jamie Davis) [#20445](`nodejs/node#20445) * [[`bc20ec0c0f`](nodejs/node@bc20ec0c0f)] - **doc**: eliminate \_you\_ from N-API doc (Rich Trott) [#21382](`nodejs/node#21382) * [[`318d6831bf`](nodejs/node@318d6831bf)] - **doc**: use imperative in COLLABORATOR\_GUIDE (Rich Trott) [#21340](`nodejs/node#21340) * [[`177a7c06a8`](nodejs/node@177a7c06a8)] - **doc**: remove obsolete wiki references from BUILDING (Rich Trott) [#21369](`nodejs/node#21369) * [[`15023df050`](nodejs/node@15023df050)] - **doc**: add davisjam to collaborators (Jamie Davis) [#21273](`nodejs/node#21273) * [[`17c21b67ac`](nodejs/node@17c21b67ac)] - **doc**: fix indentation in console.md (Vse Mozhet Byt) [#21367](`nodejs/node#21367) * [[`ef74368416`](nodejs/node@ef74368416)] - **doc**: fix heading of optional console method args (Michaël Zasso) [#21311](`nodejs/node#21311) * [[`4f17841c20`](nodejs/node@4f17841c20)] - **doc**: use Class Method label consistently (Rich Trott) [#21357](`nodejs/node#21357) * [[`4566ebacf4`](nodejs/node@4566ebacf4)] - **doc**: wrap style guide at 80 characters (Rich Trott) [#21361](`nodejs/node#21361) * [[`6c41f33571`](nodejs/node@6c41f33571)] - **doc**: wrap pull-requests.md at 80 characters (Rich Trott) [#21361](`nodejs/node#21361) * [[`b8213f17cc`](nodejs/node@b8213f17cc)] - **doc**: remove linking of url text to url (Rich Trott) [#21361](`nodejs/node#21361) * [[`3f78220c2b`](nodejs/node@3f78220c2b)] - **doc**: correct styling of \_GitHub\_ in onboarding doc (Rich Trott) [#21361](`nodejs/node#21361) * [[`9e994cb119`](nodejs/node@9e994cb119)] - **doc**: wrap releases.md at 80 chars (Rich Trott) [#21361](`nodejs/node#21361) * [[`e00e5e6d5d`](nodejs/node@e00e5e6d5d)] - **doc**: switch the order of Writable and Readable (Joseph Gordon) [#21333](`nodejs/node#21333) * [[`e1b571d6b7`](nodejs/node@e1b571d6b7)] - **doc**: make Deprecation cycle explanation more brief (Rich Trott) [#21303](`nodejs/node#21303) * [[`df0f7a3b4d`](nodejs/node@df0f7a3b4d)] - **doc**: clarify async execute callback usage (Michael Dawson) [#21217](`nodejs/node#21217) * [[`c5a65594ef`](nodejs/node@c5a65594ef)] - **doc**: move 5 collaborators to emeritus status (Rich Trott) [#21272](`nodejs/node#21272) * [[`c1d53f86f8`](nodejs/node@c1d53f86f8)] - **doc**: update NODE\_OPTIONS section in cli.md (Vse Mozhet Byt) [#21229](`nodejs/node#21229) * [[`13fd09bfa7`](nodejs/node@13fd09bfa7)] - **doc**: add build wg info to releases.md (Jon Moss) [#21275](`nodejs/node#21275) * [[`0da910f9a5`](nodejs/node@0da910f9a5)] - **doc**: move Italo A. Casas to Release Emeritus (Myles Borins) [#21315](`nodejs/node#21315) * [[`6f7de0b8d9`](nodejs/node@6f7de0b8d9)] - **doc**: trim deprecation level definition text (Rich Trott) [#21241](`nodejs/node#21241) * [[`dd2fc90dcf`](nodejs/node@dd2fc90dcf)] - **doc**: fix reference to workerData in worker\_threads (Jeremiah Senkpiel) [#21180](`nodejs/node#21180) * [[`5e46c16371`](nodejs/node@5e46c16371)] - **doc**: fix type in stream doc (Aliaksei Tuzik) [#21178](`nodejs/node#21178) * [[`85dc9ac418`](nodejs/node@85dc9ac418)] - **doc**: add Michaël Zasso to Release team (Michaël Zasso) [#21114](`nodejs/node#21114) * [[`5fa5ab6c48`](nodejs/node@5fa5ab6c48)] - **doc**: naming function as suggested in addon docs (Tommaso Allevi) [#21067](`nodejs/node#21067) * [[`fe5d35123b`](nodejs/node@fe5d35123b)] - **(SEMVER-MINOR)** **doc**: document BigInt support in fs.Stats (Joyee Cheung) [#20220](`nodejs/node#20220) * [[`2c4f80ffba`](nodejs/node@2c4f80ffba)] - **doc**: remove spaces around slashes (Rich Trott) [#21140](`nodejs/node#21140) * [[`72e7e1da2d`](nodejs/node@72e7e1da2d)] - **doc**: alphabetize tls options (Rich Trott) [#21139](`nodejs/node#21139) * [[`06ac81e786`](nodejs/node@06ac81e786)] - **doc**: streamline errors.md introductory material (Rich Trott) [#21138](`nodejs/node#21138) * [[`73b8975b41`](nodejs/node@73b8975b41)] - **doc**: simplify deprecation language (Rich Trott) [#21136](`nodejs/node#21136) * [[`6caa354377`](nodejs/node@6caa354377)] - **(SEMVER-MINOR)** **doc**: explain Worker semantics in async\_hooks.md (Anna Henningsen) [#20876](`nodejs/node#20876) * [[`9f9355d6d2`](nodejs/node@9f9355d6d2)] - **doc**: fix inconsistent documentation (host vs hostname) (Davis Okoth) [#20933](`nodejs/node#20933) * [[`a5c571424a`](nodejs/node@a5c571424a)] - **doc**: document file mode caveats on Windows (Joyee Cheung) [#20636](`nodejs/node#20636) * [[`a75e44d135`](nodejs/node@a75e44d135)] - **esm**: ensure require.main for CJS top-level loads (Guy Bedford) [#21150](`nodejs/node#21150) * [[`04e8f0749e`](nodejs/node@04e8f0749e)] - **(SEMVER-MINOR)** **fs**: support BigInt in fs.\*stat and fs.watchFile (Joyee Cheung) [#20220](`nodejs/node#20220) * [[`c09bfd81b7`](nodejs/node@c09bfd81b7)] - **fs**: do not crash when using a closed fs event watcher (Joyee Cheung) [#20985](`nodejs/node#20985) * [[`bacb2cb550`](nodejs/node@bacb2cb550)] - **fs**: refactor fs module (James M Snell) [#20764](`nodejs/node#20764) * [[`db0bb5214a`](nodejs/node@db0bb5214a)] - **fs**: improve fchmod{Sync} validation (cjihrig) [#20588](`nodejs/node#20588) * [[`2ffb9d6b5c`](nodejs/node@2ffb9d6b5c)] - **fs**: drop duplicate API in promises mode (Сковорода Никита Андреевич) [#20559](`nodejs/node#20559) * [[`fc0b3610e2`](nodejs/node@fc0b3610e2)] - **fs**: don't limit ftruncate() length to 32 bits (cjihrig) [#20851](`nodejs/node#20851) * [[`469baa062e`](nodejs/node@469baa062e)] - **fs**: add length validation to fs.truncate() (cjihrig) [#20851](`nodejs/node#20851) * [[`6aade4a765`](nodejs/node@6aade4a765)] - **http**: remove a pair of outdated comments (Mark S. Everitt) [#21214](`nodejs/node#21214) * [[`bcaf59c739`](nodejs/node@bcaf59c739)] - **http2**: fix memory leak for uncommon headers (Anna Henningsen) [#21336](`nodejs/node#21336) * [[`dee250fd77`](nodejs/node@dee250fd77)] - **http2**: safer Http2Session destructor (Anatoli Papirovski) [#21194](`nodejs/node#21194) * [[`296fd57324`](nodejs/node@296fd57324)] - **inspector**: stop dragging platform pointer (Eugene Ostroukhov) * [[`fb71337bdf`](nodejs/node@fb71337bdf)] - **(SEMVER-MINOR)** **lib**: rename checkIsArrayBufferView() (Ben Noordhuis) [#20816](`nodejs/node#20816) * [[`f3570f201b`](nodejs/node@f3570f201b)] - **(SEMVER-MINOR)** **lib**: replace checkUint() with validateInt32() (Ben Noordhuis) [#20816](`nodejs/node#20816) * [[`b4b7d368be`](nodejs/node@b4b7d368be)] - **lib**: unmask mode\_t values with 0o777 (Joyee Cheung) [#20975](`nodejs/node#20975) * [[`36e5100a39`](nodejs/node@36e5100a39)] - **lib**: support ranges in validateInt32() (cjihrig) [#20588](`nodejs/node#20588) * [[`2fe88d2218`](nodejs/node@2fe88d2218)] - **lib**: mask mode\_t type of arguments with 0o777 (Joyee Cheung) [#20636](`nodejs/node#20636) * [[`a0cfb0c9d4`](nodejs/node@a0cfb0c9d4)] - **lib**: add validateInteger() validator (cjihrig) [#20851](`nodejs/node#20851) * [[`740d9f1a0e`](nodejs/node@740d9f1a0e)] - **lib,src**: make `StatWatcher` a `HandleWrap` (Anna Henningsen) [#21244](`nodejs/node#21244) * [[`a657984109`](nodejs/node@a657984109)] - **lib,src**: remove openssl feature conditionals (Ben Noordhuis) [#21094](`nodejs/node#21094) * [[`653b20b26d`](nodejs/node@653b20b26d)] - **loader**: remove unused error code in module\_job (Gus Caplan) [#21354](`nodejs/node#21354) * [[`5d3dfedca2`](nodejs/node@5d3dfedca2)] - **meta**: remove CODEOWNERS (Rich Trott) [#21161](`nodejs/node#21161) * [[`169bff3e9e`](nodejs/node@169bff3e9e)] - **n-api**: name CallbackBundle function fields (Anna Henningsen) [#21240](`nodejs/node#21240) * [[`1dc9330b3a`](nodejs/node@1dc9330b3a)] - **n-api**: improve runtime perf of n-api func call (Kenny Yuan) [#21072](`nodejs/node#21072) * [[`9047c8182c`](nodejs/node@9047c8182c)] - **n-api**: remove unused napi\_env member (Gabriel Schulhof) [#21127](`nodejs/node#21127) * [[`18c057ab26`](nodejs/node@18c057ab26)] - **net**: emit 'close' when socket ends before connect (Brett Kiefer) [#21290](`nodejs/node#21290) * [[`a3fd1cd8ea`](nodejs/node@a3fd1cd8ea)] - **perf_hooks**: remove less useful bootstrap marks (James M Snell) [#21247](`nodejs/node#21247) * [[`8fddf591c5`](nodejs/node@8fddf591c5)] - **perf_hooks**: set bootstrap complete in only one place (James M Snell) [#21247](`nodejs/node#21247) * [[`fc2956d37a`](nodejs/node@fc2956d37a)] - **process**: backport process/methods file (Michaël Zasso) [#21172](`nodejs/node#21172) * [[`78ad4e9dde`](nodejs/node@78ad4e9dde)] - **src**: remove unused argc var in node\_stat\_watcher (Daniel Bevenius) [#21337](`nodejs/node#21337) * [[`7fa1344143`](nodejs/node@7fa1344143)] - **src**: use `%zx` in printf for size\_t (Anna Henningsen) [#21323](`nodejs/node#21323) * [[`671346ee8f`](nodejs/node@671346ee8f)] - **src**: do proper error checking in `AsyncWrap::MakeCallback` (Anna Henningsen) [#21189](`nodejs/node#21189) * [[`aa468abc4c`](nodejs/node@aa468abc4c)] - **src**: unify native symbol inspection code (Anna Henningsen) [#21238](`nodejs/node#21238) * [[`e92b89a75d`](nodejs/node@e92b89a75d)] - **src**: fix http2 typos (Anatoli Papirovski) [#21194](`nodejs/node#21194) * [[`4f01168414`](nodejs/node@4f01168414)] - **src**: do not persist fs\_poll handle in stat\_watcher (Anatoli Papirovski) [#21093](`nodejs/node#21093) * [[`685b9b2a6a`](nodejs/node@685b9b2a6a)] - **src**: do not persist timer handle in cares\_wrap (Anatoli Papirovski) [#21093](`nodejs/node#21093) * [[`4757771db3`](nodejs/node@4757771db3)] - **src**: add consistency check to node\_platform.cc (Anna Henningsen) [#21156](`nodejs/node#21156) * [[`8e2e16721b`](nodejs/node@8e2e16721b)] - **src**: add node\_encoding.cc (James M Snell) [#21112](`nodejs/node#21112) * [[`39b38754eb`](nodejs/node@39b38754eb)] - **src**: cleanup beforeExit for consistency (James M Snell) [#21113](`nodejs/node#21113) * [[`314b47d1cf`](nodejs/node@314b47d1cf)] - **(SEMVER-MINOR)** **src**: add Env::profiler\_idle\_notifier\_started() (Timothy Gu) [#20876](`nodejs/node#20876) * [[`5209ff9562`](nodejs/node@5209ff9562)] - **(SEMVER-MINOR)** **src**: remove unused fields msg\_ and env\_ (Daniel Bevenius) [#20876](`nodejs/node#20876) * [[`9a734132f9`](nodejs/node@9a734132f9)] - **(SEMVER-MINOR)** **src**: make handle onclose property a Symbol (Anna Henningsen) [#20876](`nodejs/node#20876) * [[`e6f06807b1`](nodejs/node@e6f06807b1)] - **(SEMVER-MINOR)** **src**: simplify handle closing (Anna Henningsen) [#20876](`nodejs/node#20876) * [[`65924c70e8`](nodejs/node@65924c70e8)] - **(SEMVER-MINOR)** **src**: remove unused fields isolate\_ (Daniel Bevenius) [#20876](`nodejs/node#20876) * [[`de7403f813`](nodejs/node@de7403f813)] - **(SEMVER-MINOR)** **src**: cleanup per-isolate state on platform on isolate unregister (Anna Henningsen) [#20876](`nodejs/node#20876) * [[`ba17c9e46b`](nodejs/node@ba17c9e46b)] - **src**: refactor bootstrap to use bootstrap object (James M Snell) [#20917](`nodejs/node#20917) * [[`cbdc1fdf44`](nodejs/node@cbdc1fdf44)] - **src, tools**: add check for left leaning pointers (Daniel Bevenius) [#21010](`nodejs/node#21010) * [[`935309325b`](nodejs/node@935309325b)] - **test**: fix deprecation warning due to util.print (Tobias Nießen) [#21265](`nodejs/node#21265) * [[`d7ba75f8aa`](nodejs/node@d7ba75f8aa)] - **test**: add test to check colorMode type of Console (Masashi Hirano) [#21248](`nodejs/node#21248) * [[`0b00172df8`](nodejs/node@0b00172df8)] - **test**: removing unnecessary parameter from assert call (djmgit) [#21307](`nodejs/node#21307) * [[`dea3ac7bff`](nodejs/node@dea3ac7bff)] - **test**: improve statwatcher async\_hooks test (Anna Henningsen) [#21244](`nodejs/node#21244) * [[`792335f712`](nodejs/node@792335f712)] - **test**: add workerdata-sharedarraybuffer test (Jeremiah Senkpiel) [#21180](`nodejs/node#21180) * [[`e8d15cb149`](nodejs/node@e8d15cb149)] - **test**: mark test-inspector-port-zero-cluster flaky (Rich Trott) [#21251](`nodejs/node#21251) * [[`688bdfef7f`](nodejs/node@688bdfef7f)] - **test**: add crypto check to test-http2-debug (Daniel Bevenius) [#21205](`nodejs/node#21205) * [[`2270ab2a12`](nodejs/node@2270ab2a12)] - **test**: remove string literals from assert.strictEqual() calls (James Kylstra) [#21211](`nodejs/node#21211) * [[`187951c0fc`](nodejs/node@187951c0fc)] - **test**: move inspector-stress-http to sequential (Rich Trott) [#21227](`nodejs/node#21227) * [[`bda34ea203`](nodejs/node@bda34ea203)] - **test**: check gc does not resurrect the loop (Anatoli Papirovski) [#21093](`nodejs/node#21093) * [[`4d782c4720`](nodejs/node@4d782c4720)] - **test**: improve assert error messages (Hristijan Gjorgjievski) [#21160](`nodejs/node#21160) * [[`2655c7b194`](nodejs/node@2655c7b194)] - **test**: mark fs-readfile-tostring-fail flaky for all (Rich Trott) [#21177](`nodejs/node#21177) * [[`17954c2b01`](nodejs/node@17954c2b01)] - **test**: improve internal/buffer.js test coverage (Masashi Hirano) [#21061](`nodejs/node#21061) * [[`2ff4704447`](nodejs/node@2ff4704447)] - **test**: move test-readuint to test-buffer-readuint (Michaël Zasso) [#21170](`nodejs/node#21170) * [[`9c3a7bf076`](nodejs/node@9c3a7bf076)] - **test**: make url-util-format engine agnostic (Rich Trott) [#21141](`nodejs/node#21141) * [[`3d8ec8f85c`](nodejs/node@3d8ec8f85c)] - **test**: make url-parse-invalid-input engine agnostic (Rich Trott) [#21132](`nodejs/node#21132) * [[`0b0370f884`](nodejs/node@0b0370f884)] - **test**: remove unref in http2 test (Anatoli Papirovski) [#21145](`nodejs/node#21145) * [[`14a017cf8d`](nodejs/node@14a017cf8d)] - **test**: apply promises API to fourth appendFile test (Rich Trott) [#21131](`nodejs/node#21131) * [[`aa9dbf666b`](nodejs/node@aa9dbf666b)] - **test**: apply promises API to fourth appendFile test (Rich Trott) [#21131](`nodejs/node#21131) * [[`185b9e45d3`](nodejs/node@185b9e45d3)] - **test**: apply promises API to third appendFile test (Rich Trott) [#21131](`nodejs/node#21131) * [[`c400448e85`](nodejs/node@c400448e85)] - **test**: improve debug output in trace-events test (Rich Trott) [#21120](`nodejs/node#21120) * [[`a4ad9891e3`](nodejs/node@a4ad9891e3)] - **test**: add test for Linux perf (Matheus Marchini) [#20783](`nodejs/node#20783) * [[`e16036c462`](nodejs/node@e16036c462)] - **test**: create new directory v8-updates (Matheus Marchini) [#20783](`nodejs/node#20783) * [[`93ce63c89f`](nodejs/node@93ce63c89f)] - **(SEMVER-MINOR)** **test**: add test against unsupported worker features (Timothy Gu) [#20876](`nodejs/node#20876) * [[`94dcdfb898`](nodejs/node@94dcdfb898)] - **test**: increase coverage for fs.promises.truncate (Masashi Hirano) [#20638](`nodejs/node#20638) * [[`c9cee63179`](nodejs/node@c9cee63179)] - **test,tools**: refactor custom ESLint for readability (Rich Trott) [#21134](`nodejs/node#21134) * [[`ed05d9a821`](nodejs/node@ed05d9a821)] - **(SEMVER-MINOR)** **test,tools**: enable running tests under workers (Anna Henningsen) [#20876](`nodejs/node#20876) * [[`6285fe94f6`](nodejs/node@6285fe94f6)] - **tools**: do not disable `quotes` rule in .eslintrc.js (Rich Trott) [#21338](`nodejs/node#21338) * [[`98346de08c`](nodejs/node@98346de08c)] - **tools**: lint doc/\*.md files (Rich Trott) [#21361](`nodejs/node#21361) * [[`521f8f1d95`](nodejs/node@521f8f1d95)] - **tools**: add BigInt64Array and BigUint64Array to globals (Joyee Cheung) [#21255](`nodejs/node#21255) * [[`a5c386d1ba`](nodejs/node@a5c386d1ba)] - **tools**: add option to use custom template with js2c.py (Shelley Vohr) [#21187](`nodejs/node#21187) * [[`7f70fe83ef`](nodejs/node@7f70fe83ef)] - **tools**: add BigInt to globals (Nikolai Vavilov) [#21237](`nodejs/node#21237) * [[`4e742e379b`](nodejs/node@4e742e379b)] - **tools**: update tooling to work with new macOS CLI … (Rich Trott) [#21173](`nodejs/node#21173) * [[`ed2b57bcd5`](nodejs/node@ed2b57bcd5)] - **tools**: remove unused global types from type-parser (Rich Trott) [#21135](`nodejs/node#21135) * [[`d46446afc5`](nodejs/node@d46446afc5)] - **v8**: replace Buffer with FastBuffer in deserialize (Ujjwal Sharma) [#21196](`nodejs/node#21196) * [[`917960e0a1`](nodejs/node@917960e0a1)] - **win, build**: add documentation support to vcbuild (Bartosz Sosnowski) [#19663](`nodejs/node#19663) * [[`03fbc9e749`](nodejs/node@03fbc9e749)] - **(SEMVER-MINOR)** **worker**: rename to worker\_threads (Anna Henningsen) [#20876](`nodejs/node#20876) * [[`9ad42b766e`](nodejs/node@9ad42b766e)] - **(SEMVER-MINOR)** **worker**: improve error (de)serialization (Anna Henningsen) [#20876](`nodejs/node#20876) * [[`6b1a887aa2`](nodejs/node@6b1a887aa2)] - **(SEMVER-MINOR)** **worker**: enable stdio (Anna Henningsen) [#20876](`nodejs/node#20876) * [[`c97fb91e55`](nodejs/node@c97fb91e55)] - **(SEMVER-MINOR)** **worker**: restrict supported extensions (Timothy Gu) [#20876](`nodejs/node#20876) * [[`109c92e8fa`](nodejs/node@109c92e8fa)] - **(SEMVER-MINOR)** **worker**: initial implementation (Anna Henningsen) [#20876](`nodejs/node#20876) * [[`d1f372f052`](nodejs/node@d1f372f052)] - **(SEMVER-MINOR)** **worker**: add `SharedArrayBuffer` sharing (Anna Henningsen) [#20876](`nodejs/node#20876) * [[`f447acd87b`](nodejs/node@f447acd87b)] - **(SEMVER-MINOR)** **worker**: support MessagePort passing in messages (Anna Henningsen) [#20876](`nodejs/node#20876) * [[`337be58ee6`](nodejs/node@337be58ee6)] - **(SEMVER-MINOR)** **worker**: implement `MessagePort` and `MessageChannel` (Anna Henningsen) [#20876](`nodejs/node#20876) * [[`4a54ebc3bd`](nodejs/node@4a54ebc3bd)] - **worker,src**: display remaining handles if `uv\_loop\_close` fails (Anna Henningsen) [#21238](`nodejs/node#21238) * [[`529d24e3e8`](nodejs/node@529d24e3e8)] - ***Revert*** "**workers,trace_events**: set thread name for workers" (James M Snell) [#21363](`nodejs/node#21363) * [[`dfb5cf6963`](nodejs/node@dfb5cf6963)] - **workers,trace_events**: set thread name for workers (James M Snell) [#21246](`nodejs/node#21246) --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com).

nodejs-github-bot added the dns Issues and PRs related to the dns subsystem. label May 1, 2018

davisjam commented May 1, 2018

View reviewed changes

davisjam changed the title ~~DNS issues~~ dns: consistency, performance May 1, 2018

davisjam commented May 1, 2018

View reviewed changes

davisjam force-pushed the DNSIssues branch from abb1dcc to 404f067 Compare May 1, 2018 01:55

davisjam commented May 1, 2018

View reviewed changes

BridgeAR reviewed May 1, 2018

View reviewed changes

bnoordhuis reviewed May 18, 2018

View reviewed changes

davisjam force-pushed the DNSIssues branch from 404f067 to fc93b59 Compare May 31, 2018 04:42

yorkie reviewed May 31, 2018

View reviewed changes

address review comments

03451b4

davisjam changed the title ~~dns: consistency, performance~~ dns: consistency; performance on malicious input Jun 1, 2018

BridgeAR approved these changes Jun 2, 2018

View reviewed changes

BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 2, 2018

Trott approved these changes Jun 3, 2018

View reviewed changes

Update test-dns.js

260ee07

jasnell approved these changes Jun 9, 2018

View reviewed changes

Trott closed this Jun 9, 2018

targos mentioned this pull request Jun 19, 2018

v10.5.0 proposal #21400

Merged

This was referenced Jun 20, 2018

2018-06-20 Version 10.5.0 (Current) @targos beevelop/docker-android-nodejs#216

Closed

2018-06-20 Version 10.5.0 (Current) @targos beevelop/docker-nodejs#201

Closed

dns: consistency; performance on malicious input #20445

dns: consistency; performance on malicious input #20445

Conversation

davisjam commented May 1, 2018

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BridgeAR left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BridgeAR commented May 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davisjam commented May 18, 2018 • edited Loading

BridgeAR commented May 29, 2018

davisjam commented May 31, 2018

davisjam commented May 31, 2018

yorkie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davisjam commented May 31, 2018

yorkie commented Jun 1, 2018

davisjam commented Jun 1, 2018

davisjam commented Jun 1, 2018

yorkie commented Jun 1, 2018

davisjam commented Jun 1, 2018

yorkie commented Jun 1, 2018

davisjam commented Jun 1, 2018 • edited Loading

BridgeAR commented Jun 2, 2018

Trott left a comment

Choose a reason for hiding this comment

addaleax commented Jun 7, 2018

Trott commented Jun 7, 2018

davisjam commented Jun 8, 2018

Trott commented Jun 8, 2018

jasnell left a comment

Choose a reason for hiding this comment

Trott commented Jun 9, 2018

Trott commented Jun 9, 2018

Trott commented Jun 9, 2018

davisjam commented May 18, 2018 •

edited

Loading

davisjam commented Jun 1, 2018 •

edited

Loading