dns: make dns.setServers support customized port
#13723
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -301,7 +301,13 @@ function resolve(hostname, rrtype, callback) { | |
|
|
||
|
|
||
| function getServers() { | ||
| const ret = cares.getServers(); | ||
| return ret.map((val) => { | ||
| if (!val[1] || val[1] === 53) return val[0]; | ||
|
There was a problem hiding this comment. I just noticed this, IMHO we should format IPv6 with return ret.map(([host, port]) => {
if (isIP(host) === 6) host = `[${host}]`;
return (!port || port === 53) host : `${host}:${port}`;
}But that will make this There was a problem hiding this comment. Fix this in a future PR. There was a problem hiding this comment. Can you put the There was a problem hiding this comment. Suggestion: |
||
|
|
||
| const host = isIP(val[0]) === 6 ? `[${val[0]}]` : val[0]; | ||
| return `${host}:${val[1]}`; | ||
| }); | ||
| } | ||
|
|
||
|
|
||
|
|
@@ -311,26 +317,29 @@ function setServers(servers) { | |
| const orig = cares.getServers(); | ||
| const newSet = []; | ||
| const IPv6RE = /\[(.*)\]/; | ||
| const addrSplitRE = /(^.+?)(?::(\d+))?$/; | ||
|
There was a problem hiding this comment. another possible way to handle this that may provide more robust and consistent handling of the address and port would be to leverage the new const URL = require('url').URL;
const u = new URL(`dns://${serv}`);If perf is a concern around creating multiple const url = new URL(`dns://${serv}`);
// use it
url.host = nextServ;
// etcThere was a problem hiding this comment. I'm +1 since this is a "cold-spot" There was a problem hiding this comment. But how about something like There was a problem hiding this comment. three options:
There was a problem hiding this comment. I think keeping RegEx would be fine because URL does some extra operation in semantics, though URL's performance may be better than RegEx. There was a problem hiding this comment. Not sure I follow? Just ignore any additional input. The host and the port are the only bits we care about. If you do not think it is appropriate to ignore the rest, then throw if values for the other parts are provided. There was a problem hiding this comment.
I think the issue is that this is our only validity check. Now |
||
|
|
||
| servers.forEach((serv) => { | ||
| var ipVersion = isIP(serv); | ||
| if (ipVersion !== 0) | ||
| return newSet.push([ipVersion, serv, 53]); | ||
|
|
||
| const match = serv.match(IPv6RE); | ||
| // we have an IPv6 in brackets | ||
| if (match) { | ||
| ipVersion = isIP(match[1]); | ||
| if (ipVersion !== 0) { | ||
| const port = parseInt(serv.replace(addrSplitRE, '$2')) || 53; | ||
| return newSet.push([ipVersion, match[1], port]); | ||
| } | ||
| } | ||
|
|
||
| const [, s, p] = serv.match(addrSplitRE); | ||
| ipVersion = isIP(s); | ||
|
|
||
| if (ipVersion !== 0) { | ||
| return newSet.push([ipVersion, s, parseInt(p)]); | ||
| } | ||
|
|
||
| throw new Error(`IP address is not properly formatted: ${serv}`); | ||
| }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to remove
[]for this line.Ref: #13795
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still needs to be changed, since currently the output will be without the
[]There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.