Skip to content

Commit e093be2

Browse files
author
Jake Champion
committed
improve conformance for parsing a host
1 parent b9c78c4 commit e093be2

File tree

3 files changed

+91
-38
lines changed

3 files changed

+91
-38
lines changed

c-dependencies/js-compute-runtime/builtins/backend.cpp

Lines changed: 46 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <optional>
55
#include <string>
66
#include <string_view>
7+
#include <vector>
78

89
// TODO: remove these once the warnings are fixed
910
#pragma clang diagnostic push
@@ -15,20 +16,28 @@
1516
#include "host_call.h"
1617
#include "js/Conversions.h"
1718

18-
bool isDash(char character) { return character == 45; }
19-
bool isDot(char character) { return character == 46; }
20-
21-
bool isNotAlphaNumericOrDash(char character) {
22-
return !std::isalnum(character) && !isDash(character);
23-
}
24-
25-
bool isNotAlphaNumericDotOrDash(char character) {
26-
return !std::isalnum(character) && !isDash(character) && !isDot(character);
19+
std::vector<std::string_view> split(std::string_view string, char delimiter) {
20+
auto start = 0;
21+
auto end = string.find(delimiter, start);
22+
std::vector<std::string_view> result;
23+
while (end != std::string::npos) {
24+
result.push_back(string.substr(start, end - start));
25+
start = end + 1;
26+
end = string.find(delimiter, start);
27+
}
28+
result.push_back(string.substr(start));
29+
return result;
2730
}
2831

32+
// A "host" is a "hostname" and an optional "port" in the format hostname:port
33+
// A "hostname" is between 1 and 255 octets -- https://www.rfc-editor.org/rfc/rfc1123#page-13
34+
// A "hostname" must start with a letter or digit -- https://www.rfc-editor.org/rfc/rfc1123#page-13
35+
// A "hostname" is made up of "labels" delimited by a dot `.`
36+
// A "label" is between 1 and 63 octets
2937
bool isValidHost(std::string_view host) {
30-
// ValidHostRegex =
31-
// "^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])(:[0-9]+)$";
38+
if (host.length() < 1) {
39+
return false;
40+
}
3241
auto firstCharacter = host.front();
3342
// check first character is in the regex [a-zA-Z0-9]
3443
if (!std::isalnum(firstCharacter)) {
@@ -38,37 +47,42 @@ bool isValidHost(std::string_view host) {
3847
int pos = host.find_first_of(':');
3948
std::string_view hostname = host.substr(0, pos);
4049

50+
// hostnames can not be longer than 253 characters
51+
// This is because a hostname is represented as a series of labels, and is terminated by a label of length zero.
52+
// A label consists of a length octet followed by that number of octets representing the name itself.
53+
// https://www.rfc-editor.org/rfc/rfc1035#section-3.3
54+
// https://www.rfc-editor.org/rfc/rfc2181#section-11
55+
if (hostname.length() > 253) {
56+
return false;
57+
}
58+
4159
auto lastCharacter = hostname.back();
4260
// check last character is in the regex [a-zA-Z0-9]
4361
if (!std::isalnum(lastCharacter)) {
4462
return false;
4563
}
4664

47-
// find the position of the last .
48-
auto lastDot = hostname.rfind('.');
49-
if (lastDot == std::string::npos) {
50-
return true;
51-
}
65+
auto labels = split(hostname, '.');
5266

53-
// check the character before the last . is in the regex [a-zA-Z0-9]
54-
if (!std::isalnum(hostname.at(lastDot - 1))) {
55-
return false;
56-
}
57-
// check all other characters before the last . are in the regex [a-zA-Z0-9\-.]
58-
auto last = std::next(hostname.begin(), lastDot);
59-
auto it = std::find_if(std::next(hostname.begin()), last, isNotAlphaNumericDotOrDash);
67+
for (auto label:labels) {
68+
// Each label in a hostname can not be longer than 63 characters
69+
// https://www.rfc-editor.org/rfc/rfc2181#section-11
70+
if (label.length() > 63) {
71+
return false;
72+
}
6073

61-
if (it != last) {
62-
return false;
63-
}
64-
// check all characters after the last . (but not the last character) are in the regex
65-
// [a-zA-Z0-9\-]
66-
last = std::prev(hostname.end());
67-
it = std::find_if(std::next(hostname.begin(), lastDot + 1), last, isNotAlphaNumericOrDash);
68-
if (it != last) {
69-
return false;
74+
// Each label can only contain the characters in the regex [a-zA-Z0-9\-]
75+
auto it = std::find_if_not(label.begin(), label.end(), [&](auto character) {
76+
return std::isalnum(character) || character == '-';
77+
});
78+
if (it != label.end()) {
79+
80+
return false;
81+
}
7082
}
83+
7184
// if there is a port - confirm it is all digits and is between 0 and 65536
85+
// https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml
7286
if (pos != std::string::npos) {
7387
std::string_view port = host.substr(pos + 1);
7488
if (!std::all_of(port.begin(), port.end(), ::isdigit)) {

c-dependencies/js-compute-runtime/error-numbers.msg

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ MSG_DEF(JSMSG_BACKEND_NAME_TOO_LONG, 0, JSEXN_TYPEERR,
6464
MSG_DEF(JSMSG_BACKEND_NAME_EMPTY, 0, JSEXN_TYPEERR, "Backend constructor: name can not be an empty string")
6565
MSG_DEF(JSMSG_BACKEND_TARGET_NOT_SET, 0, JSEXN_TYPEERR, "Backend constructor: target can not be null or undefined")
6666
MSG_DEF(JSMSG_BACKEND_TARGET_EMPTY, 0, JSEXN_TYPEERR, "Backend constructor: target can not be an empty string")
67-
MSG_DEF(JSMSG_BACKEND_TARGET_INVALID, 0, JSEXN_TYPEERR, "Backend constructor: target is not a valid host or IP address")
67+
MSG_DEF(JSMSG_BACKEND_TARGET_INVALID, 0, JSEXN_TYPEERR, "Backend constructor: target does not contain a valid IPv4, IPv6, or hostname address")
6868
MSG_DEF(JSMSG_BACKEND_CIPHERS_EMPTY, 0, JSEXN_TYPEERR, "Backend constructor: ciphers can not be an empty string")
6969
MSG_DEF(JSMSG_BACKEND_HOST_OVERRIDE_EMPTY, 0, JSEXN_TYPEERR, "Backend constructor: hostOverride can not be an empty string")
7070
MSG_DEF(JSMSG_BACKEND_CERTIFICATE_HOSTNAME_EMPTY, 0, JSEXN_TYPEERR, "Backend constructor: certificateHostname can not be an empty string")

integration-tests/js-compute/fixtures/dynamic-backend/bin/index.js

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -305,11 +305,50 @@ routes.set('/', () => {
305305
return pass()
306306
});
307307

308-
routes.set("/backend/constructor/parameter-target-property-valid-string", async () => {
309-
let error = assertDoesNotThrow(() => {
310-
new Backend({ name: 'target-property-valid-string', target: "www.fastly.com" })
311-
})
312-
if (error) { return error }
308+
routes.set("/backend/constructor/parameter-target-property-valid-host", async () => {
309+
const targets = [
310+
"www.fastly.com",
311+
"w-w-w.f-a-s-t-l-y.c-o-m",
312+
"a".repeat(63) + ".com",
313+
`${"a".repeat(63)}.${"a".repeat(63)}.${"a".repeat(63)}.${"a".repeat(57)}.com`,
314+
"ai",
315+
"w.a:1",
316+
"fastly.com:1",
317+
"fastly.com:80",
318+
"fastly.com:443",
319+
"fastly.com:65535",
320+
];
321+
let i = 0;
322+
for (const target of targets) {
323+
let error = assertDoesNotThrow(() => {
324+
new Backend({ name: 'target-property-valid-host-'+i, target })
325+
})
326+
if (error) { return error }
327+
i++;
328+
}
329+
return pass()
330+
});
331+
332+
routes.set("/backend/constructor/parameter-target-property-invalid-host", async () => {
333+
const targets = [
334+
"-www.fastly.com",
335+
"www.fastly.com-",
336+
"a".repeat(64) + ".com",
337+
`${"a".repeat(63)}.${"a".repeat(63)}.${"a".repeat(63)}.${"a".repeat(58)}.com`,
338+
"w$.com",
339+
"w.a:a",
340+
"fastly.com:-1",
341+
"fastly.com:0",
342+
"fastly.com:65536",
343+
];
344+
let i = 0;
345+
for (const target of targets) {
346+
let error = assertThrows(() => {
347+
new Backend({ name: 'target-property-invalid-host-'+i, target })
348+
}, TypeError, `Backend constructor: target does not contain a valid IPv4, IPv6, or hostname address`)
349+
if (error) { return error }
350+
i++;
351+
}
313352
return pass()
314353
});
315354
}

0 commit comments

Comments
 (0)