Skip to content

Commit 283c301

Browse files
wkerckhojoachimvh
authored andcommitted
feat: new helper functions to replace regexes #807
Implemented new StringUtil helper functions: splitCommaSeparated, sanitizeUrlPart, isValidFileName. Added helper functions to HeaderUtil: matchesAuthorizationScheme, hasScheme. Added unit tests for the new helper functions. Refactored codebase to use helper functions instead of regexes if applicable.
1 parent 1b7cc1e commit 283c301

18 files changed

+186
-23
lines changed

src/authentication/BearerWebIdExtractor.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { getLoggerFor } from '../logging/LogUtil';
44
import type { HttpRequest } from '../server/HttpRequest';
55
import { BadRequestHttpError } from '../util/errors/BadRequestHttpError';
66
import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
7+
import { matchesAuthorizationScheme } from '../util/HeaderUtil';
78
import { CredentialGroup } from './Credentials';
89
import type { CredentialSet } from './Credentials';
910
import { CredentialsExtractor } from './CredentialsExtractor';
@@ -19,7 +20,7 @@ export class BearerWebIdExtractor extends CredentialsExtractor {
1920

2021
public async canHandle({ headers }: HttpRequest): Promise<void> {
2122
const { authorization } = headers;
22-
if (!authorization || !/^Bearer /ui.test(authorization)) {
23+
if (!matchesAuthorizationScheme('Bearer', authorization)) {
2324
throw new NotImplementedHttpError('No Bearer Authorization header specified.');
2425
}
2526
}

src/authentication/DPoPWebIdExtractor.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { getLoggerFor } from '../logging/LogUtil';
55
import type { HttpRequest } from '../server/HttpRequest';
66
import { BadRequestHttpError } from '../util/errors/BadRequestHttpError';
77
import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
8+
import { matchesAuthorizationScheme } from '../util/HeaderUtil';
89
import { CredentialGroup } from './Credentials';
910
import type { CredentialSet } from './Credentials';
1011
import { CredentialsExtractor } from './CredentialsExtractor';
@@ -27,7 +28,7 @@ export class DPoPWebIdExtractor extends CredentialsExtractor {
2728

2829
public async canHandle({ headers }: HttpRequest): Promise<void> {
2930
const { authorization } = headers;
30-
if (!authorization || !/^DPoP /ui.test(authorization)) {
31+
if (!matchesAuthorizationScheme('DPoP', authorization)) {
3132
throw new NotImplementedHttpError('No DPoP-bound Authorization header specified.');
3233
}
3334
}

src/authentication/UnsecureWebIdExtractor.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { getLoggerFor } from '../logging/LogUtil';
22
import type { HttpRequest } from '../server/HttpRequest';
33
import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
4+
import { matchesAuthorizationScheme } from '../util/HeaderUtil';
45
import { CredentialGroup } from './Credentials';
56
import type { CredentialSet } from './Credentials';
67
import { CredentialsExtractor } from './CredentialsExtractor';
@@ -13,7 +14,7 @@ export class UnsecureWebIdExtractor extends CredentialsExtractor {
1314

1415
public async canHandle({ headers }: HttpRequest): Promise<void> {
1516
const { authorization } = headers;
16-
if (!authorization || !/^WebID /ui.test(authorization)) {
17+
if (!matchesAuthorizationScheme('WebID', authorization)) {
1718
throw new NotImplementedHttpError('No WebID Authorization header specified.');
1819
}
1920
}

src/http/UnsecureWebSocketsProtocol.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { getLoggerFor } from '../logging/LogUtil';
44
import type { HttpRequest } from '../server/HttpRequest';
55
import { WebSocketHandler } from '../server/WebSocketHandler';
66
import { parseForwarded } from '../util/HeaderUtil';
7+
import { splitCommaSeparated } from '../util/StringUtil';
78
import type { ResourceIdentifier } from './representation/ResourceIdentifier';
89

910
const VERSION = 'solid-0.1';
@@ -36,7 +37,7 @@ class WebSocketListener extends EventEmitter {
3637
if (!protocolHeader) {
3738
this.sendMessage('warning', `Missing Sec-WebSocket-Protocol header, expected value '${VERSION}'`);
3839
} else {
39-
const supportedProtocols = protocolHeader.split(/\s*,\s*/u);
40+
const supportedProtocols = splitCommaSeparated(protocolHeader);
4041
if (!supportedProtocols.includes(VERSION)) {
4142
this.sendMessage('error', `Client does not support protocol ${VERSION}`);
4243
this.stop();

src/http/input/conditions/BasicConditionsParser.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { HttpRequest } from '../../../server/HttpRequest';
33
import type { BasicConditionsOptions } from '../../../storage/BasicConditions';
44
import { BasicConditions } from '../../../storage/BasicConditions';
55
import type { Conditions } from '../../../storage/Conditions';
6+
import { splitCommaSeparated } from '../../../util/StringUtil';
67
import { ConditionsParser } from './ConditionsParser';
78

89
/**
@@ -58,6 +59,9 @@ export class BasicConditionsParser extends ConditionsParser {
5859
* Undefined if there is no value for the given header name.
5960
*/
6061
private parseTagHeader(request: HttpRequest, header: 'if-match' | 'if-none-match'): string[] | undefined {
61-
return request.headers[header]?.trim().split(/\s*,\s*/u);
62+
const headerValue = request.headers[header];
63+
if (headerValue) {
64+
return splitCommaSeparated(headerValue.trim());
65+
}
6266
}
6367
}

src/identity/interaction/email-password/util/RegistrationManager.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { getLoggerFor } from '../../../../logging/LogUtil';
44
import type { IdentifierGenerator } from '../../../../pods/generate/IdentifierGenerator';
55
import type { PodManager } from '../../../../pods/PodManager';
66
import type { PodSettings } from '../../../../pods/settings/PodSettings';
7+
import { hasScheme } from '../../../../util/HeaderUtil';
78
import { joinUrl } from '../../../../util/PathUtil';
89
import type { OwnershipValidator } from '../../../ownership/OwnershipValidator';
910
import { assertPassword } from '../EmailPasswordUtil';
@@ -139,7 +140,7 @@ export class RegistrationManager {
139140
// Parse WebID
140141
if (!validated.createWebId) {
141142
const trimmedWebId = this.trimString(webId);
142-
assert(trimmedWebId && /^https?:\/\/[^/]+/u.test(trimmedWebId), 'Please enter a valid WebID.');
143+
assert(trimmedWebId && hasScheme(trimmedWebId, 'http', 'https'), 'Please enter a valid WebID.');
143144
validated.webId = trimmedWebId;
144145
}
145146

src/identity/storage/WebIdAdapterFactory.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { getLoggerFor } from '../../logging/LogUtil';
66
import type { RepresentationConverter } from '../../storage/conversion/RepresentationConverter';
77
import { createErrorMessage } from '../../util/errors/ErrorUtil';
88
import { responseToDataset } from '../../util/FetchUtil';
9+
import { hasScheme } from '../../util/HeaderUtil';
910
import { OIDC } from '../../util/Vocabularies';
1011
import type { AdapterFactory } from './AdapterFactory';
1112

@@ -42,7 +43,7 @@ export class WebIdAdapter implements Adapter {
4243
// Try to see if valid client metadata is found at the given Client ID.
4344
// The oidc-provider library will check if the redirect_uri matches an entry in the list of redirect_uris,
4445
// so no extra checks are needed from our side.
45-
if (!payload && this.name === 'Client' && /^https?:\/\/.+/u.test(id)) {
46+
if (!payload && this.name === 'Client' && hasScheme(id, 'http', 'https')) {
4647
this.logger.debug(`Looking for payload data at ${id}`);
4748
// All checks based on https://solid.github.io/authentication-panel/solid-oidc/#clientids-webid
4849
if (!/^https:|^http:\/\/localhost(?::\d+)?(?:\/|$)/u.test(id)) {

src/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,7 @@ export * from './util/QuadUtil';
433433
export * from './util/RecordObject';
434434
export * from './util/ResourceUtil';
435435
export * from './util/StreamUtil';
436+
export * from './util/StringUtil';
436437
export * from './util/TermUtil';
437438
export * from './util/TimerUtil';
438439
export * from './util/Vocabularies';

src/pods/generate/SubdomainIdentifierGenerator.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier';
22
import { ensureTrailingSlash, extractScheme } from '../../util/PathUtil';
3+
import { sanitizeUrlPart } from '../../util/StringUtil';
34
import type { IdentifierGenerator } from './IdentifierGenerator';
45

56
/**
@@ -15,7 +16,7 @@ export class SubdomainIdentifierGenerator implements IdentifierGenerator {
1516

1617
public generate(name: string): ResourceIdentifier {
1718
// Using the punycode converter is a risk as it doesn't convert slashes for example
18-
const cleanName = name.replace(/\W/gu, '-');
19+
const cleanName = sanitizeUrlPart(name);
1920
return { path: `${this.baseParts.scheme}${cleanName}.${this.baseParts.rest}` };
2021
}
2122
}

src/pods/generate/SuffixIdentifierGenerator.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier';
22
import { ensureTrailingSlash } from '../../util/PathUtil';
3+
import { sanitizeUrlPart } from '../../util/StringUtil';
34
import type { IdentifierGenerator } from './IdentifierGenerator';
45

56
/**
@@ -14,7 +15,7 @@ export class SuffixIdentifierGenerator implements IdentifierGenerator {
1415
}
1516

1617
public generate(name: string): ResourceIdentifier {
17-
const cleanName = name.replace(/\W/gu, '-');
18+
const cleanName = sanitizeUrlPart(name);
1819
return { path: ensureTrailingSlash(new URL(cleanName, this.base).href) };
1920
}
2021
}

src/server/middleware/WebSocketAdvertiser.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { addHeader } from '../../util/HeaderUtil';
1+
import { addHeader, hasScheme } from '../../util/HeaderUtil';
22
import { HttpHandler } from '../HttpHandler';
33
import type { HttpResponse } from '../HttpResponse';
44

@@ -11,7 +11,7 @@ export class WebSocketAdvertiser extends HttpHandler {
1111
public constructor(baseUrl: string) {
1212
super();
1313
const socketUrl = new URL(baseUrl);
14-
socketUrl.protocol = /^(?:http|ws):/u.test(baseUrl) ? 'ws:' : 'wss:';
14+
socketUrl.protocol = hasScheme(baseUrl, 'http', 'ws') ? 'ws:' : 'wss:';
1515
this.socketUrl = socketUrl.href;
1616
}
1717

src/storage/IndexRepresentationStore.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { RepresentationPreferences } from '../http/representation/Represent
44
import type { ResourceIdentifier } from '../http/representation/ResourceIdentifier';
55
import { NotFoundHttpError } from '../util/errors/NotFoundHttpError';
66
import { isContainerIdentifier } from '../util/PathUtil';
7+
import { isValidFileName } from '../util/StringUtil';
78
import type { Conditions } from './Conditions';
89
import { cleanPreferences, matchesMediaType } from './conversion/ConversionUtil';
910
import { PassthroughStore } from './PassthroughStore';
@@ -31,7 +32,7 @@ export class IndexRepresentationStore extends PassthroughStore {
3132

3233
public constructor(source: ResourceStore, indexName = 'index.html', mediaRange = 'text/html') {
3334
super(source);
34-
assert(/^[\w.-]+$/u.test(indexName), 'Invalid index name');
35+
assert(isValidFileName(indexName), 'Invalid index name');
3536
this.indexName = indexName;
3637
this.mediaRange = mediaRange;
3738
}

src/storage/conversion/ErrorToTemplateConverter.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { INTERNAL_ERROR } from '../../util/ContentTypes';
55
import { HttpError } from '../../util/errors/HttpError';
66
import { resolveModulePath } from '../../util/PathUtil';
77
import { getSingleItem } from '../../util/StreamUtil';
8+
import { isValidFileName } from '../../util/StringUtil';
89
import type { TemplateEngine } from '../../util/templates/TemplateEngine';
910
import { BaseTypedRepresentationConverter } from './BaseTypedRepresentationConverter';
1011
import type { RepresentationConverterArgs } from './RepresentationConverter';
@@ -63,7 +64,7 @@ export class ErrorToTemplateConverter extends BaseTypedRepresentationConverter {
6364
if (HttpError.isInstance(error)) {
6465
try {
6566
const templateFile = `${error.errorCode}${this.extension}`;
66-
assert(/^[\w.-]+$/u.test(templateFile), 'Invalid error template name');
67+
assert(isValidFileName(templateFile), 'Invalid error template name');
6768
description = await this.templateEngine.render(error.details ?? {},
6869
{ templateFile, templatePath: this.codeTemplatesPath });
6970
} catch {

src/util/HeaderUtil.ts

+33
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import type { IncomingHttpHeaders } from 'http';
2+
import escapeStringRegexp from 'escape-string-regexp';
23
import { getLoggerFor } from '../logging/LogUtil';
34
import type { HttpResponse } from '../server/HttpResponse';
45
import { BadRequestHttpError } from './errors/BadRequestHttpError';
56

67
const logger = getLoggerFor('HeaderUtil');
8+
// Map used as a simple cache in the helper function matchesAuthorizationScheme.
9+
const authSchemeRegexCache: Map<string, RegExp> = new Map();
710

811
// BNF based on https://tools.ietf.org/html/rfc7231
912
//
@@ -508,6 +511,7 @@ export function parseForwarded(headers: IncomingHttpHeaders): Forwarded {
508511

509512
/**
510513
* Parses the link header(s) and returns an array of LinkEntry objects.
514+
*
511515
* @param link - A single link header or an array of link headers
512516
* @returns A LinkEntry array, LinkEntry contains a link and a params Record&lt;string,string&gt;
513517
*/
@@ -547,3 +551,32 @@ export function parseLinkHeader(link: string | string[] = []): LinkEntry[] {
547551
}
548552
return links;
549553
}
554+
555+
/**
556+
* Checks if the value of an HTTP Authorization header matches a specific scheme (e.g. Basic, Bearer, etc).
557+
*
558+
* @param scheme - Name of the authorization scheme (case insensitive).
559+
* @param authorization - The value of the Authorization header (may be undefined).
560+
* @returns True if the Authorization header uses the specified scheme, false otherwise.
561+
*/
562+
export function matchesAuthorizationScheme(scheme: string, authorization?: string): boolean {
563+
const lowerCaseScheme = scheme.toLowerCase();
564+
if (!authSchemeRegexCache.has(lowerCaseScheme)) {
565+
authSchemeRegexCache.set(lowerCaseScheme, new RegExp(`^${escapeStringRegexp(lowerCaseScheme)} `, 'ui'));
566+
}
567+
// Support authorization being undefined (for the sake of usability).
568+
return typeof authorization !== 'undefined' && authSchemeRegexCache.get(lowerCaseScheme)!.test(authorization);
569+
}
570+
571+
/**
572+
* Checks if the scheme part of the specified url matches at least one of the provided options.
573+
*
574+
* @param url - A string representing the URL.
575+
* @param schemes - Scheme value options (the function will check if at least one matches the URL scheme).
576+
* @returns True if the URL scheme matches at least one of the provided options, false otherwise.
577+
*/
578+
export function hasScheme(url: string, ...schemes: string[]): boolean {
579+
const schemeOptions = new Set(schemes.map((item): string => item.toLowerCase()));
580+
const urlSchemeResult = /^(.+?):\/\//u.exec(url);
581+
return urlSchemeResult ? schemeOptions.has(urlSchemeResult[1].toLowerCase()) : false;
582+
}

src/util/StringUtil.ts

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* Splits a string by comma.
3+
*
4+
* @param input - String instance to split.
5+
*
6+
* @returns A String array containining the split parts.
7+
*/
8+
export function splitCommaSeparated(input: string): string[] {
9+
return input.split(/\s*,\s*/u);
10+
}
11+
12+
/**
13+
* Sanitizes part of a URL by replacing non-word content with a '-'.
14+
*
15+
* @param urlPart - The URL part to sanitize.
16+
* @returns The sanitized output.
17+
*/
18+
export function sanitizeUrlPart(urlPart: string): string {
19+
return urlPart.replace(/\W/gu, '-');
20+
}
21+
22+
/**
23+
* Checks the validity of a file name. A valid name consists of word characters, '-' or '.'.
24+
*
25+
* @param name - The name of the file to validate.
26+
* @returns True if the filename is valid, false otherwise.
27+
*/
28+
export function isValidFileName(name: string): boolean {
29+
return /^[\w.-]+$/u.test(name);
30+
}

test/integration/Middleware.test.ts

+11-10
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import request from 'supertest';
33
import type { BaseHttpServerFactory } from '../../src/server/BaseHttpServerFactory';
44
import type { HttpHandlerInput } from '../../src/server/HttpHandler';
55
import { HttpHandler } from '../../src/server/HttpHandler';
6+
import { splitCommaSeparated } from '../../src/util/StringUtil';
67
import { getPort } from '../util/Util';
78
import { getTestConfigPath, instantiateFromConfig } from './Config';
89

@@ -96,46 +97,46 @@ describe('An http server with middleware', (): void => {
9697
it('exposes the Accept-[Method] header via CORS.', async(): Promise<void> => {
9798
const res = await request(server).get('/').expect(200);
9899
const exposed = res.header['access-control-expose-headers'];
99-
expect(exposed.split(/\s*,\s*/u)).toContain('Accept-Patch');
100-
expect(exposed.split(/\s*,\s*/u)).toContain('Accept-Post');
101-
expect(exposed.split(/\s*,\s*/u)).toContain('Accept-Put');
100+
expect(splitCommaSeparated(exposed)).toContain('Accept-Patch');
101+
expect(splitCommaSeparated(exposed)).toContain('Accept-Post');
102+
expect(splitCommaSeparated(exposed)).toContain('Accept-Put');
102103
});
103104

104105
it('exposes the Last-Modified and ETag headers via CORS.', async(): Promise<void> => {
105106
const res = await request(server).get('/').expect(200);
106107
const exposed = res.header['access-control-expose-headers'];
107-
expect(exposed.split(/\s*,\s*/u)).toContain('ETag');
108-
expect(exposed.split(/\s*,\s*/u)).toContain('Last-Modified');
108+
expect(splitCommaSeparated(exposed)).toContain('ETag');
109+
expect(splitCommaSeparated(exposed)).toContain('Last-Modified');
109110
});
110111

111112
it('exposes the Link header via CORS.', async(): Promise<void> => {
112113
const res = await request(server).get('/').expect(200);
113114
const exposed = res.header['access-control-expose-headers'];
114-
expect(exposed.split(/\s*,\s*/u)).toContain('Link');
115+
expect(splitCommaSeparated(exposed)).toContain('Link');
115116
});
116117

117118
it('exposes the Location header via CORS.', async(): Promise<void> => {
118119
const res = await request(server).get('/').expect(200);
119120
const exposed = res.header['access-control-expose-headers'];
120-
expect(exposed.split(/\s*,\s*/u)).toContain('Location');
121+
expect(splitCommaSeparated(exposed)).toContain('Location');
121122
});
122123

123124
it('exposes the MS-Author-Via header via CORS.', async(): Promise<void> => {
124125
const res = await request(server).get('/').expect(200);
125126
const exposed = res.header['access-control-expose-headers'];
126-
expect(exposed.split(/\s*,\s*/u)).toContain('MS-Author-Via');
127+
expect(splitCommaSeparated(exposed)).toContain('MS-Author-Via');
127128
});
128129

129130
it('exposes the WAC-Allow header via CORS.', async(): Promise<void> => {
130131
const res = await request(server).get('/').expect(200);
131132
const exposed = res.header['access-control-expose-headers'];
132-
expect(exposed.split(/\s*,\s*/u)).toContain('WAC-Allow');
133+
expect(splitCommaSeparated(exposed)).toContain('WAC-Allow');
133134
});
134135

135136
it('exposes the Updates-Via header via CORS.', async(): Promise<void> => {
136137
const res = await request(server).get('/').expect(200);
137138
const exposed = res.header['access-control-expose-headers'];
138-
expect(exposed.split(/\s*,\s*/u)).toContain('Updates-Via');
139+
expect(splitCommaSeparated(exposed)).toContain('Updates-Via');
139140
});
140141

141142
it('sends incoming requests to the handler.', async(): Promise<void> => {

0 commit comments

Comments
 (0)