Skip to content

Commit 7068321

Browse files
author
Guy Bedford
authored
fix: never error for missing client data, return null instead (#979)
1 parent d4c9b69 commit 7068321

File tree

8 files changed

+125
-90
lines changed

8 files changed

+125
-90
lines changed

‎documentation/docs/globals/FetchEvent/FetchEvent.mdx

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,22 @@ It provides the [`event.respondWith()`](./prototype/respondWith.mdx) method, whi
1515
- : The `Request` that was received by the application.
1616
- `FetchEvent.client` _**readonly**_
1717
- : Information about the downstream client that made the request.
18+
While these fields are always defined on Compute, they may be *null* when not available in testing environments
19+
such as Viceroy.
1820
- `FetchEvent.client.address` _**readonly**_
1921
- : A string representation of the IPv4 or IPv6 address of the downstream client.
2022
- `FetchEvent.client.geo` _**readonly**_
21-
- : A [geolocation dictionary](../../fastly:geolocation/getGeolocationForIpAddress.mdx) corresponding to the IP address of the downstream client.
23+
- : Either `null`, or a [geolocation dictionary](../../fastly:geolocation/getGeolocationForIpAddress.mdx) corresponding to the IP address of the downstream client.
2224
- `FetchEvent.client.tlsJA3MD5` _**readonly**_
23-
- : A string representation of the JA3 hash of the TLS ClientHello message.
25+
- : Either `null` or a string representation of the JA3 hash of the TLS ClientHello message.
2426
- `FetchEvent.client.tlsCipherOpensslName` _**readonly**_
25-
- : A string representation of the cipher suite used to secure the client TLS connection.
27+
- : Either `null` or a string representation of the cipher suite used to secure the client TLS connection.
2628
- `FetchEvent.client.tlsProtocol` _**readonly**_
27-
- : A string representation of the TLS protocol version used to secure the client TLS connection.
29+
- : Either `null` or a string representation of the TLS protocol version used to secure the client TLS connection.
2830
- `FetchEvent.client.tlsClientCertificate` _**readonly**_
29-
- : An ArrayBuffer containing the raw client certificate in the mutual TLS handshake message. It is in PEM format. Returns an empty ArrayBuffer if this is not mTLS or available.
31+
- : Either `null` or an ArrayBuffer containing the raw client certificate in the mutual TLS handshake message. It is in PEM format. Returns an empty ArrayBuffer if this is not mTLS or available.
3032
- `FetchEvent.client.tlsClientHello` _**readonly**_
31-
- : An ArrayBuffer containing the raw bytes sent by the client in the TLS ClientHello message.
33+
- : Either `null` or an ArrayBuffer containing the raw bytes sent by the client in the TLS ClientHello message.
3234
- `FetchEvent.server` _**readonly**_
3335
- : Information about the server receiving the request for the Fastly Compute service.
3436
- `FetchEvent.server.address` _**readonly**_

‎integration-tests/js-compute/fixtures/app/src/client.js

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,32 @@
1-
import { assert } from './assertions.js';
1+
import { strictEqual } from './assertions.js';
22
import { routes, isRunningLocally } from './routes.js';
33

44
routes.set('/client/tlsJA3MD5', (event) => {
5-
if (!isRunningLocally()) {
6-
assert(
5+
if (isRunningLocally()) {
6+
strictEqual(event.client.tlsJA3MD5, null);
7+
} else {
8+
strictEqual(
79
typeof event.client.tlsJA3MD5,
810
'string',
911
'typeof event.client.tlsJA3MD5',
1012
);
11-
assert(event.client.tlsJA3MD5.length, 32, 'event.client.tlsJA3MD5.length');
13+
strictEqual(
14+
event.client.tlsJA3MD5.length,
15+
32,
16+
'event.client.tlsJA3MD5.length',
17+
);
1218
}
1319
});
1420
routes.set('/client/tlsClientHello', (event) => {
15-
if (!isRunningLocally()) {
16-
assert(
21+
if (isRunningLocally()) {
22+
strictEqual(event.client.tlsClientHello, null);
23+
} else {
24+
strictEqual(
1725
event.client.tlsClientHello instanceof ArrayBuffer,
1826
true,
1927
'event.client.tlsClientHello instanceof ArrayBuffer',
2028
);
21-
assert(
29+
strictEqual(
2230
typeof event.client.tlsClientHello.byteLength,
2331
'number',
2432
'typeof event.client.tlsClientHello.byteLength',
@@ -27,13 +35,15 @@ routes.set('/client/tlsClientHello', (event) => {
2735
});
2836

2937
routes.set('/client/tlsClientCertificate', (event) => {
30-
if (!isRunningLocally()) {
31-
assert(
38+
if (isRunningLocally()) {
39+
strictEqual(event.client.tlsClientCertificate, null);
40+
} else {
41+
strictEqual(
3242
event.client.tlsClientCertificate instanceof ArrayBuffer,
3343
true,
3444
'event.client.tlsClientCertificate instanceof ArrayBuffer',
3545
);
36-
assert(
46+
strictEqual(
3747
event.client.tlsClientCertificate.byteLength,
3848
0,
3949
'event.client.tlsClientCertificate.byteLength',
@@ -42,8 +52,10 @@ routes.set('/client/tlsClientCertificate', (event) => {
4252
});
4353

4454
routes.set('/client/tlsCipherOpensslName', (event) => {
45-
if (!isRunningLocally()) {
46-
assert(
55+
if (isRunningLocally()) {
56+
strictEqual(event.client.tlsCipherOpensslName, null);
57+
} else {
58+
strictEqual(
4759
typeof event.client.tlsCipherOpensslName,
4860
'string',
4961
'typeof event.client.tlsCipherOpensslName',
@@ -52,8 +64,10 @@ routes.set('/client/tlsCipherOpensslName', (event) => {
5264
});
5365

5466
routes.set('/client/tlsProtocol', (event) => {
55-
if (!isRunningLocally()) {
56-
assert(
67+
if (isRunningLocally()) {
68+
strictEqual(event.client.tlsProtocol, null);
69+
} else {
70+
strictEqual(
5771
typeof event.client.tlsProtocol,
5872
'string',
5973
'typeof event.client.tlsProtocol',

‎integration-tests/js-compute/fixtures/app/tests.json

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -308,41 +308,11 @@
308308
"GET /simple-cache/getOrSet/does-not-freeze-when-called-after-a-get": {
309309
"environments": ["compute"]
310310
},
311-
"GET /client/tlsJA3MD5": {
312-
"environments": ["compute"],
313-
"downstream_response": {
314-
"status": 200,
315-
"body": "ok"
316-
}
317-
},
318-
"GET /client/tlsClientHello": {
319-
"environments": ["compute"],
320-
"downstream_response": {
321-
"status": 200,
322-
"body": "ok"
323-
}
324-
},
325-
"GET /client/tlsClientCertificate": {
326-
"environments": ["compute"],
327-
"downstream_response": {
328-
"status": 200,
329-
"body": "ok"
330-
}
331-
},
332-
"GET /client/tlsCipherOpensslName": {
333-
"environments": ["compute"],
334-
"downstream_response": {
335-
"status": 200,
336-
"body": "ok"
337-
}
338-
},
339-
"GET /client/tlsProtocol": {
340-
"environments": ["compute"],
341-
"downstream_response": {
342-
"status": 200,
343-
"body": "ok"
344-
}
345-
},
311+
"GET /client/tlsJA3MD5": {},
312+
"GET /client/tlsClientHello": {},
313+
"GET /client/tlsClientCertificate": {},
314+
"GET /client/tlsCipherOpensslName": {},
315+
"GET /client/tlsProtocol": {},
346316
"GET /config-store": {},
347317
"GET /console": {
348318
"environments": ["viceroy"],

‎runtime/fastly/builtins/fetch-event.cpp

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,12 @@ bool ClientInfo::tls_cipher_openssl_name_get(JSContext *cx, unsigned argc, JS::V
165165
return false;
166166
}
167167

168-
auto cipher = std::move(res.unwrap());
168+
if (!res.unwrap().has_value()) {
169+
args.rval().setNull();
170+
return true;
171+
}
172+
173+
auto cipher = std::move(res.unwrap().value());
169174
result.set(JS_NewStringCopyN(cx, cipher.ptr.get(), cipher.len));
170175
JS::SetReservedSlot(self, static_cast<uint32_t>(ClientInfo::Slots::Cipher),
171176
JS::StringValue(result));
@@ -186,7 +191,12 @@ bool ClientInfo::tls_ja3_md5_get(JSContext *cx, unsigned argc, JS::Value *vp) {
186191
return false;
187192
}
188193

189-
auto ja3 = std::move(res.unwrap());
194+
if (!res.unwrap().has_value()) {
195+
args.rval().setNull();
196+
return true;
197+
}
198+
199+
auto ja3 = std::move(res.unwrap().value());
190200
JS::UniqueChars hex{OPENSSL_buf2hexstr(ja3.ptr.get(), ja3.len)};
191201
std::string ja3hex{hex.get(), std::remove(hex.get(), hex.get() + strlen(hex.get()), ':')};
192202

@@ -209,7 +219,12 @@ bool ClientInfo::tls_client_hello_get(JSContext *cx, unsigned argc, JS::Value *v
209219
return false;
210220
}
211221

212-
auto hello = std::move(res.unwrap());
222+
if (!res.unwrap().has_value()) {
223+
args.rval().setNull();
224+
return true;
225+
}
226+
227+
auto hello = std::move(res.unwrap().value());
213228
buffer.set(JS::NewArrayBufferWithContents(cx, hello.len, hello.ptr.get(),
214229
JS::NewArrayBufferOutOfMemory::CallerMustFreeMemory));
215230
if (!buffer) {
@@ -238,7 +253,13 @@ bool ClientInfo::tls_client_certificate_get(JSContext *cx, unsigned argc, JS::Va
238253
HANDLE_ERROR(cx, *err);
239254
return false;
240255
}
241-
auto cert = std::move(res.unwrap());
256+
257+
if (!res.unwrap().has_value()) {
258+
args.rval().setNull();
259+
return true;
260+
}
261+
262+
auto cert = std::move(res.unwrap().value());
242263

243264
buffer.set(JS::NewArrayBufferWithContents(cx, cert.len, cert.ptr.get(),
244265
JS::NewArrayBufferOutOfMemory::CallerMustFreeMemory));
@@ -269,7 +290,12 @@ bool ClientInfo::tls_protocol_get(JSContext *cx, unsigned argc, JS::Value *vp) {
269290
return false;
270291
}
271292

272-
auto protocol = std::move(res.unwrap());
293+
if (!res.unwrap().has_value()) {
294+
args.rval().setNull();
295+
return true;
296+
}
297+
298+
auto protocol = std::move(res.unwrap().value());
273299
result.set(JS_NewStringCopyN(cx, protocol.ptr.get(), protocol.len));
274300
JS::SetReservedSlot(self, static_cast<uint32_t>(ClientInfo::Slots::Protocol),
275301
JS::StringValue(result));

‎runtime/fastly/host-api/host_api.cpp

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,8 +1261,8 @@ Result<HostBytes> HttpReq::downstream_server_ip_addr() {
12611261
}
12621262

12631263
// http-req-downstream-tls-cipher-openssl-name: func() -> result<string, error>
1264-
Result<HostString> HttpReq::http_req_downstream_tls_cipher_openssl_name() {
1265-
Result<HostString> res;
1264+
Result<std::optional<HostString>> HttpReq::http_req_downstream_tls_cipher_openssl_name() {
1265+
Result<std::optional<HostString>> res;
12661266

12671267
fastly::fastly_host_error err;
12681268
fastly::fastly_world_string ret;
@@ -1278,7 +1278,11 @@ Result<HostString> HttpReq::http_req_downstream_tls_cipher_openssl_name() {
12781278

12791279
if (!convert_result(status, &err)) {
12801280
cabi_free(ret.ptr);
1281-
res.emplace_err(err);
1281+
if (error_is_optional_none(err)) {
1282+
res.emplace(std::nullopt);
1283+
} else {
1284+
res.emplace_err(err);
1285+
}
12821286
} else {
12831287
res.emplace(make_host_string(ret));
12841288
}
@@ -1287,8 +1291,8 @@ Result<HostString> HttpReq::http_req_downstream_tls_cipher_openssl_name() {
12871291
}
12881292

12891293
// http-req-downstream-tls-protocol: func() -> result<string, error>
1290-
Result<HostString> HttpReq::http_req_downstream_tls_protocol() {
1291-
Result<HostString> res;
1294+
Result<std::optional<HostString>> HttpReq::http_req_downstream_tls_protocol() {
1295+
Result<std::optional<HostString>> res;
12921296

12931297
fastly::fastly_host_error err;
12941298
fastly::fastly_world_string ret;
@@ -1303,7 +1307,11 @@ Result<HostString> HttpReq::http_req_downstream_tls_protocol() {
13031307
}
13041308
if (!convert_result(status, &err)) {
13051309
cabi_free(ret.ptr);
1306-
res.emplace_err(err);
1310+
if (error_is_optional_none(err)) {
1311+
res.emplace(std::nullopt);
1312+
} else {
1313+
res.emplace_err(err);
1314+
}
13071315
} else {
13081316
res.emplace(make_host_string(ret));
13091317
}
@@ -1312,8 +1320,8 @@ Result<HostString> HttpReq::http_req_downstream_tls_protocol() {
13121320
}
13131321

13141322
// http-req-downstream-tls-client-hello: func() -> result<list<u8>, error>
1315-
Result<HostBytes> HttpReq::http_req_downstream_tls_client_hello() {
1316-
Result<HostBytes> res;
1323+
Result<std::optional<HostBytes>> HttpReq::http_req_downstream_tls_client_hello() {
1324+
Result<std::optional<HostBytes>> res;
13171325

13181326
fastly::fastly_world_list_u8 ret;
13191327
fastly::fastly_host_error err;
@@ -1327,7 +1335,11 @@ Result<HostBytes> HttpReq::http_req_downstream_tls_client_hello() {
13271335

13281336
if (!convert_result(status, &err)) {
13291337
cabi_free(ret.ptr);
1330-
res.emplace_err(err);
1338+
if (error_is_optional_none(err)) {
1339+
res.emplace(std::nullopt);
1340+
} else {
1341+
res.emplace_err(err);
1342+
}
13311343
} else {
13321344
res.emplace(make_host_bytes(ret));
13331345
}
@@ -1336,8 +1348,8 @@ Result<HostBytes> HttpReq::http_req_downstream_tls_client_hello() {
13361348
}
13371349

13381350
// http-req-downstream-tls-raw-client-certificate: func() -> result<list<u8>, error>
1339-
Result<HostBytes> HttpReq::http_req_downstream_tls_raw_client_certificate() {
1340-
Result<HostBytes> res;
1351+
Result<std::optional<HostBytes>> HttpReq::http_req_downstream_tls_raw_client_certificate() {
1352+
Result<std::optional<HostBytes>> res;
13411353

13421354
fastly::fastly_world_list_u8 ret;
13431355
fastly::fastly_host_error err;
@@ -1350,7 +1362,11 @@ Result<HostBytes> HttpReq::http_req_downstream_tls_raw_client_certificate() {
13501362
}
13511363
if (!convert_result(status, &err)) {
13521364
cabi_free(ret.ptr);
1353-
res.emplace_err(err);
1365+
if (error_is_optional_none(err)) {
1366+
res.emplace(std::nullopt);
1367+
} else {
1368+
res.emplace_err(err);
1369+
}
13541370
} else {
13551371
res.emplace(make_host_bytes(ret));
13561372
}
@@ -1359,8 +1375,8 @@ Result<HostBytes> HttpReq::http_req_downstream_tls_raw_client_certificate() {
13591375
}
13601376

13611377
// http-req-downstream-tls-ja3-md5: func() -> result<list<u8>, error>
1362-
Result<HostBytes> HttpReq::http_req_downstream_tls_ja3_md5() {
1363-
Result<HostBytes> res;
1378+
Result<std::optional<HostBytes>> HttpReq::http_req_downstream_tls_ja3_md5() {
1379+
Result<std::optional<HostBytes>> res;
13641380

13651381
fastly::fastly_world_list_u8 ret;
13661382
fastly::fastly_host_error err;
@@ -1373,7 +1389,11 @@ Result<HostBytes> HttpReq::http_req_downstream_tls_ja3_md5() {
13731389
}
13741390
if (!convert_result(status, &err)) {
13751391
cabi_free(ret.ptr);
1376-
res.emplace_err(err);
1392+
if (error_is_optional_none(err)) {
1393+
res.emplace(std::nullopt);
1394+
} else {
1395+
res.emplace_err(err);
1396+
}
13771397
} else {
13781398
res.emplace(make_host_bytes(ret));
13791399
}

‎runtime/fastly/host-api/host_api_fastly.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -402,15 +402,15 @@ class HttpReq final : public HttpBase {
402402

403403
static Result<HostBytes> downstream_server_ip_addr();
404404

405-
static Result<HostString> http_req_downstream_tls_cipher_openssl_name();
405+
static Result<std::optional<HostString>> http_req_downstream_tls_cipher_openssl_name();
406406

407-
static Result<HostString> http_req_downstream_tls_protocol();
407+
static Result<std::optional<HostString>> http_req_downstream_tls_protocol();
408408

409-
static Result<HostBytes> http_req_downstream_tls_client_hello();
409+
static Result<std::optional<HostBytes>> http_req_downstream_tls_client_hello();
410410

411-
static Result<HostBytes> http_req_downstream_tls_raw_client_certificate();
411+
static Result<std::optional<HostBytes>> http_req_downstream_tls_raw_client_certificate();
412412

413-
static Result<HostBytes> http_req_downstream_tls_ja3_md5();
413+
static Result<std::optional<HostBytes>> http_req_downstream_tls_ja3_md5();
414414

415415
Result<Void> auto_decompress_gzip();
416416

‎test-d/globals.test-d.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -208,11 +208,11 @@ import { expectError, expectType } from 'tsd';
208208
const client = {} as ClientInfo
209209
expectType<string>(client.address)
210210
expectType<Geolocation | null>(client.geo)
211-
expectType<string>(client.tlsJA3MD5)
212-
expectType<string>(client.tlsCipherOpensslName)
213-
expectType<string>(client.tlsProtocol)
214-
expectType<ArrayBuffer>(client.tlsClientCertificate)
215-
expectType<ArrayBuffer>(client.tlsClientHello)
211+
expectType<string | null>(client.tlsJA3MD5)
212+
expectType<string | null>(client.tlsCipherOpensslName)
213+
expectType<string | null>(client.tlsProtocol)
214+
expectType<ArrayBuffer | null>(client.tlsClientCertificate)
215+
expectType<ArrayBuffer | null>(client.tlsClientHello)
216216
// They are readonly properties
217217
expectError(client.address = '')
218218
expectError(client.geo = {} as Geolocation)

0 commit comments

Comments
 (0)