Skip to content

Commit

Permalink
lib,src: port isIPv4() to js
Browse files Browse the repository at this point in the history
Removes a few lines of C++ code while making `isIPv4()` about 3x faster.
`isIPv6()` and `isIP()` for the IPv6 case stay about the same.

I removed the homegrown `isIPv4()` in lib/dns.js that utilized a lookup
table.  It is in fact a little faster than the new `isIPv4()` function
but:

1. The difference is only measurable at around 10M iterations, and
2. The function is a "probably IPv4" heuristic, not a proper validator.

PR-URL: #18398
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
  • Loading branch information
bnoordhuis committed Jan 29, 2018
1 parent 1598ec7 commit 742ae61
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 44 deletions.
27 changes: 1 addition & 26 deletions lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
const util = require('util');

const cares = process.binding('cares_wrap');
const { isLegalPort } = require('internal/net');
const { isIP, isIPv4, isLegalPort } = require('internal/net');
const { customPromisifyArgs } = require('internal/util');
const errors = require('internal/errors');
const {
Expand All @@ -38,7 +38,6 @@ const {
GetNameInfoReqWrap,
QueryReqWrap,
ChannelWrap,
isIP
} = cares;

function errnoException(err, syscall, hostname) {
Expand Down Expand Up @@ -66,30 +65,6 @@ function errnoException(err, syscall, hostname) {
}

const IANA_DNS_PORT = 53;
const digits = [
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0-15
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 16-31
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 32-47
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, // 48-63
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 64-79
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 80-95
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 96-111
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 // 112-127
];
function isIPv4(str) {
if (!digits[str.charCodeAt(0)]) return false;
if (str.length === 1) return false;
if (str.charCodeAt(1) === 46/*'.'*/)
return true;
else if (!digits[str.charCodeAt(1)])
return false;
if (str.length === 2) return false;
if (str.charCodeAt(2) === 46/*'.'*/)
return true;
else if (!digits[str.charCodeAt(2)])
return false;
return (str.length > 3 && str.charCodeAt(3) === 46/*'.'*/);
}


function onlookup(err, addresses) {
Expand Down
17 changes: 17 additions & 0 deletions lib/internal/net.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,22 @@
'use strict';

const Buffer = require('buffer').Buffer;
const { isIPv6 } = process.binding('cares_wrap');
const { writeBuffer } = process.binding('fs');

const octet = '(?:[0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])';
const re = new RegExp(`^${octet}[.]${octet}[.]${octet}[.]${octet}$`);

function isIPv4(s) {
return re.test(s);
}

function isIP(s) {
if (isIPv4(s)) return 4;
if (isIPv6(s)) return 6;
return 0;
}

// Check that the port number is not NaN when coerced to a number,
// is an integer and that it falls within the legal range of port numbers.
function isLegalPort(port) {
Expand Down Expand Up @@ -33,6 +47,9 @@ function makeSyncWrite(fd) {
}

module.exports = {
isIP,
isIPv4,
isIPv6,
isLegalPort,
makeSyncWrite,
normalizedArgsSymbol: Symbol('normalizedArgs')
Expand Down
14 changes: 8 additions & 6 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@ const stream = require('stream');
const util = require('util');
const internalUtil = require('internal/util');
const {
isIP,
isIPv4,
isIPv6,
isLegalPort,
normalizedArgsSymbol,
makeSyncWrite
} = require('internal/net');
const assert = require('assert');
const cares = process.binding('cares_wrap');
const {
UV_EADDRINUSE,
UV_EINVAL,
Expand Down Expand Up @@ -1066,7 +1068,7 @@ function lookupAndConnect(self, options) {
var localAddress = options.localAddress;
var localPort = options.localPort;

if (localAddress && !cares.isIP(localAddress)) {
if (localAddress && !isIP(localAddress)) {
throw new errors.TypeError('ERR_INVALID_IP_ADDRESS', localAddress);
}

Expand All @@ -1091,7 +1093,7 @@ function lookupAndConnect(self, options) {
port |= 0;

// If host is an IP, skip performing a lookup
var addressType = cares.isIP(host);
var addressType = isIP(host);
if (addressType) {
nextTick(self[async_id_symbol], function() {
if (self.connecting)
Expand Down Expand Up @@ -1775,9 +1777,9 @@ module.exports = {
connect,
createConnection: connect,
createServer,
isIP: cares.isIP,
isIPv4: cares.isIPv4,
isIPv6: cares.isIPv6,
isIP: isIP,
isIPv4: isIPv4,
isIPv6: isIPv6,
Server,
Socket,
Stream: Socket, // Legacy naming
Expand Down
12 changes: 0 additions & 12 deletions src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1886,16 +1886,6 @@ int ParseIP(const char* ip, ParseIPResult* result = nullptr) {
return 0;
}

void IsIP(const FunctionCallbackInfo<Value>& args) {
node::Utf8Value ip(args.GetIsolate(), args[0]);
args.GetReturnValue().Set(ParseIP(*ip));
}

void IsIPv4(const FunctionCallbackInfo<Value>& args) {
node::Utf8Value ip(args.GetIsolate(), args[0]);
args.GetReturnValue().Set(4 == ParseIP(*ip));
}

void IsIPv6(const FunctionCallbackInfo<Value>& args) {
node::Utf8Value ip(args.GetIsolate(), args[0]);
args.GetReturnValue().Set(6 == ParseIP(*ip));
Expand Down Expand Up @@ -2138,8 +2128,6 @@ void Initialize(Local<Object> target,

env->SetMethod(target, "getaddrinfo", GetAddrInfo);
env->SetMethod(target, "getnameinfo", GetNameInfo);
env->SetMethod(target, "isIP", IsIP);
env->SetMethod(target, "isIPv4", IsIPv4);
env->SetMethod(target, "isIPv6", IsIPv6);
env->SetMethod(target, "canonicalizeIP", CanonicalizeIP);

Expand Down

0 comments on commit 742ae61

Please sign in to comment.