feat(server): lint for trailing slashes in sync URL and extra slashes… (#2345)

This commit is contained in:
Elian Doran 2025-06-20 10:55:46 +03:00 committed by GitHub
commit a861defbee
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 151 additions and 5 deletions

View File

@ -5,7 +5,7 @@ import cls from "../services/cls.js";
import sql from "../services/sql.js"; import sql from "../services/sql.js";
import becca from "../becca/becca.js"; import becca from "../becca/becca.js";
import type { Request, Response, Router } from "express"; import type { Request, Response, Router } from "express";
import { safeExtractMessageAndStackFromError } from "../services/utils.js"; import { safeExtractMessageAndStackFromError, normalizeCustomHandlerPattern } from "../services/utils.js";
function handleRequest(req: Request, res: Response) { function handleRequest(req: Request, res: Response) {
@ -38,11 +38,19 @@ function handleRequest(req: Request, res: Response) {
continue; continue;
} }
const regex = new RegExp(`^${attr.value}$`); // Get normalized patterns to handle both trailing slash cases
let match; const patterns = normalizeCustomHandlerPattern(attr.value);
let match: RegExpMatchArray | null = null;
try { try {
match = path.match(regex); // Try each pattern until we find a match
for (const pattern of patterns) {
const regex = new RegExp(`^${pattern}$`);
match = path.match(regex);
if (match) {
break; // Found a match, exit pattern loop
}
}
} catch (e: unknown) { } catch (e: unknown) {
const [errMessage, errStack] = safeExtractMessageAndStackFromError(e); const [errMessage, errStack] = safeExtractMessageAndStackFromError(e);
log.error(`Testing path for label '${attr.attributeId}', regex '${attr.value}' failed with error: ${errMessage}, stack: ${errStack}`); log.error(`Testing path for label '${attr.attributeId}', regex '${attr.value}' failed with error: ${errMessage}, stack: ${errStack}`);

View File

@ -2,6 +2,7 @@
import optionService from "./options.js"; import optionService from "./options.js";
import config from "./config.js"; import config from "./config.js";
import { normalizeUrl } from "./utils.js";
/* /*
* Primary configuration for sync is in the options (document), but we allow to override * Primary configuration for sync is in the options (document), but we allow to override
@ -17,7 +18,10 @@ function get(name: keyof typeof config.Sync) {
export default { export default {
// env variable is the easiest way to guarantee we won't overwrite prod data during development // env variable is the easiest way to guarantee we won't overwrite prod data during development
// after copying prod document/data directory // after copying prod document/data directory
getSyncServerHost: () => get("syncServerHost"), getSyncServerHost: () => {
const host = get("syncServerHost");
return host ? normalizeUrl(host) : host;
},
isSyncSetup: () => { isSyncSetup: () => {
const syncServerHost = get("syncServerHost"); const syncServerHost = get("syncServerHost");

View File

@ -628,3 +628,56 @@ describe("#formatDownloadTitle", () => {
}); });
}); });
}); });
describe("#normalizeUrl", () => {
const testCases: TestCase<typeof utils.normalizeUrl>[] = [
[ "should remove trailing slash from simple URL", [ "https://example.com/" ], "https://example.com" ],
[ "should remove trailing slash from URL with path", [ "https://example.com/path/" ], "https://example.com/path" ],
[ "should preserve URL without trailing slash", [ "https://example.com" ], "https://example.com" ],
[ "should preserve URL without trailing slash with path", [ "https://example.com/path" ], "https://example.com/path" ],
[ "should preserve protocol-only URLs", [ "https://" ], "https://" ],
[ "should preserve protocol-only URLs", [ "http://" ], "http://" ],
[ "should fix double slashes in path", [ "https://example.com//api//test" ], "https://example.com/api/test" ],
[ "should handle multiple double slashes", [ "https://example.com///api///test" ], "https://example.com/api/test" ],
[ "should handle trailing slash with double slashes", [ "https://example.com//api//" ], "https://example.com/api" ],
[ "should preserve protocol double slash", [ "https://example.com/api" ], "https://example.com/api" ],
[ "should handle empty string", [ "" ], "" ],
[ "should handle whitespace-only string", [ " " ], "" ],
[ "should trim whitespace", [ " https://example.com/ " ], "https://example.com" ],
[ "should handle null as empty", [ null as any ], null ],
[ "should handle undefined as empty", [ undefined as any ], undefined ]
];
testCases.forEach((testCase) => {
const [ desc, fnParams, expected ] = testCase;
it(desc, () => {
const result = utils.normalizeUrl(...fnParams);
expect(result).toStrictEqual(expected);
});
});
});
describe("#normalizeCustomHandlerPattern", () => {
const testCases: TestCase<typeof utils.normalizeCustomHandlerPattern>[] = [
[ "should handle pattern without ending - add both versions", [ "foo" ], [ "foo", "foo/" ] ],
[ "should handle pattern with trailing slash - add both versions", [ "foo/" ], [ "foo", "foo/" ] ],
[ "should handle pattern ending with $ - add optional slash", [ "foo$" ], [ "foo/?$" ] ],
[ "should handle pattern with trailing slash and $ - add both versions", [ "foo/$" ], [ "foo$", "foo/$" ] ],
[ "should preserve existing optional slash pattern", [ "foo/?$" ], [ "foo/?$" ] ],
[ "should preserve existing optional slash pattern (alternative)", [ "foo/?)" ], [ "foo/?)" ] ],
[ "should handle regex pattern with special chars", [ "api/[a-z]+$" ], [ "api/[a-z]+/?$" ] ],
[ "should handle complex regex pattern", [ "user/([0-9]+)/profile$" ], [ "user/([0-9]+)/profile/?$" ] ],
[ "should handle empty string", [ "" ], [ "" ] ],
[ "should handle whitespace-only string", [ " " ], [ "" ] ],
[ "should handle null", [ null as any ], [ null ] ],
[ "should handle undefined", [ undefined as any ], [ undefined ] ]
];
testCases.forEach((testCase) => {
const [ desc, fnParams, expected ] = testCase;
it(desc, () => {
const result = utils.normalizeCustomHandlerPattern(...fnParams);
expect(result).toStrictEqual(expected);
});
});
});

View File

@ -375,6 +375,85 @@ export function safeExtractMessageAndStackFromError(err: unknown): [errMessage:
return (err instanceof Error) ? [err.message, err.stack] as const : ["Unknown Error", undefined] as const; return (err instanceof Error) ? [err.message, err.stack] as const : ["Unknown Error", undefined] as const;
} }
/**
* Normalizes URL by removing trailing slashes and fixing double slashes.
* Preserves the protocol (http://, https://) but removes trailing slashes from the rest.
*
* @param url The URL to normalize
* @returns The normalized URL without trailing slashes
*/
export function normalizeUrl(url: string | null | undefined): string | null | undefined {
if (!url || typeof url !== 'string') {
return url;
}
// Trim whitespace
url = url.trim();
if (!url) {
return url;
}
// Fix double slashes (except in protocol) first
url = url.replace(/([^:]\/)\/+/g, '$1');
// Remove trailing slash, but preserve protocol
if (url.endsWith('/') && !url.match(/^https?:\/\/$/)) {
url = url.slice(0, -1);
}
return url;
}
/**
* Normalizes a path pattern for custom request handlers.
* Ensures both trailing slash and non-trailing slash versions are handled.
*
* @param pattern The original pattern from customRequestHandler attribute
* @returns An array of patterns to match both with and without trailing slash
*/
export function normalizeCustomHandlerPattern(pattern: string | null | undefined): (string | null | undefined)[] {
if (!pattern || typeof pattern !== 'string') {
return [pattern];
}
pattern = pattern.trim();
if (!pattern) {
return [pattern];
}
// If pattern already ends with optional trailing slash, return as-is
if (pattern.endsWith('/?$') || pattern.endsWith('/?)')) {
return [pattern];
}
// If pattern ends with $, handle it specially
if (pattern.endsWith('$')) {
const basePattern = pattern.slice(0, -1);
// If already ends with slash, create both versions
if (basePattern.endsWith('/')) {
const withoutSlash = basePattern.slice(0, -1) + '$';
const withSlash = pattern;
return [withoutSlash, withSlash];
} else {
// Add optional trailing slash
const withSlash = basePattern + '/?$';
return [withSlash];
}
}
// For patterns without $, add both versions
if (pattern.endsWith('/')) {
const withoutSlash = pattern.slice(0, -1);
return [withoutSlash, pattern];
} else {
const withSlash = pattern + '/';
return [pattern, withSlash];
}
}
export default { export default {
compareVersions, compareVersions,
@ -400,6 +479,8 @@ export default {
md5, md5,
newEntityId, newEntityId,
normalize, normalize,
normalizeCustomHandlerPattern,
normalizeUrl,
quoteRegex, quoteRegex,
randomSecureToken, randomSecureToken,
randomString, randomString,