From 69c2698ed9fe44a1a0139fd319678fa3d1fcea96 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Wed, 27 Sep 2023 16:49:32 +0300 Subject: [PATCH 1/2] fix: redirect when missing trailing slash --- src/handlers/get.ts | 3 ++ src/util.ts | 18 ++++++--- tests/e2e/directory.test.ts | 45 ++++++++++++++------- tests/e2e/test-data/expected-html/dist.html | 4 +- 4 files changed, 49 insertions(+), 21 deletions(-) diff --git a/src/handlers/get.ts b/src/handlers/get.ts index 3427881..e5d35a8 100644 --- a/src/handlers/get.ts +++ b/src/handlers/get.ts @@ -2,6 +2,7 @@ import responses from '../responses'; import { isCacheEnabled, isDirectoryPath, + hasTrailingSlash, mapUrlPathToBucketPath, parseUrl, } from '../util'; @@ -39,6 +40,8 @@ const getHandler: Handler = async (request, env, ctx, cache) => { // File not found since we should only be allowing // file paths if directory listing is off return responses.FILE_NOT_FOUND(request); + } else if (isPathADirectory && !hasTrailingSlash(bucketPath)) { + return Response.redirect(url.toString() + '/', 301); } const response: Response = isPathADirectory diff --git a/src/util.ts b/src/util.ts index 472ccb3..b7a7e86 100644 --- a/src/util.ts +++ b/src/util.ts @@ -99,6 +99,18 @@ export function mapBucketPathToUrlPath( : ['/' + bucketPath]; } +export function hasTrailingSlash(path: string): boolean { + return path[path.length - 1] === '/'; +} + +export function isExtensionless(path: string): boolean { + // `path.lastIndexOf('.') == -1` is a Node-specific + // heuristic here. There aren't any files that don't + // have file extensions, so, if there are no file extensions + // specified in the url, treat it like a directory. + return path.lastIndexOf('.') === -1; +} + /** * Checks if a R2 path is for a directory or not. * If a path ends in a `/` or there's no file @@ -107,11 +119,7 @@ export function mapBucketPathToUrlPath( * @returns True if it's for a directory */ export function isDirectoryPath(path: string): boolean { - // `path.lastIndexOf('.') == -1` is a Node-specific - // heuristic here. There aren't any files that don't - // have file extensions, so, if there are no file extensions - // specified in the url, treat it like a directory. - return path[path.length - 1] == '/' || path.lastIndexOf('.') == -1; + return hasTrailingSlash(path) || isExtensionless(path); } /** diff --git a/tests/e2e/directory.test.ts b/tests/e2e/directory.test.ts index 39b730f..aa8eeb1 100644 --- a/tests/e2e/directory.test.ts +++ b/tests/e2e/directory.test.ts @@ -24,15 +24,16 @@ describe('Directory Tests (Restricted Directory Listing)', () => { url = await mf.ready; }); - it('allows `/dist` and returns expected html', async () => { - const [res, expectedHtml] = await Promise.all([ - mf.dispatchFetch(`${url}dist`), + it('redirects `/dist` to `/dist/` and returns expected html', async () => { + const [originalRes, expectedHtml] = await Promise.all([ + mf.dispatchFetch(`${url}dist`, { redirect: 'manual' }), readFile('./tests/e2e/test-data/expected-html/dist.html', { encoding: 'utf-8', }), ]); - assert.strictEqual(res.status, 200); + assert.strictEqual(originalRes.status, 301); + const res = await mf.dispatchFetch(originalRes.headers.get('location')!); // Assert that the html matches what we're expecting // to be returned. If this passes, we can assume @@ -48,8 +49,12 @@ describe('Directory Tests (Restricted Directory Listing)', () => { assert.strictEqual(res.status, 200); }); - it('allows `/download`', async () => { - const res = await mf.dispatchFetch(`${url}download`); + it('redirects `/download` to `/download/`', async () => { + const originalRes = await mf.dispatchFetch(`${url}download`, { + redirect: 'manual', + }); + assert.strictEqual(originalRes.status, 301); + const res = await mf.dispatchFetch(originalRes.headers.get('location')!); assert.strictEqual(res.status, 200); }); @@ -58,8 +63,12 @@ describe('Directory Tests (Restricted Directory Listing)', () => { assert.strictEqual(res.status, 200); }); - it('allows `/docs`', async () => { - const res = await mf.dispatchFetch(`${url}docs`); + it('redirects `/docs` to `/docs/`', async () => { + const originalRes = await mf.dispatchFetch(`${url}docs`, { + redirect: 'manual', + }); + assert.strictEqual(originalRes.status, 301); + const res = await mf.dispatchFetch(originalRes.headers.get('location')!); assert.strictEqual(res.status, 200); }); @@ -68,8 +77,12 @@ describe('Directory Tests (Restricted Directory Listing)', () => { assert.strictEqual(res.status, 200); }); - it('allows `/api`', async () => { - const res = await mf.dispatchFetch(`${url}api`); + it('redirects `/api` to `/api/`', async () => { + const originalRes = await mf.dispatchFetch(`${url}api`, { + redirect: 'manual', + }); + assert.strictEqual(originalRes.status, 301); + const res = await mf.dispatchFetch(originalRes.headers.get('location')!); assert.strictEqual(res.status, 200); }); @@ -78,8 +91,12 @@ describe('Directory Tests (Restricted Directory Listing)', () => { assert.strictEqual(res.status, 200); }); - it('allows `/metrics`', async () => { - const res = await mf.dispatchFetch(`${url}metrics`); + it('redirects `/metrics` to `/metrics/`', async () => { + const originalRes = await mf.dispatchFetch(`${url}metrics`, { + redirect: 'manual', + }); + assert.strictEqual(originalRes.status, 301); + const res = await mf.dispatchFetch(originalRes.headers.get('location')!); assert.strictEqual(res.status, 200); }); @@ -92,7 +109,7 @@ describe('Directory Tests (Restricted Directory Listing)', () => { let res = await mf.dispatchFetch(url); assert.strictEqual(res.status, 401); - res = await mf.dispatchFetch(`${url}/asd`); + res = await mf.dispatchFetch(`${url}/asd/`); assert.strictEqual(res.status, 401); res = await mf.dispatchFetch(`${url}/asd/123/`); @@ -100,7 +117,7 @@ describe('Directory Tests (Restricted Directory Listing)', () => { }); it('returns 404 for unknown directory', async () => { - const res = await mf.dispatchFetch(`${url}/dist/asd123`); + const res = await mf.dispatchFetch(`${url}/dist/asd123/`); assert.strictEqual(res.status, 404); const body = await res.text(); diff --git a/tests/e2e/test-data/expected-html/dist.html b/tests/e2e/test-data/expected-html/dist.html index 518ace1..ef89609 100644 --- a/tests/e2e/test-data/expected-html/dist.html +++ b/tests/e2e/test-data/expected-html/dist.html @@ -1,7 +1,7 @@ - Index of /dist + Index of /dist/ -

Index of /dist

+

Index of /dist/

From 7c13ad454fd89447ffe283f7fd173e3f95549593 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Wed, 27 Sep 2023 18:28:56 +0300 Subject: [PATCH 2/2] CR --- src/handlers/get.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/handlers/get.ts b/src/handlers/get.ts index e5d35a8..0f4a9ca 100644 --- a/src/handlers/get.ts +++ b/src/handlers/get.ts @@ -41,7 +41,8 @@ const getHandler: Handler = async (request, env, ctx, cache) => { // file paths if directory listing is off return responses.FILE_NOT_FOUND(request); } else if (isPathADirectory && !hasTrailingSlash(bucketPath)) { - return Response.redirect(url.toString() + '/', 301); + url.pathname += '/'; + return Response.redirect(url.toString(), 301); } const response: Response = isPathADirectory
FilenameModifiedSize