Skip to content

Commit c6b2f42

Browse files
authored
Merge pull request #1045 from murgatroid99/grpc-js_dns_fix
Fix DNS name regex and add tests
2 parents 3c74283 + fe60128 commit c6b2f42

File tree

3 files changed

+130
-6
lines changed

3 files changed

+130
-6
lines changed

packages/grpc-js/package.json

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@grpc/grpc-js",
3-
"version": "0.6.0",
3+
"version": "0.6.1",
44
"description": "gRPC Library for Node - pure JS implementation",
55
"homepage": "https://grpc.io/",
66
"repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js",
@@ -22,7 +22,6 @@
2222
"@types/mocha": "^5.2.6",
2323
"@types/node": "^12.7.5",
2424
"@types/ncp": "^2.0.1",
25-
"@types/node": "^12.7.5",
2625
"@types/pify": "^3.0.2",
2726
"@types/semver": "^6.0.1",
2827
"clang-format": "^1.0.55",

packages/grpc-js/src/resolver-dns.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,11 @@ const IPV6_BRACKET_REGEX = /^\[([0-9a-f]{0,4}(?::{1,2}[0-9a-f]{0,4})+)\](?::(\d+
5050

5151
/**
5252
* Matches `[dns:][//authority/]host[:port]`, where `authority` and `host` are
53-
* both arbitrary sequences of alphanumeric characters and `port` is a sequence
54-
* of digits. Group 1 contains the hostname and group 2 contains the port
55-
* number if provided.
53+
* both arbitrary sequences of dot-separated strings of alphanumeric characters
54+
* and `port` is a sequence of digits. Group 1 contains the hostname and group
55+
* 2 contains the port number if provided.
5656
*/
57-
const DNS_REGEX = /^(?:dns:)?(?:\/\/\w+\/)?(\w+)(?::(\d+))?$/;
57+
const DNS_REGEX = /^(?:dns:)?(?:\/\/(?:[a-zA-Z0-9-]+\.?)+\/)?((?:[a-zA-Z0-9-]+\.?)+)(?::(\d+))?$/;
5858

5959
/**
6060
* The default TCP port to connect to if not explicitly specified in the target.
+125
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
/*
2+
* Copyright 2019 gRPC authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
*/
17+
18+
// Allow `any` data type for testing runtime type checking.
19+
// tslint:disable no-any
20+
import * as assert from 'assert';
21+
import * as resolverManager from '../src/resolver';
22+
import { ServiceConfig } from '../src/service-config';
23+
import { StatusObject } from '../src/call-stream';
24+
25+
describe('Name Resolver', () => {
26+
describe('DNS Names', () => {
27+
before(() => {
28+
resolverManager.registerAll();
29+
});
30+
it('Should resolve localhost properly', done => {
31+
const target = 'localhost:50051';
32+
const listener: resolverManager.ResolverListener = {
33+
onSuccessfulResolution: (
34+
addressList: string[],
35+
serviceConfig: ServiceConfig | null,
36+
serviceConfigError: StatusObject | null
37+
) => {
38+
assert(addressList.includes('127.0.0.1:50051'));
39+
// We would check for the IPv6 address but it needs to be omitted on some Node versions
40+
done();
41+
},
42+
onError: (error: StatusObject) => {
43+
done(new Error(`Failed with status ${error.details}`));
44+
},
45+
};
46+
const resolver = resolverManager.createResolver(target, listener);
47+
resolver.updateResolution();
48+
});
49+
it('Should default to port 443', done => {
50+
const target = 'localhost';
51+
const listener: resolverManager.ResolverListener = {
52+
onSuccessfulResolution: (
53+
addressList: string[],
54+
serviceConfig: ServiceConfig | null,
55+
serviceConfigError: StatusObject | null
56+
) => {
57+
assert(addressList.includes('127.0.0.1:443'));
58+
// We would check for the IPv6 address but it needs to be omitted on some Node versions
59+
done();
60+
},
61+
onError: (error: StatusObject) => {
62+
done(new Error(`Failed with status ${error.details}`));
63+
},
64+
};
65+
const resolver = resolverManager.createResolver(target, listener);
66+
resolver.updateResolution();
67+
});
68+
it('Should resolve a public address', done => {
69+
const target = 'example.com';
70+
const listener: resolverManager.ResolverListener = {
71+
onSuccessfulResolution: (
72+
addressList: string[],
73+
serviceConfig: ServiceConfig | null,
74+
serviceConfigError: StatusObject | null
75+
) => {
76+
assert(addressList.length > 0);
77+
done();
78+
},
79+
onError: (error: StatusObject) => {
80+
done(new Error(`Failed with status ${error.details}`));
81+
},
82+
};
83+
const resolver = resolverManager.createResolver(target, listener);
84+
resolver.updateResolution();
85+
});
86+
it('Should resolve a name with multiple dots', done => {
87+
const target = 'loopback4.unittest.grpc.io';
88+
const listener: resolverManager.ResolverListener = {
89+
onSuccessfulResolution: (
90+
addressList: string[],
91+
serviceConfig: ServiceConfig | null,
92+
serviceConfigError: StatusObject | null
93+
) => {
94+
assert(addressList.length > 0);
95+
done();
96+
},
97+
onError: (error: StatusObject) => {
98+
done(new Error(`Failed with status ${error.details}`));
99+
},
100+
};
101+
const resolver = resolverManager.createResolver(target, listener);
102+
resolver.updateResolution();
103+
});
104+
it('Should resolve a name with a hyphen', done => {
105+
/* TODO(murgatroid99): Find or create a better domain name to test this with.
106+
* This is just the first one I found with a hyphen. */
107+
const target = 'network-tools.com';
108+
const listener: resolverManager.ResolverListener = {
109+
onSuccessfulResolution: (
110+
addressList: string[],
111+
serviceConfig: ServiceConfig | null,
112+
serviceConfigError: StatusObject | null
113+
) => {
114+
assert(addressList.length > 0);
115+
done();
116+
},
117+
onError: (error: StatusObject) => {
118+
done(new Error(`Failed with status ${error.details}`));
119+
},
120+
};
121+
const resolver = resolverManager.createResolver(target, listener);
122+
resolver.updateResolution();
123+
});
124+
});
125+
});

0 commit comments

Comments
 (0)