Skip to content

Commit b1a432f

Browse files
authored
chore(types): enable strictNullChecks in base config with explicit null guards in server/config/core (#20510)
- enable strictNullChecks in base TypeScript config - fix strict-null regressions in server, config-lib, and core-lib with explicit null/undefined checks (no implicit truthy/falsy checks in touched code) - improve unhandled rejection type logging with explicit type narrowing - add/update tests for new null-handling behavior: - client build date parsing when version is missing - server config defaults when URLs are missing - link preview audit upload with missing title - browser util test helper null safety - keep webapp temporarily opted out via local strictNullChecks: false override due to existing nullability backlog and unresolved module typing in webapp:type-check
1 parent 2b43bae commit b1a432f

File tree

11 files changed

+241
-64
lines changed

11 files changed

+241
-64
lines changed

apps/server/index.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,16 @@ import {formatDate} from './util/TimeUtil';
2323

2424
const server = new Server(serverConfig, clientConfig);
2525

26+
function getUnhandledRejectionType(unhandledRejection: unknown): string {
27+
if (unhandledRejection instanceof Error) {
28+
return unhandledRejection.name;
29+
}
30+
if (unhandledRejection === null) {
31+
return 'null';
32+
}
33+
return typeof unhandledRejection;
34+
}
35+
2636
server
2737
.start()
2838
.then(port => {
@@ -37,5 +47,5 @@ process.on('uncaughtException', error =>
3747
console.error(`[${formatDate()}] Uncaught exception: ${error.message}`, error),
3848
);
3949
process.on('unhandledRejection', error =>
40-
console.error(`[${formatDate()}] Uncaught rejection "${error.constructor.name}"`, error),
50+
console.error(`[${formatDate()}] Uncaught rejection "${getUnhandledRejectionType(error)}"`, error),
4151
);

apps/server/routes/Root.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,20 @@ async function addGeoIP(req: Request) {
2525
let countryCode = '';
2626

2727
try {
28-
const ip = req.header('X-Forwarded-For') || req.ip;
29-
const lookup = await maxmind.open<CountryResponse>(geolite2.paths.country);
30-
const result = lookup.get(ip);
28+
const ipAddress = req.header('X-Forwarded-For') ?? req.ip ?? '';
29+
if (ipAddress.length === 0) {
30+
return;
31+
}
32+
33+
const countryDatabasePath = geolite2.paths.country;
34+
if (countryDatabasePath === undefined) {
35+
return;
36+
}
37+
38+
const lookup = await maxmind.open<CountryResponse>(countryDatabasePath);
39+
const result = lookup.get(ipAddress);
3140
if (result) {
32-
countryCode = result.country.iso_code;
41+
countryCode = result.country?.iso_code ?? '';
3342
}
3443
} catch (error) {
3544
// It's okay to go without a detected country.

apps/server/routes/client-version-check/ClientBuildDate.test.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import {Maybe, Result} from 'true-myth';
22
import {parseMinimumRequiredClientBuildDate, ParseMinimumRequiredClientBuildDateDependencies} from './ClientBuildDate';
33

4+
const validClientVersion = '2026.02.12.17.51.00';
5+
46
type Overrides = {
57
readonly parseClientVersion?: jest.Mock;
68
readonly clientVersion?: string | undefined;
@@ -16,20 +18,34 @@ function createParseMinimumRequiredClientBuildDateDependencies(
1618
}
1719

1820
describe('parseMinimumRequiredClientBuildDate()', () => {
19-
it('returns a Nothing when parseClientVersion() returns a Result Err', () => {
21+
it('returns a Nothing when no client version is configured', () => {
22+
const parseClientVersion = jest.fn();
2023
const dependencies = createParseMinimumRequiredClientBuildDateDependencies({
21-
parseClientVersion: jest.fn().mockReturnValue(Result.err()),
24+
parseClientVersion,
25+
clientVersion: undefined,
2226
});
2327

2428
expect(parseMinimumRequiredClientBuildDate(dependencies)).toStrictEqual(Maybe.nothing());
29+
expect(parseClientVersion).not.toHaveBeenCalled();
2530
});
2631

27-
it('returns a Just when parseClientVersion() returns a Result Ok', () => {
28-
const expectedDate = new Date(2026, 1, 12, 17, 51, 0);
32+
it.each([
33+
{
34+
description: 'returns a Nothing when parseClientVersion() returns a Result Err',
35+
parseResult: Result.err<Date, Error>(),
36+
expectedResult: Maybe.nothing<Date>(),
37+
},
38+
{
39+
description: 'returns a Just when parseClientVersion() returns a Result Ok',
40+
parseResult: Result.ok(new Date(2026, 1, 12, 17, 51, 0)),
41+
expectedResult: Maybe.just(new Date(2026, 1, 12, 17, 51, 0)),
42+
},
43+
])('$description', ({parseResult, expectedResult}) => {
2944
const dependencies = createParseMinimumRequiredClientBuildDateDependencies({
30-
parseClientVersion: jest.fn().mockReturnValue(Result.ok(expectedDate)),
45+
parseClientVersion: jest.fn().mockReturnValue(parseResult),
46+
clientVersion: validClientVersion,
3147
});
3248

33-
expect(parseMinimumRequiredClientBuildDate(dependencies)).toStrictEqual(Maybe.just(expectedDate));
49+
expect(parseMinimumRequiredClientBuildDate(dependencies)).toStrictEqual(expectedResult);
3450
});
3551
});

apps/server/routes/client-version-check/ClientBuildDate.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,5 +29,9 @@ export function parseMinimumRequiredClientBuildDate(
2929
): Maybe<Date> {
3030
const {parseClientVersion, clientVersion} = dependencies;
3131

32+
if (clientVersion === undefined || clientVersion.length === 0) {
33+
return Maybe.nothing();
34+
}
35+
3236
return toolbelt.fromResult(parseClientVersion(clientVersion));
3337
}

apps/server/util/BrowserUtil.test.ts

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,36 +19,61 @@
1919

2020
import * as BrowserUtil from './BrowserUtil';
2121

22-
describe('BrowserUtil', () => {
23-
it('detects iPhone', () => {
24-
const userAgent =
25-
'Mozilla/5.0 (iPhone; CPU iPhone OS 10_0_1 like Mac OS X) AppleWebKit/602.1.50 (KHTML, like Gecko) Version/10.0 Mobile/14A403 Safari/602.1';
26-
27-
expect(BrowserUtil.parseUserAgent(userAgent).is.ios).toBe(true);
28-
expect(BrowserUtil.parseUserAgent(userAgent).is.mobile).toBe(true);
29-
});
22+
type UserAgentExpectation = readonly [
23+
description: string,
24+
userAgent: string,
25+
expectedIos: boolean,
26+
expectedAndroid: boolean,
27+
expectedMobile: boolean,
28+
];
3029

31-
it('detects iPad', () => {
32-
const userAgent =
33-
'Mozilla/5.0 (iPad; CPU OS 11_0 like Mac OS X) AppleWebKit/604.1.34 (KHTML, like Gecko) Version/11.0 Mobile/15A5341f Safari/604.1';
30+
function parseUserAgentOrThrow(userAgent: string) {
31+
const parsedUserAgent = BrowserUtil.parseUserAgent(userAgent);
3432

35-
expect(BrowserUtil.parseUserAgent(userAgent).is.ios).toBe(true);
36-
expect(BrowserUtil.parseUserAgent(userAgent).is.mobile).toBe(true);
37-
});
33+
if (parsedUserAgent === null) {
34+
throw new Error('Expected parseUserAgent to return a parsed user agent.');
35+
}
3836

39-
it('detects Android', () => {
40-
const userAgent =
41-
'Mozilla/5.0 (Linux; Android 8.0; Pixel 2 Build/OPD3.170816.012) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.110 Mobile Safari/537.36';
37+
return parsedUserAgent;
38+
}
4239

43-
expect(BrowserUtil.parseUserAgent(userAgent).is.android).toBe(true);
44-
expect(BrowserUtil.parseUserAgent(userAgent).is.mobile).toBe(true);
45-
});
40+
const userAgentExpectations: UserAgentExpectation[] = [
41+
[
42+
'detects iPhone',
43+
'Mozilla/5.0 (iPhone; CPU iPhone OS 10_0_1 like Mac OS X) AppleWebKit/602.1.50 (KHTML, like Gecko) Version/10.0 Mobile/14A403 Safari/602.1',
44+
true,
45+
false,
46+
true,
47+
],
48+
[
49+
'detects iPad',
50+
'Mozilla/5.0 (iPad; CPU OS 11_0 like Mac OS X) AppleWebKit/604.1.34 (KHTML, like Gecko) Version/11.0 Mobile/15A5341f Safari/604.1',
51+
true,
52+
false,
53+
true,
54+
],
55+
[
56+
'detects Android',
57+
'Mozilla/5.0 (Linux; Android 8.0; Pixel 2 Build/OPD3.170816.012) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.110 Mobile Safari/537.36',
58+
false,
59+
true,
60+
true,
61+
],
62+
[
63+
'detects Android tablet',
64+
'Mozilla/5.0 (Linux; Android 7.1; vivo 1716 Build/N2G47H) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.98 Mobile Safari/537.36',
65+
false,
66+
true,
67+
true,
68+
],
69+
];
4670

47-
it('detects Android tablet', () => {
48-
const userAgent =
49-
'Mozilla/5.0 (Linux; Android 7.1; vivo 1716 Build/N2G47H) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.98 Mobile Safari/537.36';
71+
describe('BrowserUtil', () => {
72+
it.each(userAgentExpectations)('%s', (_description, userAgent, expectedIos, expectedAndroid, expectedMobile) => {
73+
const parsedUserAgent = parseUserAgentOrThrow(userAgent);
5074

51-
expect(BrowserUtil.parseUserAgent(userAgent).is.android).toBe(true);
52-
expect(BrowserUtil.parseUserAgent(userAgent).is.mobile).toBe(true);
75+
expect(parsedUserAgent.is.ios).toBe(expectedIos);
76+
expect(parsedUserAgent.is.android).toBe(expectedAndroid);
77+
expect(parsedUserAgent.is.mobile).toBe(expectedMobile);
5378
});
5479
});

apps/webapp/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
"rootDirs": ["src"],
2828
"target": "ESNext",
2929
"strict": true,
30+
"strictNullChecks": false,
3031
"strictFunctionTypes": false
3132
},
3233
"types": ["@types/wicg-file-system-access", "jest", "@testing-library/jest-dom"],

libraries/config/src/server.config.spec.ts

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,33 @@ describe('Server Config', () => {
6565
expect(config.ENVIRONMENT).toBe('production');
6666
});
6767

68-
it('should map URL parameters correctly', () => {
69-
const config = generateConfig(mockParams, mockEnv);
70-
71-
expect(config.APP_BASE).toBe('https://app.wire.com');
72-
expect(config.BACKEND_REST).toBe('https://prod-nginz-https.wire.com');
73-
expect(config.BACKEND_WS).toBe('wss://prod-nginz-ssl.wire.com');
68+
it.each([
69+
{
70+
description: 'maps configured URL parameters',
71+
params: mockParams,
72+
expectedAppBase: 'https://app.wire.com',
73+
expectedBackendRest: 'https://prod-nginz-https.wire.com',
74+
expectedBackendWebSocket: 'wss://prod-nginz-ssl.wire.com',
75+
expectedTlsEnabled: true,
76+
},
77+
{
78+
description: 'defaults missing URL parameters to empty strings',
79+
params: {
80+
...mockParams,
81+
urls: {},
82+
},
83+
expectedAppBase: '',
84+
expectedBackendRest: '',
85+
expectedBackendWebSocket: '',
86+
expectedTlsEnabled: false,
87+
},
88+
])('should $description', ({params, expectedAppBase, expectedBackendRest, expectedBackendWebSocket, expectedTlsEnabled}) => {
89+
const config = generateConfig(params, mockEnv);
90+
91+
expect(config.APP_BASE).toBe(expectedAppBase);
92+
expect(config.BACKEND_REST).toBe(expectedBackendRest);
93+
expect(config.BACKEND_WS).toBe(expectedBackendWebSocket);
94+
expect(config.DEVELOPMENT_ENABLE_TLS).toBe(expectedTlsEnabled);
7495
});
7596

7697
it('should parse PORT correctly with default', () => {

libraries/config/src/server.config.ts

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ const logger = logdown('config', {
5050
markdown: false,
5151
});
5252

53-
function readFile(filePath: string, fallback?: string): string {
53+
function readFile(filePath: string, fallback: string = ''): string {
5454
try {
5555
return fs.readFileSync(filePath, {encoding: 'utf8', flag: 'r'});
5656
} catch (error) {
@@ -59,15 +59,28 @@ function readFile(filePath: string, fallback?: string): string {
5959
}
6060
}
6161

62-
function parseCommaSeparatedList(list: string = ''): string[] {
62+
function parseCommaSeparatedList(list: string | undefined = ''): string[] {
6363
const cleanedList = list.replace(/\s/g, '');
64-
if (!cleanedList) {
64+
if (cleanedList.length === 0) {
6565
return [];
6666
}
6767
return cleanedList.split(',');
6868
}
6969

70-
function mergedCSP({urls}: ConfigGeneratorParams, env: Record<string, string>): Record<string, Iterable<string>> {
70+
function getNonEmptyStringValueOrDefault(value: string | undefined, fallback: string): string {
71+
return value !== undefined && value.length > 0 ? value : fallback;
72+
}
73+
74+
function resolveServerCertificatePath(certificateFileName: string): string {
75+
const certificatePathInWorkspaceRoot = path.resolve(process.cwd(), `certificate/${certificateFileName}`);
76+
if (fs.existsSync(certificatePathInWorkspaceRoot)) {
77+
return certificatePathInWorkspaceRoot;
78+
}
79+
80+
return path.resolve(process.cwd(), `apps/server/dist/certificate/${certificateFileName}`);
81+
}
82+
83+
function mergedCSP({urls}: ConfigGeneratorParams, env: Env): Record<string, Iterable<string>> {
7184
const objectSrc = parseCommaSeparatedList(env.CSP_EXTRA_OBJECT_SRC);
7285
const csp = {
7386
connectSrc: [
@@ -96,16 +109,27 @@ function mergedCSP({urls}: ConfigGeneratorParams, env: Record<string, string>):
96109

97110
export function generateConfig(params: ConfigGeneratorParams, env: Env) {
98111
const {commit, version, urls, env: nodeEnv} = params;
112+
const baseUrl = urls.base ?? '';
113+
const apiUrl = urls.api ?? '';
114+
const websocketUrl = urls.ws ?? '';
115+
const parsedHttpPort = Number(env.PORT);
116+
const isHttpPortMissingOrZero = Number.isNaN(parsedHttpPort) || parsedHttpPort === 0;
117+
const httpPort = isHttpPortMissingOrZero ? 21080 : parsedHttpPort;
118+
const defaultSslCertificateKeyPath = resolveServerCertificatePath('development-key.pem');
119+
const defaultSslCertificatePath = resolveServerCertificatePath('development-cert.pem');
120+
const sslCertificateKeyPath = getNonEmptyStringValueOrDefault(env.SSL_CERTIFICATE_KEY_PATH, defaultSslCertificateKeyPath);
121+
const sslCertificatePath = getNonEmptyStringValueOrDefault(env.SSL_CERTIFICATE_PATH, defaultSslCertificatePath);
122+
99123
return {
100-
APP_BASE: urls.base,
124+
APP_BASE: baseUrl,
101125
COMMIT: commit,
102126
VERSION: version,
103127
CACHE_DURATION_SECONDS: 300,
104128
CSP: mergedCSP(params, env),
105-
BACKEND_REST: urls.api,
106-
BACKEND_WS: urls.ws,
129+
BACKEND_REST: apiUrl,
130+
BACKEND_WS: websocketUrl,
107131
DEVELOPMENT: nodeEnv === 'development',
108-
DEVELOPMENT_ENABLE_TLS: urls.base.startsWith('https://'),
132+
DEVELOPMENT_ENABLE_TLS: baseUrl.startsWith('https://'),
109133
ENFORCE_HTTPS: env.ENFORCE_HTTPS != 'false',
110134
ENVIRONMENT: nodeEnv,
111135
GOOGLE_WEBMASTER_ID: env.GOOGLE_WEBMASTER_ID,
@@ -115,23 +139,15 @@ export function generateConfig(params: ConfigGeneratorParams, env: Env) {
115139
TITLE: env.OPEN_GRAPH_TITLE,
116140
},
117141
ENABLE_DYNAMIC_HOSTNAME: env.ENABLE_DYNAMIC_HOSTNAME === 'true',
118-
PORT_HTTP: Number(env.PORT) || 21080,
142+
PORT_HTTP: httpPort,
119143
MINIMUM_REQUIRED_CLIENT_BUILD_DATE: env.MINIMUM_REQUIRED_CLIENT_BUILD_DATE,
120144
ROBOTS: {
121145
ALLOW: readFile(ROBOTS_ALLOW_FILE, 'User-agent: *\r\nDisallow: /'),
122146
ALLOWED_HOSTS: ['app.wire.com'],
123147
DISALLOW: readFile(ROBOTS_DISALLOW_FILE, 'User-agent: *\r\nDisallow: /'),
124148
},
125-
SSL_CERTIFICATE_KEY_PATH:
126-
env.SSL_CERTIFICATE_KEY_PATH ||
127-
(fs.existsSync(path.resolve(process.cwd(), 'certificate/development-key.pem'))
128-
? path.resolve(process.cwd(), 'certificate/development-key.pem')
129-
: path.resolve(process.cwd(), 'apps/server/dist/certificate/development-key.pem')),
130-
SSL_CERTIFICATE_PATH:
131-
env.SSL_CERTIFICATE_PATH ||
132-
(fs.existsSync(path.resolve(process.cwd(), 'certificate/development-cert.pem'))
133-
? path.resolve(process.cwd(), 'certificate/development-cert.pem')
134-
: path.resolve(process.cwd(), 'apps/server/dist/certificate/development-cert.pem')),
149+
SSL_CERTIFICATE_KEY_PATH: sslCertificateKeyPath,
150+
SSL_CERTIFICATE_PATH: sslCertificatePath,
135151
} as const;
136152
}
137153

0 commit comments

Comments
 (0)