From dbdb9b424e4c4f68c19c66396064486bff93a7e4 Mon Sep 17 00:00:00 2001 From: Wannes Kerckhove Date: Thu, 14 Apr 2022 10:19:13 +0200 Subject: [PATCH 1/3] fix: %2F not handled correctly in file backend #1184 Fix --- src/util/PathUtil.ts | 64 +++++++- .../FileBackendEncodedSlashHandling.test.ts | 149 ++++++++++++++++++ test/integration/config/server-file.json | 54 +++++++ test/unit/util/PathUtil.test.ts | 10 ++ test/util/Util.ts | 1 + 5 files changed, 274 insertions(+), 4 deletions(-) create mode 100644 test/integration/FileBackendEncodedSlashHandling.test.ts create mode 100644 test/integration/config/server-file.json diff --git a/src/util/PathUtil.ts b/src/util/PathUtil.ts index aa2f8a830..46541fa8f 100644 --- a/src/util/PathUtil.ts +++ b/src/util/PathUtil.ts @@ -5,6 +5,45 @@ import type { ResourceIdentifier } from '../http/representation/ResourceIdentifi import type { HttpRequest } from '../server/HttpRequest'; import { BadRequestHttpError } from './errors/BadRequestHttpError'; +// Characters to ignore when URL decoding the URI path to a file path. +const pathComponentDelimiters = [ '/', '\\' ]; +// Regex to find all instances of encoded path component delimiters. +const encodedDelimiterRegex = new RegExp( + `${pathComponentDelimiters.map((delimiter): string => encodeURIComponent(delimiter)).join('|')}`, 'giu', +); +// Mapping of the replacements to perform in the preventDelimiterDecoding helper function. +const preventDelimiterDecodingMap = Object.fromEntries(pathComponentDelimiters.map((delimiter): [string, string] => { + const encodedDelimiter = encodeURIComponent(delimiter); + return [ encodedDelimiter, encodeURIComponent(encodedDelimiter) ]; +})); +// Mapping of the replacements to perform in the preventDelimiterEncoding helper function. +const preventDelimiterEncodingMap = Object.fromEntries(pathComponentDelimiters.map((delimiter): [string, string] => { + const encodedDelimiter = encodeURIComponent(delimiter); + return [ encodedDelimiter, delimiter ]; +})); + +/** + * Prevents some characters from being URL decoded by escaping them. + * The characters to 'escape' are declared in codecExceptions. + * + * @param pathComponent - The path component to apply the escaping on. + * @returns A copy of the input path that is safe to apply URL decoding on. + */ +function preventDelimiterDecoding(pathComponent: string): string { + return pathComponent.replace(encodedDelimiterRegex, (delimiter): string => preventDelimiterDecodingMap[delimiter]); +} + +/** + * Prevents some characters from being URL encoded by escaping them. + * The characters to 'escape' are declared in codecExceptions. + * + * @param pathComponent - The path component to apply the escaping on. + * @returns A copy of the input path that is safe to apply URL encoding on. + */ +function preventDelimiterEncoding(pathComponent: string): string { + return pathComponent.replace(encodedDelimiterRegex, (delimiter): string => preventDelimiterEncodingMap[delimiter]); +} + /** * Changes a potential Windows path into a POSIX path. * @@ -118,6 +157,11 @@ function transformPathComponents(path: string, transform: (part: string) => stri /** * Converts a URI path to the canonical version by splitting on slashes, * decoding any percent-based encodings, and then encoding any special characters. + * This function is used to clean unwanted characters in the components of + * the provided path. + * + * @param path - The path to convert to its canonical URI path form. + * @returns The canonical URI path form of the provided path. */ export function toCanonicalUriPath(path: string): string { return transformPathComponents(path, (part): string => @@ -125,17 +169,29 @@ export function toCanonicalUriPath(path: string): string { } /** - * Decodes all components of a URI path. + * This function is used when converting a URI to a file path. Decodes all components of a URI path, + * with the exception of encoded slash characters, as this would lead to unexpected file locations + * being targeted (resulting in erroneous behaviour of the file based backend). + * + * @param path - The path to decode the URI path components of. + * @returns A decoded copy of the provided URI path (ignoring encoded slash characters). */ export function decodeUriPathComponents(path: string): string { - return transformPathComponents(path, decodeURIComponent); + return transformPathComponents(path, (part): string => + decodeURIComponent(preventDelimiterDecoding(part))); } /** - * Encodes all (non-slash) special characters in a URI path. + * This function is used in the process of converting a file path to a URI. Encodes all (non-slash) + * special characters in a URI path, with the exception of encoded slash characters, as this would + * lead to unnecessary double encoding, resulting in a URI that differs from the expected result. + * + * @param path - The path to encode the URI path components of. + * @returns An encoded copy of the provided URI path (ignoring encoded slash characters). */ export function encodeUriPathComponents(path: string): string { - return transformPathComponents(path, encodeURIComponent); + return transformPathComponents(path, (part): string => + encodeURIComponent(preventDelimiterEncoding(part))); } /** diff --git a/test/integration/FileBackendEncodedSlashHandling.test.ts b/test/integration/FileBackendEncodedSlashHandling.test.ts new file mode 100644 index 000000000..a4e6293ca --- /dev/null +++ b/test/integration/FileBackendEncodedSlashHandling.test.ts @@ -0,0 +1,149 @@ +import fetch from 'cross-fetch'; +import { pathExists } from 'fs-extra'; +import type { App } from '../../src/init/App'; +import { getPort } from '../util/Util'; +import { + getDefaultVariables, + getTestConfigPath, + getTestFolder, + instantiateFromConfig, + removeFolder, +} from './Config'; + +const port = getPort('FileBackendEncodedSlashHandling'); +const baseUrl = `http://localhost:${port}/`; + +const rootFilePath = getTestFolder('file-backend-encoded-slash-handling'); + +describe('A server with a file backend storage', (): void => { + let app: App; + + beforeAll(async(): Promise => { + await removeFolder(rootFilePath); + const variables = { + ...getDefaultVariables(port, baseUrl), + 'urn:solid-server:default:variable:rootFilePath': rootFilePath, + }; + + // Create and start the server + const instances = await instantiateFromConfig( + 'urn:solid-server:test:Instances', + [ + getTestConfigPath('server-file.json'), + ], + variables, + ) as Record; + ({ app } = instances); + + await app.start(); + }); + + afterAll(async(): Promise => { + // Await removeFolder(rootFilePath); + await app.stop(); + }); + + it('can put a document for which the URI path contains url encoded separator characters.', async(): Promise => { + const url = `${baseUrl}/c1/c2/t1%2F`; + const res = await fetch(url, { + method: 'PUT', + headers: { + 'content-type': 'text/plain', + }, + body: 'abc', + }); + expect(res.status).toBe(201); + expect(res.headers.get('location')).toBe(url); + + // The resource should not be accessible through ${baseUrl}/c1/c2/t1/. + const check1 = await fetch(`${baseUrl}/c1/c2/t1/}`, { + method: 'GET', + headers: { + accept: 'text/plain', + }, + }); + expect(check1.status).toBe(404); + + // Check that the created resource is not a container + const check2 = await fetch(url, { + method: 'GET', + headers: { + accept: 'text/plain', + }, + }); + const linkHeaderValues = check2.headers.get('link')!.split(',').map((item): string => item.trim()); + expect(linkHeaderValues).not.toContain('; rel="type"'); + + // Check that the appropriate file path exists + const check3 = await pathExists(`${rootFilePath}/c1/c2/t1%2F$.txt`); + expect(check3).toBe(true); + }); + + it('can post a document using a slug that contains url encoded separator characters.', async(): Promise => { + const slug = 't1%2Faa'; + const res = await fetch(baseUrl, { + method: 'POST', + headers: { + 'content-type': 'text/plain', + slug, + }, + body: 'abc', + }); + expect(res.status).toBe(201); + expect(res.headers.get('location')).toBe(`${baseUrl}${slug}`); + + // Check that the the appropriate file path exists + const check = await pathExists(`${rootFilePath}/${slug}$.txt`); + expect(check).toBe(true); + }); + + it('prevents accessing a document via a different identifier that results in the same path after url decoding.', + async(): Promise => { + // First put a resource using a path without encoded separator characters: foo/bar + const url = `${baseUrl}/foo/bar`; + await fetch(url, { + method: 'PUT', + headers: { + 'content-type': 'text/plain', + }, + body: 'abc', + }); + + // The resource at foo/bar should not be accessible using the url encoded variant of this path: foo%2Fbar + const check1 = await fetch(`${baseUrl}/foo%2Fbar`, { + method: 'GET', + headers: { + accept: 'text/plain', + }, + }); + // Expect foo%2Fbar to correctly refer to a different document, which does not exist. + expect(check1.status).toBe(404); + + // Check that the the appropriate file path for foo/bar exists + const check2 = await pathExists(`${rootFilePath}/foo/bar$.txt`); + expect(check2).toBe(true); + + // Next, put a resource using a path with an encoded separator character: bar%2Ffoo + await fetch(`${baseUrl}/bar%2Ffoo`, { + method: 'PUT', + headers: { + 'content-type': 'text/plain', + }, + body: 'abc', + }); + + // The resource at bar%2Ffoo should not be accessible through bar/foo + const check3 = await fetch(`${baseUrl}/bar/foo`, { + method: 'GET', + headers: { + accept: 'text/plain', + }, + }); + // Expect bar/foo to correctly refer to a different document, which does not exist. + expect(check3.status).toBe(404); + + // Check that the the appropriate file path for bar%foo exists + const check4 = await pathExists(`${rootFilePath}/bar%2Ffoo$.txt`); + expect(check4).toBe(true); + }); +}); diff --git a/test/integration/config/server-file.json b/test/integration/config/server-file.json new file mode 100644 index 000000000..9b5fe98aa --- /dev/null +++ b/test/integration/config/server-file.json @@ -0,0 +1,54 @@ +{ + "@context": "https://linkedsoftwaredependencies.org/bundles/npm/@solid/community-server/^4.0.0/components/context.jsonld", + "import": [ + "files-scs:config/app/main/default.json", + "files-scs:config/app/init/initialize-root.json", + "files-scs:config/app/setup/disabled.json", + "files-scs:config/http/handler/default.json", + "files-scs:config/http/middleware/websockets.json", + "files-scs:config/http/server-factory/websockets.json", + "files-scs:config/http/static/default.json", + "files-scs:config/identity/access/public.json", + "files-scs:config/identity/handler/default.json", + "files-scs:config/identity/ownership/token.json", + "files-scs:config/identity/pod/static.json", + "files-scs:config/identity/registration/enabled.json", + "files-scs:config/ldp/authentication/dpop-bearer.json", + "files-scs:config/ldp/authorization/webacl.json", + "files-scs:config/ldp/handler/default.json", + "files-scs:config/ldp/metadata-parser/default.json", + "files-scs:config/ldp/metadata-writer/default.json", + "files-scs:config/ldp/modes/default.json", + "files-scs:config/storage/backend/file.json", + "files-scs:config/storage/key-value/resource-store.json", + "files-scs:config/storage/middleware/default.json", + "files-scs:config/util/auxiliary/acl.json", + "files-scs:config/util/identifiers/suffix.json", + "files-scs:config/util/index/default.json", + "files-scs:config/util/logging/winston.json", + "files-scs:config/util/representation-conversion/default.json", + "files-scs:config/util/resource-locker/memory.json", + "files-scs:config/util/variables/default.json" + ], + "@graph": [ + { + "@id": "urn:solid-server:test:Instances", + "@type": "RecordObject", + "RecordObject:_record": [ + { + "RecordObject:_record_key": "app", + "RecordObject:_record_value": { "@id": "urn:solid-server:default:App" } + } + ] + }, + { + "@id": "urn:solid-server:default:EmailSender", + "@type": "BaseEmailSender", + "args_senderName": "Solid Server", + "args_emailConfig_host": "smtp.example.email", + "args_emailConfig_port": 587, + "args_emailConfig_auth_user": "alice@example.email", + "args_emailConfig_auth_pass": "NYEaCsqV7aVStRCbmC" + } + ] +} diff --git a/test/unit/util/PathUtil.test.ts b/test/unit/util/PathUtil.test.ts index 0d5c49ee4..2ae158e58 100644 --- a/test/unit/util/PathUtil.test.ts +++ b/test/unit/util/PathUtil.test.ts @@ -112,6 +112,11 @@ describe('PathUtil', (): void => { it('leaves the query string untouched.', (): void => { expect(decodeUriPathComponents('/a%20path&/name?abc=def&xyz')).toBe('/a path&/name?abc=def&xyz'); }); + + it('ignores url encoded path separator characters.', (): void => { + expect(decodeUriPathComponents('/a%20path&/c1/c2/t1%2F')).toBe('/a path&/c1/c2/t1%2F'); + expect(decodeUriPathComponents('/a%20path&/c1/c2/t1%5C')).toBe('/a path&/c1/c2/t1%5C'); + }); }); describe('#encodeUriPathComponents', (): void => { @@ -122,6 +127,11 @@ describe('PathUtil', (): void => { it('leaves the query string untouched.', (): void => { expect(encodeUriPathComponents('/a%20path&/name?abc=def&xyz')).toBe('/a%2520path%26/name?abc=def&xyz'); }); + + it('does not double-encode url encoded path separator characters.', (): void => { + expect(encodeUriPathComponents('/a%20path&/c1/c2/t1%2F')).toBe('/a%2520path%26/c1/c2/t1%2F'); + expect(encodeUriPathComponents('/a%20path&/c1/c2/t1%5C')).toBe('/a%2520path%26/c1/c2/t1%5C'); + }); }); describe('#isContainerPath', (): void => { diff --git a/test/util/Util.ts b/test/util/Util.ts index e96f2c1af..c6563b3ef 100644 --- a/test/util/Util.ts +++ b/test/util/Util.ts @@ -8,6 +8,7 @@ const portNames = [ 'ContentNegotiation', 'DynamicPods', 'ExpiringDataCleanup', + 'FileBackendEncodedSlashHandling', 'GlobalQuota', 'Identity', 'LpdHandlerWithAuth', From 50469e2c1f3d9c808062fde96d2ce62d5e85475e Mon Sep 17 00:00:00 2001 From: Ruben Verborgh Date: Fri, 22 Apr 2022 00:16:49 +0200 Subject: [PATCH 2/3] fix: Make delimiter encoding case-insensitive. --- src/util/PathUtil.ts | 63 ++++++------------- .../FileBackendEncodedSlashHandling.test.ts | 26 ++++---- test/unit/util/PathUtil.test.ts | 46 +++++++++++++- 3 files changed, 75 insertions(+), 60 deletions(-) diff --git a/src/util/PathUtil.ts b/src/util/PathUtil.ts index 46541fa8f..b5745f351 100644 --- a/src/util/PathUtil.ts +++ b/src/util/PathUtil.ts @@ -5,45 +5,6 @@ import type { ResourceIdentifier } from '../http/representation/ResourceIdentifi import type { HttpRequest } from '../server/HttpRequest'; import { BadRequestHttpError } from './errors/BadRequestHttpError'; -// Characters to ignore when URL decoding the URI path to a file path. -const pathComponentDelimiters = [ '/', '\\' ]; -// Regex to find all instances of encoded path component delimiters. -const encodedDelimiterRegex = new RegExp( - `${pathComponentDelimiters.map((delimiter): string => encodeURIComponent(delimiter)).join('|')}`, 'giu', -); -// Mapping of the replacements to perform in the preventDelimiterDecoding helper function. -const preventDelimiterDecodingMap = Object.fromEntries(pathComponentDelimiters.map((delimiter): [string, string] => { - const encodedDelimiter = encodeURIComponent(delimiter); - return [ encodedDelimiter, encodeURIComponent(encodedDelimiter) ]; -})); -// Mapping of the replacements to perform in the preventDelimiterEncoding helper function. -const preventDelimiterEncodingMap = Object.fromEntries(pathComponentDelimiters.map((delimiter): [string, string] => { - const encodedDelimiter = encodeURIComponent(delimiter); - return [ encodedDelimiter, delimiter ]; -})); - -/** - * Prevents some characters from being URL decoded by escaping them. - * The characters to 'escape' are declared in codecExceptions. - * - * @param pathComponent - The path component to apply the escaping on. - * @returns A copy of the input path that is safe to apply URL decoding on. - */ -function preventDelimiterDecoding(pathComponent: string): string { - return pathComponent.replace(encodedDelimiterRegex, (delimiter): string => preventDelimiterDecodingMap[delimiter]); -} - -/** - * Prevents some characters from being URL encoded by escaping them. - * The characters to 'escape' are declared in codecExceptions. - * - * @param pathComponent - The path component to apply the escaping on. - * @returns A copy of the input path that is safe to apply URL encoding on. - */ -function preventDelimiterEncoding(pathComponent: string): string { - return pathComponent.replace(encodedDelimiterRegex, (delimiter): string => preventDelimiterEncodingMap[delimiter]); -} - /** * Changes a potential Windows path into a POSIX path. * @@ -146,11 +107,25 @@ export function getExtension(path: string): string { } /** - * Performs a transformation on the path components of a URI. + * Performs a transformation on the path components of a URI, + * preserving but normalizing path delimiters and their escaped forms. */ function transformPathComponents(path: string, transform: (part: string) => string): string { const [ , base, queryString ] = /^([^?]*)(.*)$/u.exec(path)!; - const transformed = base.split('/').map((element): string => transform(element)).join('/'); + const transformed = base + // We split on actual URI path component delimiters (slash and backslash), + // but also on things that could be wrongly interpreted as component delimiters, + // such that they cannot be transformed incorrectly. + // We thus ensure that encoded slashes (%2F) and backslashes (%5C) are preserved, + // since they would become _actual_ delimiters if accidentally decoded. + // Additionally, we need to preserve any encoded percent signs (%25) + // that precede them, because these might change their interpretation as well. + .split(/(\/|\\|%(?:25)*(?:2f|5c))/ui) + // Even parts map to components that need to be transformed, + // odd parts to (possibly escaped) delimiters that need to be normalized. + .map((part, index): string => + index % 2 === 0 ? transform(part) : part.toUpperCase()) + .join(''); return !queryString ? transformed : `${transformed}${queryString}`; } @@ -177,8 +152,7 @@ export function toCanonicalUriPath(path: string): string { * @returns A decoded copy of the provided URI path (ignoring encoded slash characters). */ export function decodeUriPathComponents(path: string): string { - return transformPathComponents(path, (part): string => - decodeURIComponent(preventDelimiterDecoding(part))); + return transformPathComponents(path, decodeURIComponent); } /** @@ -190,8 +164,7 @@ export function decodeUriPathComponents(path: string): string { * @returns An encoded copy of the provided URI path (ignoring encoded slash characters). */ export function encodeUriPathComponents(path: string): string { - return transformPathComponents(path, (part): string => - encodeURIComponent(preventDelimiterEncoding(part))); + return transformPathComponents(path, encodeURIComponent); } /** diff --git a/test/integration/FileBackendEncodedSlashHandling.test.ts b/test/integration/FileBackendEncodedSlashHandling.test.ts index a4e6293ca..ad09a3992 100644 --- a/test/integration/FileBackendEncodedSlashHandling.test.ts +++ b/test/integration/FileBackendEncodedSlashHandling.test.ts @@ -39,12 +39,12 @@ describe('A server with a file backend storage', (): void => { }); afterAll(async(): Promise => { - // Await removeFolder(rootFilePath); + await removeFolder(rootFilePath); await app.stop(); }); - it('can put a document for which the URI path contains url encoded separator characters.', async(): Promise => { - const url = `${baseUrl}/c1/c2/t1%2F`; + it('can put a document for which the URI path contains URL-encoded separator characters.', async(): Promise => { + const url = `${baseUrl}c1/c2/t1%2f`; const res = await fetch(url, { method: 'PUT', headers: { @@ -53,10 +53,10 @@ describe('A server with a file backend storage', (): void => { body: 'abc', }); expect(res.status).toBe(201); - expect(res.headers.get('location')).toBe(url); + expect(res.headers.get('location')).toBe(`${baseUrl}c1/c2/t1%2F`); - // The resource should not be accessible through ${baseUrl}/c1/c2/t1/. - const check1 = await fetch(`${baseUrl}/c1/c2/t1/}`, { + // The resource should not be accessible through ${baseUrl}c1/c2/t1/. + const check1 = await fetch(`${baseUrl}c1/c2/t1/}`, { method: 'GET', headers: { accept: 'text/plain', @@ -79,7 +79,7 @@ describe('A server with a file backend storage', (): void => { expect(check3).toBe(true); }); - it('can post a document using a slug that contains url encoded separator characters.', async(): Promise => { + it('can post a document using a slug that contains URL-encoded separator characters.', async(): Promise => { const slug = 't1%2Faa'; const res = await fetch(baseUrl, { method: 'POST', @@ -92,15 +92,15 @@ describe('A server with a file backend storage', (): void => { expect(res.status).toBe(201); expect(res.headers.get('location')).toBe(`${baseUrl}${slug}`); - // Check that the the appropriate file path exists + // Check that the appropriate file path exists const check = await pathExists(`${rootFilePath}/${slug}$.txt`); expect(check).toBe(true); }); - it('prevents accessing a document via a different identifier that results in the same path after url decoding.', + it('prevents accessing a document via a different identifier that results in the same path after URL decoding.', async(): Promise => { // First put a resource using a path without encoded separator characters: foo/bar - const url = `${baseUrl}/foo/bar`; + const url = `${baseUrl}foo/bar`; await fetch(url, { method: 'PUT', headers: { @@ -110,7 +110,7 @@ describe('A server with a file backend storage', (): void => { }); // The resource at foo/bar should not be accessible using the url encoded variant of this path: foo%2Fbar - const check1 = await fetch(`${baseUrl}/foo%2Fbar`, { + const check1 = await fetch(`${baseUrl}foo%2Fbar`, { method: 'GET', headers: { accept: 'text/plain', @@ -124,7 +124,7 @@ describe('A server with a file backend storage', (): void => { expect(check2).toBe(true); // Next, put a resource using a path with an encoded separator character: bar%2Ffoo - await fetch(`${baseUrl}/bar%2Ffoo`, { + await fetch(`${baseUrl}bar%2Ffoo`, { method: 'PUT', headers: { 'content-type': 'text/plain', @@ -133,7 +133,7 @@ describe('A server with a file backend storage', (): void => { }); // The resource at bar%2Ffoo should not be accessible through bar/foo - const check3 = await fetch(`${baseUrl}/bar/foo`, { + const check3 = await fetch(`${baseUrl}bar/foo`, { method: 'GET', headers: { accept: 'text/plain', diff --git a/test/unit/util/PathUtil.test.ts b/test/unit/util/PathUtil.test.ts index 2ae158e58..4eb681a26 100644 --- a/test/unit/util/PathUtil.test.ts +++ b/test/unit/util/PathUtil.test.ts @@ -113,9 +113,30 @@ describe('PathUtil', (): void => { expect(decodeUriPathComponents('/a%20path&/name?abc=def&xyz')).toBe('/a path&/name?abc=def&xyz'); }); - it('ignores url encoded path separator characters.', (): void => { + it('ignores URL-encoded path separator characters.', (): void => { expect(decodeUriPathComponents('/a%20path&/c1/c2/t1%2F')).toBe('/a path&/c1/c2/t1%2F'); expect(decodeUriPathComponents('/a%20path&/c1/c2/t1%5C')).toBe('/a path&/c1/c2/t1%5C'); + expect(decodeUriPathComponents('/a%20path&/c1/c2/t1%252F')).toBe('/a path&/c1/c2/t1%252F'); + expect(decodeUriPathComponents('/a%20path&/c1/c2/t1%255C')).toBe('/a path&/c1/c2/t1%255C'); + expect(decodeUriPathComponents('/a%20path&/c1/c2/t1%25%252F')).toBe('/a path&/c1/c2/t1%%252F'); + expect(decodeUriPathComponents('/a%20path&/c1/c2/t1%25%255C')).toBe('/a path&/c1/c2/t1%%255C'); + }); + + it('normalizes to uppercase encoding.', (): void => { + expect(decodeUriPathComponents('/a%20path&/c1/c2/t1%2f')).toBe('/a path&/c1/c2/t1%2F'); + expect(decodeUriPathComponents('/a%20path&/c1/c2/t1%5c')).toBe('/a path&/c1/c2/t1%5C'); + }); + + it('accepts paths with mixed lowercase and uppercase encoding.', (): void => { + expect(decodeUriPathComponents('/a%20path&/c1/c2/t1%2F%2f')).toBe('/a path&/c1/c2/t1%2F%2F'); + expect(decodeUriPathComponents('/a%20path&/c1/c2/t1%5C%5c')).toBe('/a path&/c1/c2/t1%5C%5C'); + }); + + it('takes sequences of encoded percent signs into account.', (): void => { + expect(decodeUriPathComponents('/a%2Fb')).toBe('/a%2Fb'); + expect(decodeUriPathComponents('/a%252Fb')).toBe('/a%252Fb'); + expect(decodeUriPathComponents('/a%25252Fb')).toBe('/a%25252Fb'); + expect(decodeUriPathComponents('/a%2525252Fb')).toBe('/a%2525252Fb'); }); }); @@ -128,9 +149,30 @@ describe('PathUtil', (): void => { expect(encodeUriPathComponents('/a%20path&/name?abc=def&xyz')).toBe('/a%2520path%26/name?abc=def&xyz'); }); - it('does not double-encode url encoded path separator characters.', (): void => { + it('does not double-encode URL-encoded path separator characters.', (): void => { expect(encodeUriPathComponents('/a%20path&/c1/c2/t1%2F')).toBe('/a%2520path%26/c1/c2/t1%2F'); expect(encodeUriPathComponents('/a%20path&/c1/c2/t1%5C')).toBe('/a%2520path%26/c1/c2/t1%5C'); + expect(encodeUriPathComponents('/a%20path&/c1/c2/t1%252F')).toBe('/a%2520path%26/c1/c2/t1%252F'); + expect(encodeUriPathComponents('/a%20path&/c1/c2/t1%255C')).toBe('/a%2520path%26/c1/c2/t1%255C'); + expect(encodeUriPathComponents('/a%20path&/c1/c2/t1%%252F')).toBe('/a%2520path%26/c1/c2/t1%25%252F'); + expect(encodeUriPathComponents('/a%20path&/c1/c2/t1%%255C')).toBe('/a%2520path%26/c1/c2/t1%25%255C'); + }); + + it('normalizes to uppercase encoding.', (): void => { + expect(encodeUriPathComponents('/a%20path&/c1/c2/t1%2f')).toBe('/a%2520path%26/c1/c2/t1%2F'); + expect(encodeUriPathComponents('/a%20path&/c1/c2/t1%5c')).toBe('/a%2520path%26/c1/c2/t1%5C'); + }); + + it('accepts paths with mixed lowercase and uppercase encoding.', (): void => { + expect(encodeUriPathComponents('/a%20path&/c1/c2/t1%2F%2f')).toBe('/a%2520path%26/c1/c2/t1%2F%2F'); + expect(encodeUriPathComponents('/a%20path&/c1/c2/t1%5C%5c')).toBe('/a%2520path%26/c1/c2/t1%5C%5C'); + }); + + it('takes sequences of encoded percent signs into account.', (): void => { + expect(encodeUriPathComponents('/a%2Fb')).toBe('/a%2Fb'); + expect(encodeUriPathComponents('/a%252Fb')).toBe('/a%252Fb'); + expect(encodeUriPathComponents('/a%25252Fb')).toBe('/a%25252Fb'); + expect(encodeUriPathComponents('/a%2525252Fb')).toBe('/a%2525252Fb'); }); }); From 6dd77cf8d83fedc7f8c9861c34ea1fad317df3ca Mon Sep 17 00:00:00 2001 From: Jasper Vaneessen Date: Mon, 25 Apr 2022 15:44:19 +0200 Subject: [PATCH 3/3] test: Validate/test all default configs * chore: add base script to test-run all configs * chore: job for test-deploy * chore: fine-tuning config validation * chore: config validation fully functional * chore: fix https-file-cli validation (missing var) * fix: generate self-signed CA through openSSL * chore: streamlining test script + review * chore: validate-configs accepts config args * chore: cleanup and best-practices * chore: test-configs as precond + needs consistency * chore: changes after review * chore: fix argument variable expansion * chore: more tweaks to script --- .github/workflows/ci.yml | 27 ++++++- package.json | 2 +- test/deploy/validate-configs.sh | 120 ++++++++++++++++++++++++++++++++ test/deploy/validate-package.sh | 37 ---------- 4 files changed, 146 insertions(+), 40 deletions(-) create mode 100755 test/deploy/validate-configs.sh delete mode 100755 test/deploy/validate-package.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 74d81fb80..596f32867 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,8 +58,6 @@ jobs: github-token: ${{ secrets.github_token }} flag-name: test-unit-${{ matrix.node-version }}-${{ matrix.operating-system }} parallel: true - - name: Run deployment tests - run: npm run test:deploy test-integration: runs-on: ubuntu-latest @@ -118,6 +116,27 @@ jobs: - name: Run integration tests run: npm run test:integration + test-configs: + runs-on: ubuntu-latest + services: + sparql-endpoint: + image: tenforce/virtuoso + env: + SPARQL_UPDATE: true + ports: + - 4000:8890 + steps: + - name: Use Node.js + uses: actions/setup-node@v3 + with: + node-version: '16.x' + - name: Check out repository + uses: actions/checkout@v3 + - name: Install dependencies and run build scripts + run: npm ci + - name: Run deploy tests + run: npm run test:deploy + coveralls: needs: test-unit runs-on: ubuntu-latest @@ -134,6 +153,7 @@ jobs: - test-unit - test-integration - test-integration-windows + - test-configs # Only run on tag push events starting with v prefix for now OR main branch push events if: startsWith(github.ref, 'refs/tags/v') || (github.ref == 'refs/heads/main') runs-on: ubuntu-latest @@ -175,6 +195,7 @@ jobs: - test-unit - test-integration - test-integration-windows + - test-configs # Only run on push events on a versions/* branch (ASSUMPTION: THERE SHOULD ONLY BE ONE THERE!) if: startsWith(github.ref, 'refs/heads/versions/') runs-on: ubuntu-latest @@ -231,6 +252,8 @@ jobs: - lint - test-unit - test-integration + - test-integration-windows + - test-configs runs-on: ubuntu-latest if: startsWith(github.ref, 'refs/tags/v') steps: diff --git a/package.json b/package.json index 6ac4c416b..f0bea609f 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,7 @@ "start": "node ./bin/server.js", "start:file": "node ./bin/server.js -c config/file.json -f ./data", "test": "npm run test:ts && npm run jest", - "test:deploy": "test/deploy/validate-package.sh", + "test:deploy": "test/deploy/validate-configs.sh", "test:ts": "tsc -p test --noEmit", "test:integration": "jest --coverageReporters text-summary -- test/integration", "test:unit": "jest --config=./jest.coverage.config.js test/unit", diff --git a/test/deploy/validate-configs.sh b/test/deploy/validate-configs.sh new file mode 100755 index 000000000..9364e8b73 --- /dev/null +++ b/test/deploy/validate-configs.sh @@ -0,0 +1,120 @@ +#!/usr/bin/env bash +# Script to validate the packaged module and configs + +# Ensure our workdir is that of the project root +cd "${0%/*}/../.." || { echo "Error setting workdir to project directory."; exit 1; } + +# This script takes config paths (from project directory) as optional input +# No arguments: all default configs are tested +# One ore more arguments: provided configs are tested +# Example: validate-configs.sh config/default.json config/file.json +TEST_NAME="Deployment testing" +declare -a CONFIG_ARRAY +if [[ $# -gt 0 ]]; then + for CONFIG in "$@"; do + if [ ! -f "$CONFIG" ]; then + echo "Config file $CONFIG does not exist, check the path (example: config/default.json)" + exit 1 + fi + CONFIG_ARRAY+=("$CONFIG") + done + echo "Deployment testing ${#CONFIG_ARRAY[@]} configs:" +else + mapfile -t CONFIG_ARRAY < <(ls config/*.json) + echo "Deployment testing all configs:" +fi +printf " - %s\n" "${CONFIG_ARRAY[@]}" + +mkdir -p test/tmp/data +echo "$TEST_NAME - Building and installing package" +npm pack --loglevel warn +npm install -g solid-community-server-*.tgz --loglevel warn +rm solid-community-server-*.tgz + +run_server_with_config () { + if [[ $# -ne 2 ]]; then + echo "Config arguments not set" + return 1 + elif [ ! -f "$1" ]; then + echo "Config file does not exist, check the path." + return 1 + fi + CONFIG_PATH=$1 + CONFIG_NAME=$2 + + mkdir -p test/tmp/data + + CSS_ARGS=("-p" "8888" "-l" "warn" "-f" "test/tmp/data/" "-s" "http://localhost:4000/sparql") + CSS_BASE_URL="http://localhost:8888" + + # HTTPS config specifics: self-signed key/cert + CSS base URL override + if [[ $CONFIG_NAME =~ "https" ]]; then + openssl req -x509 -nodes -days 1 -newkey rsa:2048 -keyout test/tmp/server.key -out test/tmp/server.cert -subj '/CN=localhost' &>/dev/null + CSS_BASE_URL="https://localhost:8888" + if [[ $CONFIG_NAME =~ "https-file-cli" ]]; then + CSS_ARGS+=("--httpsKey" "test/tmp/server.key" "--httpsCert" "test/tmp/server.cert") + elif [[ $CONFIG_NAME =~ "example-https" ]]; then + CONFIG_PATH=test/tmp/example-https-file.json + cp config/example-https-file.json $CONFIG_PATH + sed -i -E "s/(\W+\"options_key\".*\").+(\".*)/\1test\/tmp\/server.key\2/" $CONFIG_PATH + sed -i -E "s/(\W+\"options_cert\".*\").+(\".*)/\1test\/tmp\/server.cert\2/" $CONFIG_PATH + fi + fi + + echo -e "----------------------------------" + echo "$TEST_NAME($CONFIG_NAME) - Starting the server" + community-solid-server "${CSS_ARGS[@]}" -b $CSS_BASE_URL -c $CONFIG_PATH &>test/tmp/"$CONFIG_NAME" & + PID=$! + + FAILURE=1 + if [ -z $PID ]; then + echo "$TEST_NAME($CONFIG_NAME) - FAILURE: Server did not start" + cat test/tmp/"$CONFIG_NAME" + else + echo "$TEST_NAME($CONFIG_NAME) - Attempting HTTP access to the server" + if curl -sfkI -X GET --retry 15 --retry-connrefused --retry-delay 1 $CSS_BASE_URL > test/tmp/"$CONFIG_NAME"-curl; then + echo "$TEST_NAME($CONFIG_NAME) - SUCCESS: server reached" + FAILURE=0 + else + echo "$TEST_NAME($CONFIG_NAME) - FAILURE: Could not reach server" + fi + kill -9 $PID &> /dev/null + timeout 30s tail --pid=$PID -f /dev/null + fi + + rm -rf test/tmp/data/* + return $FAILURE +} + +VALIDATION_FAILURE=0 +SUCCESSES='' +FAILURES='' +for CONFIG_PATH in "${CONFIG_ARRAY[@]}"; do + CONFIG_NAME=$(echo "$CONFIG_PATH" | sed -E 's/.+\/(.+)\.json/\1/') + + run_server_with_config "$CONFIG_PATH" "$CONFIG_NAME" + SERVER_FAILURE=$? + if [ $SERVER_FAILURE -eq 0 ]; then + SUCCESSES="${SUCCESSES}[SUCCESS]\t$CONFIG_NAME\t($CONFIG_PATH)\n" + else + echo "$TEST_NAME($CONFIG_NAME) - CSS logs: "; + cat test/tmp/"$CONFIG_NAME" + echo "$TEST_NAME($CONFIG_NAME) - curl logs: "; + cat test/tmp/"$CONFIG_NAME"-curl + VALIDATION_FAILURE=1 + FAILURES="${FAILURES}[FAILURE]\t$CONFIG_NAME\t($CONFIG_PATH)\n" + fi + echo "" +done; + +echo "$TEST_NAME - Cleanup" +npm uninstall -g @solid/community-server +rm -rf test/tmp/* + + +echo -e "\n\n----------------------------------------" +echo "Config validation overview" +echo "----------------------------------------" +echo -e "$SUCCESSES" +echo -e "$FAILURES" +exit $VALIDATION_FAILURE diff --git a/test/deploy/validate-package.sh b/test/deploy/validate-package.sh deleted file mode 100755 index bccf345a0..000000000 --- a/test/deploy/validate-package.sh +++ /dev/null @@ -1,37 +0,0 @@ -#!/usr/bin/env bash -# Script to validate the packaged module - -TEST_NAME="Deployment test: packaged module" - -echo "$TEST_NAME - Building and installing package" -npm pack --loglevel warn -npm install -g solid-community-server-*.tgz --loglevel warn -rm solid-community-server-*.tgz - -echo "$TEST_NAME - Starting the server" -community-solid-server -p 8888 -l warn & -PID=$! - -FAILURE=1 -if [ -z $PID ]; then - echo "$TEST_NAME - Failure: Server did not start" -else - echo "$TEST_NAME - Attempting HTTP access to the server" - for i in {1..10}; do - sleep 1 - if curl -s -f localhost:8888 > /dev/null; then - echo "$TEST_NAME - Success: server reached" - FAILURE=0 - break - fi - done - if [ $FAILURE -eq 1 ]; then - echo "$TEST_NAME - Failure: Could not reach server" - fi - kill -9 $PID -fi - -echo "$TEST_NAME - Cleanup" -npm uninstall -g @solid/community-server - -exit $FAILURE