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.
This commit is contained in:
Wannes Kerckhove 2022-04-08 16:54:33 +02:00 committed by Joachim Van Herwegen
parent 1b7cc1ea3a
commit 283c301f08
18 changed files with 186 additions and 23 deletions

View File

@ -4,6 +4,7 @@ import { getLoggerFor } from '../logging/LogUtil';
import type { HttpRequest } from '../server/HttpRequest';
import { BadRequestHttpError } from '../util/errors/BadRequestHttpError';
import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
import { matchesAuthorizationScheme } from '../util/HeaderUtil';
import { CredentialGroup } from './Credentials';
import type { CredentialSet } from './Credentials';
import { CredentialsExtractor } from './CredentialsExtractor';
@ -19,7 +20,7 @@ export class BearerWebIdExtractor extends CredentialsExtractor {
public async canHandle({ headers }: HttpRequest): Promise<void> {
const { authorization } = headers;
if (!authorization || !/^Bearer /ui.test(authorization)) {
if (!matchesAuthorizationScheme('Bearer', authorization)) {
throw new NotImplementedHttpError('No Bearer Authorization header specified.');
}
}

View File

@ -5,6 +5,7 @@ import { getLoggerFor } from '../logging/LogUtil';
import type { HttpRequest } from '../server/HttpRequest';
import { BadRequestHttpError } from '../util/errors/BadRequestHttpError';
import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
import { matchesAuthorizationScheme } from '../util/HeaderUtil';
import { CredentialGroup } from './Credentials';
import type { CredentialSet } from './Credentials';
import { CredentialsExtractor } from './CredentialsExtractor';
@ -27,7 +28,7 @@ export class DPoPWebIdExtractor extends CredentialsExtractor {
public async canHandle({ headers }: HttpRequest): Promise<void> {
const { authorization } = headers;
if (!authorization || !/^DPoP /ui.test(authorization)) {
if (!matchesAuthorizationScheme('DPoP', authorization)) {
throw new NotImplementedHttpError('No DPoP-bound Authorization header specified.');
}
}

View File

@ -1,6 +1,7 @@
import { getLoggerFor } from '../logging/LogUtil';
import type { HttpRequest } from '../server/HttpRequest';
import { NotImplementedHttpError } from '../util/errors/NotImplementedHttpError';
import { matchesAuthorizationScheme } from '../util/HeaderUtil';
import { CredentialGroup } from './Credentials';
import type { CredentialSet } from './Credentials';
import { CredentialsExtractor } from './CredentialsExtractor';
@ -13,7 +14,7 @@ export class UnsecureWebIdExtractor extends CredentialsExtractor {
public async canHandle({ headers }: HttpRequest): Promise<void> {
const { authorization } = headers;
if (!authorization || !/^WebID /ui.test(authorization)) {
if (!matchesAuthorizationScheme('WebID', authorization)) {
throw new NotImplementedHttpError('No WebID Authorization header specified.');
}
}

View File

@ -4,6 +4,7 @@ import { getLoggerFor } from '../logging/LogUtil';
import type { HttpRequest } from '../server/HttpRequest';
import { WebSocketHandler } from '../server/WebSocketHandler';
import { parseForwarded } from '../util/HeaderUtil';
import { splitCommaSeparated } from '../util/StringUtil';
import type { ResourceIdentifier } from './representation/ResourceIdentifier';
const VERSION = 'solid-0.1';
@ -36,7 +37,7 @@ class WebSocketListener extends EventEmitter {
if (!protocolHeader) {
this.sendMessage('warning', `Missing Sec-WebSocket-Protocol header, expected value '${VERSION}'`);
} else {
const supportedProtocols = protocolHeader.split(/\s*,\s*/u);
const supportedProtocols = splitCommaSeparated(protocolHeader);
if (!supportedProtocols.includes(VERSION)) {
this.sendMessage('error', `Client does not support protocol ${VERSION}`);
this.stop();

View File

@ -3,6 +3,7 @@ import type { HttpRequest } from '../../../server/HttpRequest';
import type { BasicConditionsOptions } from '../../../storage/BasicConditions';
import { BasicConditions } from '../../../storage/BasicConditions';
import type { Conditions } from '../../../storage/Conditions';
import { splitCommaSeparated } from '../../../util/StringUtil';
import { ConditionsParser } from './ConditionsParser';
/**
@ -58,6 +59,9 @@ export class BasicConditionsParser extends ConditionsParser {
* Undefined if there is no value for the given header name.
*/
private parseTagHeader(request: HttpRequest, header: 'if-match' | 'if-none-match'): string[] | undefined {
return request.headers[header]?.trim().split(/\s*,\s*/u);
const headerValue = request.headers[header];
if (headerValue) {
return splitCommaSeparated(headerValue.trim());
}
}
}

View File

@ -4,6 +4,7 @@ import { getLoggerFor } from '../../../../logging/LogUtil';
import type { IdentifierGenerator } from '../../../../pods/generate/IdentifierGenerator';
import type { PodManager } from '../../../../pods/PodManager';
import type { PodSettings } from '../../../../pods/settings/PodSettings';
import { hasScheme } from '../../../../util/HeaderUtil';
import { joinUrl } from '../../../../util/PathUtil';
import type { OwnershipValidator } from '../../../ownership/OwnershipValidator';
import { assertPassword } from '../EmailPasswordUtil';
@ -139,7 +140,7 @@ export class RegistrationManager {
// Parse WebID
if (!validated.createWebId) {
const trimmedWebId = this.trimString(webId);
assert(trimmedWebId && /^https?:\/\/[^/]+/u.test(trimmedWebId), 'Please enter a valid WebID.');
assert(trimmedWebId && hasScheme(trimmedWebId, 'http', 'https'), 'Please enter a valid WebID.');
validated.webId = trimmedWebId;
}

View File

@ -6,6 +6,7 @@ import { getLoggerFor } from '../../logging/LogUtil';
import type { RepresentationConverter } from '../../storage/conversion/RepresentationConverter';
import { createErrorMessage } from '../../util/errors/ErrorUtil';
import { responseToDataset } from '../../util/FetchUtil';
import { hasScheme } from '../../util/HeaderUtil';
import { OIDC } from '../../util/Vocabularies';
import type { AdapterFactory } from './AdapterFactory';
@ -42,7 +43,7 @@ export class WebIdAdapter implements Adapter {
// Try to see if valid client metadata is found at the given Client ID.
// The oidc-provider library will check if the redirect_uri matches an entry in the list of redirect_uris,
// so no extra checks are needed from our side.
if (!payload && this.name === 'Client' && /^https?:\/\/.+/u.test(id)) {
if (!payload && this.name === 'Client' && hasScheme(id, 'http', 'https')) {
this.logger.debug(`Looking for payload data at ${id}`);
// All checks based on https://solid.github.io/authentication-panel/solid-oidc/#clientids-webid
if (!/^https:|^http:\/\/localhost(?::\d+)?(?:\/|$)/u.test(id)) {

View File

@ -433,6 +433,7 @@ export * from './util/QuadUtil';
export * from './util/RecordObject';
export * from './util/ResourceUtil';
export * from './util/StreamUtil';
export * from './util/StringUtil';
export * from './util/TermUtil';
export * from './util/TimerUtil';
export * from './util/Vocabularies';

View File

@ -1,5 +1,6 @@
import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier';
import { ensureTrailingSlash, extractScheme } from '../../util/PathUtil';
import { sanitizeUrlPart } from '../../util/StringUtil';
import type { IdentifierGenerator } from './IdentifierGenerator';
/**
@ -15,7 +16,7 @@ export class SubdomainIdentifierGenerator implements IdentifierGenerator {
public generate(name: string): ResourceIdentifier {
// Using the punycode converter is a risk as it doesn't convert slashes for example
const cleanName = name.replace(/\W/gu, '-');
const cleanName = sanitizeUrlPart(name);
return { path: `${this.baseParts.scheme}${cleanName}.${this.baseParts.rest}` };
}
}

View File

@ -1,5 +1,6 @@
import type { ResourceIdentifier } from '../../http/representation/ResourceIdentifier';
import { ensureTrailingSlash } from '../../util/PathUtil';
import { sanitizeUrlPart } from '../../util/StringUtil';
import type { IdentifierGenerator } from './IdentifierGenerator';
/**
@ -14,7 +15,7 @@ export class SuffixIdentifierGenerator implements IdentifierGenerator {
}
public generate(name: string): ResourceIdentifier {
const cleanName = name.replace(/\W/gu, '-');
const cleanName = sanitizeUrlPart(name);
return { path: ensureTrailingSlash(new URL(cleanName, this.base).href) };
}
}

View File

@ -1,4 +1,4 @@
import { addHeader } from '../../util/HeaderUtil';
import { addHeader, hasScheme } from '../../util/HeaderUtil';
import { HttpHandler } from '../HttpHandler';
import type { HttpResponse } from '../HttpResponse';
@ -11,7 +11,7 @@ export class WebSocketAdvertiser extends HttpHandler {
public constructor(baseUrl: string) {
super();
const socketUrl = new URL(baseUrl);
socketUrl.protocol = /^(?:http|ws):/u.test(baseUrl) ? 'ws:' : 'wss:';
socketUrl.protocol = hasScheme(baseUrl, 'http', 'ws') ? 'ws:' : 'wss:';
this.socketUrl = socketUrl.href;
}

View File

@ -4,6 +4,7 @@ import type { RepresentationPreferences } from '../http/representation/Represent
import type { ResourceIdentifier } from '../http/representation/ResourceIdentifier';
import { NotFoundHttpError } from '../util/errors/NotFoundHttpError';
import { isContainerIdentifier } from '../util/PathUtil';
import { isValidFileName } from '../util/StringUtil';
import type { Conditions } from './Conditions';
import { cleanPreferences, matchesMediaType } from './conversion/ConversionUtil';
import { PassthroughStore } from './PassthroughStore';
@ -31,7 +32,7 @@ export class IndexRepresentationStore extends PassthroughStore {
public constructor(source: ResourceStore, indexName = 'index.html', mediaRange = 'text/html') {
super(source);
assert(/^[\w.-]+$/u.test(indexName), 'Invalid index name');
assert(isValidFileName(indexName), 'Invalid index name');
this.indexName = indexName;
this.mediaRange = mediaRange;
}

View File

@ -5,6 +5,7 @@ import { INTERNAL_ERROR } from '../../util/ContentTypes';
import { HttpError } from '../../util/errors/HttpError';
import { resolveModulePath } from '../../util/PathUtil';
import { getSingleItem } from '../../util/StreamUtil';
import { isValidFileName } from '../../util/StringUtil';
import type { TemplateEngine } from '../../util/templates/TemplateEngine';
import { BaseTypedRepresentationConverter } from './BaseTypedRepresentationConverter';
import type { RepresentationConverterArgs } from './RepresentationConverter';
@ -63,7 +64,7 @@ export class ErrorToTemplateConverter extends BaseTypedRepresentationConverter {
if (HttpError.isInstance(error)) {
try {
const templateFile = `${error.errorCode}${this.extension}`;
assert(/^[\w.-]+$/u.test(templateFile), 'Invalid error template name');
assert(isValidFileName(templateFile), 'Invalid error template name');
description = await this.templateEngine.render(error.details ?? {},
{ templateFile, templatePath: this.codeTemplatesPath });
} catch {

View File

@ -1,9 +1,12 @@
import type { IncomingHttpHeaders } from 'http';
import escapeStringRegexp from 'escape-string-regexp';
import { getLoggerFor } from '../logging/LogUtil';
import type { HttpResponse } from '../server/HttpResponse';
import { BadRequestHttpError } from './errors/BadRequestHttpError';
const logger = getLoggerFor('HeaderUtil');
// Map used as a simple cache in the helper function matchesAuthorizationScheme.
const authSchemeRegexCache: Map<string, RegExp> = new Map();
// BNF based on https://tools.ietf.org/html/rfc7231
//
@ -508,6 +511,7 @@ export function parseForwarded(headers: IncomingHttpHeaders): Forwarded {
/**
* Parses the link header(s) and returns an array of LinkEntry objects.
*
* @param link - A single link header or an array of link headers
* @returns A LinkEntry array, LinkEntry contains a link and a params Record&lt;string,string&gt;
*/
@ -547,3 +551,32 @@ export function parseLinkHeader(link: string | string[] = []): LinkEntry[] {
}
return links;
}
/**
* Checks if the value of an HTTP Authorization header matches a specific scheme (e.g. Basic, Bearer, etc).
*
* @param scheme - Name of the authorization scheme (case insensitive).
* @param authorization - The value of the Authorization header (may be undefined).
* @returns True if the Authorization header uses the specified scheme, false otherwise.
*/
export function matchesAuthorizationScheme(scheme: string, authorization?: string): boolean {
const lowerCaseScheme = scheme.toLowerCase();
if (!authSchemeRegexCache.has(lowerCaseScheme)) {
authSchemeRegexCache.set(lowerCaseScheme, new RegExp(`^${escapeStringRegexp(lowerCaseScheme)} `, 'ui'));
}
// Support authorization being undefined (for the sake of usability).
return typeof authorization !== 'undefined' && authSchemeRegexCache.get(lowerCaseScheme)!.test(authorization);
}
/**
* Checks if the scheme part of the specified url matches at least one of the provided options.
*
* @param url - A string representing the URL.
* @param schemes - Scheme value options (the function will check if at least one matches the URL scheme).
* @returns True if the URL scheme matches at least one of the provided options, false otherwise.
*/
export function hasScheme(url: string, ...schemes: string[]): boolean {
const schemeOptions = new Set(schemes.map((item): string => item.toLowerCase()));
const urlSchemeResult = /^(.+?):\/\//u.exec(url);
return urlSchemeResult ? schemeOptions.has(urlSchemeResult[1].toLowerCase()) : false;
}

30
src/util/StringUtil.ts Normal file
View File

@ -0,0 +1,30 @@
/**
* Splits a string by comma.
*
* @param input - String instance to split.
*
* @returns A String array containining the split parts.
*/
export function splitCommaSeparated(input: string): string[] {
return input.split(/\s*,\s*/u);
}
/**
* Sanitizes part of a URL by replacing non-word content with a '-'.
*
* @param urlPart - The URL part to sanitize.
* @returns The sanitized output.
*/
export function sanitizeUrlPart(urlPart: string): string {
return urlPart.replace(/\W/gu, '-');
}
/**
* Checks the validity of a file name. A valid name consists of word characters, '-' or '.'.
*
* @param name - The name of the file to validate.
* @returns True if the filename is valid, false otherwise.
*/
export function isValidFileName(name: string): boolean {
return /^[\w.-]+$/u.test(name);
}

View File

@ -3,6 +3,7 @@ import request from 'supertest';
import type { BaseHttpServerFactory } from '../../src/server/BaseHttpServerFactory';
import type { HttpHandlerInput } from '../../src/server/HttpHandler';
import { HttpHandler } from '../../src/server/HttpHandler';
import { splitCommaSeparated } from '../../src/util/StringUtil';
import { getPort } from '../util/Util';
import { getTestConfigPath, instantiateFromConfig } from './Config';
@ -96,46 +97,46 @@ describe('An http server with middleware', (): void => {
it('exposes the Accept-[Method] header via CORS.', async(): Promise<void> => {
const res = await request(server).get('/').expect(200);
const exposed = res.header['access-control-expose-headers'];
expect(exposed.split(/\s*,\s*/u)).toContain('Accept-Patch');
expect(exposed.split(/\s*,\s*/u)).toContain('Accept-Post');
expect(exposed.split(/\s*,\s*/u)).toContain('Accept-Put');
expect(splitCommaSeparated(exposed)).toContain('Accept-Patch');
expect(splitCommaSeparated(exposed)).toContain('Accept-Post');
expect(splitCommaSeparated(exposed)).toContain('Accept-Put');
});
it('exposes the Last-Modified and ETag headers via CORS.', async(): Promise<void> => {
const res = await request(server).get('/').expect(200);
const exposed = res.header['access-control-expose-headers'];
expect(exposed.split(/\s*,\s*/u)).toContain('ETag');
expect(exposed.split(/\s*,\s*/u)).toContain('Last-Modified');
expect(splitCommaSeparated(exposed)).toContain('ETag');
expect(splitCommaSeparated(exposed)).toContain('Last-Modified');
});
it('exposes the Link header via CORS.', async(): Promise<void> => {
const res = await request(server).get('/').expect(200);
const exposed = res.header['access-control-expose-headers'];
expect(exposed.split(/\s*,\s*/u)).toContain('Link');
expect(splitCommaSeparated(exposed)).toContain('Link');
});
it('exposes the Location header via CORS.', async(): Promise<void> => {
const res = await request(server).get('/').expect(200);
const exposed = res.header['access-control-expose-headers'];
expect(exposed.split(/\s*,\s*/u)).toContain('Location');
expect(splitCommaSeparated(exposed)).toContain('Location');
});
it('exposes the MS-Author-Via header via CORS.', async(): Promise<void> => {
const res = await request(server).get('/').expect(200);
const exposed = res.header['access-control-expose-headers'];
expect(exposed.split(/\s*,\s*/u)).toContain('MS-Author-Via');
expect(splitCommaSeparated(exposed)).toContain('MS-Author-Via');
});
it('exposes the WAC-Allow header via CORS.', async(): Promise<void> => {
const res = await request(server).get('/').expect(200);
const exposed = res.header['access-control-expose-headers'];
expect(exposed.split(/\s*,\s*/u)).toContain('WAC-Allow');
expect(splitCommaSeparated(exposed)).toContain('WAC-Allow');
});
it('exposes the Updates-Via header via CORS.', async(): Promise<void> => {
const res = await request(server).get('/').expect(200);
const exposed = res.header['access-control-expose-headers'];
expect(exposed.split(/\s*,\s*/u)).toContain('Updates-Via');
expect(splitCommaSeparated(exposed)).toContain('Updates-Via');
});
it('sends incoming requests to the handler.', async(): Promise<void> => {

View File

@ -2,6 +2,8 @@ import type { HttpResponse } from '../../../src/server/HttpResponse';
import { BadRequestHttpError } from '../../../src/util/errors/BadRequestHttpError';
import {
addHeader,
hasScheme,
matchesAuthorizationScheme,
parseAccept,
parseAcceptCharset,
parseAcceptDateTime,
@ -419,4 +421,52 @@ describe('HeaderUtil', (): void => {
expect(parseLinkHeader()).toEqual([]);
});
});
describe('#matchesAuthorizationScheme', (): void => {
it('returns true if the provided authorization header value matches the provided scheme.', (): void => {
const authorization = `Bearer Q0xXTzl1dTM4RF8xLXllSGx5am51WFUzbzZ2LTZ1WU1GWXpfMTBEajBjaw==`;
expect(matchesAuthorizationScheme('Bearer', authorization)).toBeTruthy();
});
it('returns false if the provided authorization header value does not match the provided scheme.', (): void => {
const authorization = `Basic YWxpY2U6YWxpY2U=`;
expect(matchesAuthorizationScheme('Bearer', authorization)).toBeFalsy();
});
it('correctly detects scheme matches when a different casing is used.', (): void => {
const authorization = `bAsIc YWxpY2U6YWxpY2U=`;
expect(matchesAuthorizationScheme('Basic', authorization)).toBeTruthy();
});
it('escapes special regex characters in the scheme argument, resulting in a correct match.', (): void => {
const authorization = `bA.*sIc$ YWxpY2U6YWxpY2U=`;
expect(matchesAuthorizationScheme('bA.*sIc$', authorization)).toBeTruthy();
});
it('returns false if the authorization argument is undefined.', (): void => {
expect(matchesAuthorizationScheme('Bearer')).toBeFalsy();
});
});
describe('#hasScheme', (): void => {
it('returns true if the provided url matches the provided scheme.', (): void => {
expect(hasScheme('http://example.com', 'http')).toBeTruthy();
});
it('returns true if the provided url matches one of the provided schemes.', (): void => {
expect(hasScheme('ws://example.com', 'http', 'https', 'ws')).toBeTruthy();
});
it('returns false if the provided url does not match the provided scheme.', (): void => {
expect(hasScheme('http://example.com', 'https')).toBeFalsy();
});
it('returns false if the provided value is not a valid url.', (): void => {
expect(hasScheme('not-a-URL:test', 'http')).toBeFalsy();
});
it('is case insensitive: schemes with different case, result in a correct match.', (): void => {
expect(hasScheme('wss://example.com', 'http', 'WSS')).toBeTruthy();
});
});
});

View File

@ -0,0 +1,34 @@
import {
sanitizeUrlPart,
splitCommaSeparated,
isValidFileName,
} from '../../../src/util/StringUtil';
describe('HeaderUtil', (): void => {
describe('#sanitizeUrlPart', (): void => {
it('sanitizes part of a URL by replacing non-word characters with dashes (\'-\').', (): void => {
expect(sanitizeUrlPart('$path segment containing=non-word+chars'))
.toBe('-path-segment-containing-non-word-chars');
});
});
describe('#splitCommaSeparated', (): void => {
it('splits strings containing commas into parts based on the location of these commas.', (): void => {
expect(splitCommaSeparated('this,is,a,comma-separated,string'))
.toEqual([ 'this', 'is', 'a', 'comma-separated', 'string' ]);
});
it('handles strings without commas by returning an array containing solely the original string.', (): void => {
const strVal = 'this string has no commas';
expect(splitCommaSeparated(strVal)).toEqual([ strVal ]);
});
});
describe('#validateFileName', (): void => {
it('returns true if the provided file name is valid.', (): void => {
expect(isValidFileName('valid-file.test')).toBeTruthy();
});
it('returns false if the provided file name is invalid.', (): void => {
expect(isValidFileName('$%^*')).toBeFalsy();
});
});
});