Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

dns, src: add --preserve-dns-order flag with NODE_OPTIONS whitelist #17793

Closed
wants to merge 1 commit into from
Closed

dns, src: add --preserve-dns-order flag with NODE_OPTIONS whitelist #17793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib/dns.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
const util = require('util');

const cares = process.binding('cares_wrap');
const { preserveDnsOrder } = process.binding('config');
const { isLegalPort } = require('internal/net');
const { customPromisifyArgs } = require('internal/util');
const errors = require('internal/errors');
Expand Down Expand Up @@ -128,7 +129,7 @@ function lookup(hostname, options, callback) {
var hints = 0;
var family = -1;
var all = false;
var verbatim = false;
var verbatim = !!preserveDnsOrder;

// Parse arguments
if (hostname && typeof hostname !== 'string') {
Expand All @@ -143,7 +144,10 @@ function lookup(hostname, options, callback) {
hints = options.hints >>> 0;
family = options.family >>> 0;
all = options.all === true;
verbatim = options.verbatim === true;

if (Object.prototype.hasOwnProperty.call(options, 'verbatim')) {
verbatim = options.verbatim === true;
}

if (hints !== 0 &&
hints !== cares.AI_ADDRCONFIG &&
Expand Down
9 changes: 9 additions & 0 deletions src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,11 @@ std::string config_warning_file; // NOLINT(runtime/string)
// that is used by lib/internal/bootstrap_node.js
bool config_expose_internals = false;

// Set in node.cc by ParseArgs when --preserve-dns-order is used.
// Used in node_config.cc to set a constant on process.binding('config')
// that is used by lib/dns.js
bool config_preserve_dns_order = false;

bool v8_initialized = false;

bool linux_at_secure = false;
Expand Down Expand Up @@ -3626,6 +3631,7 @@ static void CheckIfAllowedInEnv(const char* exe, bool is_env,
"--pending-deprecation",
"--no-warnings",
"--napi-modules",
"--preserve-dns-order",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need discussion for whether this should be added to --help output

Note the comments for the whitelist:

// Node options, sorted in `node --help` order for ease of comparison.

so if it's decided not to add this option to --help then this option should be moved into another commented block (e.g. // Node options, not listed in 'node --help') before the V8 options.

"--expose-http2", // keep as a non-op through v9.x
"--experimental-modules",
"--loader",
Expand Down Expand Up @@ -3848,6 +3854,9 @@ static void ParseArgs(int* argc,
} else if (strcmp(arg, "--expose-http2") == 0 ||
strcmp(arg, "--expose_http2") == 0) {
// Keep as a non-op through v9.x
} else if (strcmp(arg, "--preserve-dns-order") == 0 ||
strcmp(arg, "--preserve_dns_order") == 0) {
config_preserve_dns_order = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most Node.js command line options only support the - variants (i.e. --preserve-dns-order). As this stands the other variant, --preserve_dns_order, would be allowed on the command line but not in NODE_OPTIONS.

} else if (strcmp(arg, "-") == 0) {
break;
} else if (strcmp(arg, "--") == 0) {
Expand Down
3 changes: 3 additions & 0 deletions src/node_config.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ static void InitConfig(Local<Object> target,
if (config_expose_internals)
READONLY_BOOLEAN_PROPERTY("exposeInternals");

if (config_preserve_dns_order)
READONLY_BOOLEAN_PROPERTY("preserveDnsOrder");

READONLY_PROPERTY(target,
"bits",
Number::New(env->isolate(), 8 * sizeof(intptr_t)));
Expand Down
5 changes: 5 additions & 0 deletions src/node_internals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,11 @@ extern bool config_preserve_symlinks;
// that is used by lib/module.js
extern bool config_experimental_modules;

// Set in node.cc by ParseArgs when --preserve-dns-order is used.
// Used in node_config.cc to set a constant on process.binding('config')
// that is used by lib/dns.js
extern bool config_preserve_dns_order;

// Set in node.cc by ParseArgs when --loader is used.
// Used in node_config.cc to set a constant on process.binding('config')
// that is used by lib/internal/bootstrap_node.js
Expand Down