From 0abf85184d77af7ba2e2e74cf77e4c4ec5b4ffff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Correa=20Casablanca?= Date: Tue, 19 Mar 2024 12:00:51 +0100 Subject: [PATCH 1/3] fix: do not append traling slash to subresource urls Signed-off-by: Andres Correa Casablanca --- .changeset/tender-snails-accept.md | 5 +++++ packages/integrations/node/src/serve-static.ts | 3 ++- .../fixtures/trailing-slash/public/one.css | 1 + ...railing-slash.js => trailing-slash.test.js} | 18 +++++++++++++++++- 4 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 .changeset/tender-snails-accept.md create mode 100644 packages/integrations/node/test/fixtures/trailing-slash/public/one.css rename packages/integrations/node/test/{trailing-slash.js => trailing-slash.test.js} (95%) diff --git a/.changeset/tender-snails-accept.md b/.changeset/tender-snails-accept.md new file mode 100644 index 000000000000..4b4c8f3db494 --- /dev/null +++ b/.changeset/tender-snails-accept.md @@ -0,0 +1,5 @@ +--- +"@astrojs/node": patch +--- + +fix: do not append traling slash to subresource urls diff --git a/packages/integrations/node/src/serve-static.ts b/packages/integrations/node/src/serve-static.ts index a65e52e96c24..21eddbee8cad 100644 --- a/packages/integrations/node/src/serve-static.ts +++ b/packages/integrations/node/src/serve-static.ts @@ -48,7 +48,8 @@ export function createStaticHandler(app: NodeApp, options: Options) { } break; case 'always': - if (!hasSlash) { + // trailing slash is not added to "subresources" + if (!hasSlash && !urlPath.match(/.+\.[a-z]+$/i)) { pathname = urlPath + '/' + (urlQuery ? '?' + urlQuery : ''); res.statusCode = 301; res.setHeader('Location', pathname); diff --git a/packages/integrations/node/test/fixtures/trailing-slash/public/one.css b/packages/integrations/node/test/fixtures/trailing-slash/public/one.css new file mode 100644 index 000000000000..5ce768ca55bb --- /dev/null +++ b/packages/integrations/node/test/fixtures/trailing-slash/public/one.css @@ -0,0 +1 @@ +h1 { color: red; } diff --git a/packages/integrations/node/test/trailing-slash.js b/packages/integrations/node/test/trailing-slash.test.js similarity index 95% rename from packages/integrations/node/test/trailing-slash.js rename to packages/integrations/node/test/trailing-slash.test.js index 28bfe0f5d224..69ef47ccae62 100644 --- a/packages/integrations/node/test/trailing-slash.js +++ b/packages/integrations/node/test/trailing-slash.test.js @@ -1,4 +1,4 @@ -import { expect } from 'chai'; +import { after, before, describe, it, expect } from 'node:test'; import * as cheerio from 'cheerio'; import nodejs from '../dist/index.js'; import { loadFixture } from './test-utils.js'; @@ -76,6 +76,14 @@ describe('Trailing slash', () => { expect(res.status).to.equal(200); expect($('h1').text()).to.equal('One'); }); + + it('Does not add trailing slash to subresource urls', async () => { + const res = await fetch(`http://${server.host}:${server.port}/some-base/one.css`); + const css = await res.text(); + + expect(res.status).to.equal(200); + expect(css).to.equal('h1 { color: red; }\n'); + }) }); describe('Without base', async () => { before(async () => { @@ -133,6 +141,14 @@ describe('Trailing slash', () => { expect(res.status).to.equal(200); expect($('h1').text()).to.equal('One'); }); + + it('Does not add trailing slash to subresource urls', async () => { + const res = await fetch(`http://${server.host}:${server.port}/one.css`); + const css = await res.text(); + + expect(res.status).to.equal(200); + expect(css).to.equal('h1 { color: red; }\n'); + }) }); }); describe('Never', async () => { From 2184d7571dbdc98abf8a8357006367ed9b1d9ea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Correa=20Casablanca?= Date: Tue, 19 Mar 2024 13:29:04 +0100 Subject: [PATCH 2/3] test: fix broken test Signed-off-by: Andres Correa Casablanca --- .changeset/tender-snails-accept.md | 2 +- .../integrations/node/src/serve-static.ts | 4 +- .../node/test/trailing-slash.test.js | 115 +++++++++--------- 3 files changed, 62 insertions(+), 59 deletions(-) diff --git a/.changeset/tender-snails-accept.md b/.changeset/tender-snails-accept.md index 4b4c8f3db494..c29b04123b1e 100644 --- a/.changeset/tender-snails-accept.md +++ b/.changeset/tender-snails-accept.md @@ -2,4 +2,4 @@ "@astrojs/node": patch --- -fix: do not append traling slash to subresource urls +Fixes a bug where the preview server wrongly appends trailing slashes to subresource URLs. diff --git a/packages/integrations/node/src/serve-static.ts b/packages/integrations/node/src/serve-static.ts index 21eddbee8cad..2c1d3853f9a5 100644 --- a/packages/integrations/node/src/serve-static.ts +++ b/packages/integrations/node/src/serve-static.ts @@ -6,6 +6,8 @@ import type { NodeApp } from 'astro/app/node'; import send from 'send'; import type { Options } from './types.js'; +const isSubresourceRegex = /.+\.[a-z]+$/i + /** * Creates a Node.js http listener for static files and prerendered pages. * In standalone mode, the static handler is queried first for the static files. @@ -49,7 +51,7 @@ export function createStaticHandler(app: NodeApp, options: Options) { break; case 'always': // trailing slash is not added to "subresources" - if (!hasSlash && !urlPath.match(/.+\.[a-z]+$/i)) { + if (!hasSlash && !urlPath.match(isSubresourceRegex)) { pathname = urlPath + '/' + (urlQuery ? '?' + urlQuery : ''); res.statusCode = 301; res.setHeader('Location', pathname); diff --git a/packages/integrations/node/test/trailing-slash.test.js b/packages/integrations/node/test/trailing-slash.test.js index 69ef47ccae62..64fff1964564 100644 --- a/packages/integrations/node/test/trailing-slash.test.js +++ b/packages/integrations/node/test/trailing-slash.test.js @@ -1,4 +1,5 @@ -import { after, before, describe, it, expect } from 'node:test'; +import { after, before, describe, it } from 'node:test'; +import * as assert from 'node:assert/strict'; import * as cheerio from 'cheerio'; import nodejs from '../dist/index.js'; import { loadFixture } from './test-utils.js'; @@ -48,24 +49,24 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('Index'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'Index'); }); it('Can render prerendered route with redirect', async () => { const res = await fetch(`http://${server.host}:${server.port}/some-base/one`, { redirect: 'manual', }); - expect(res.status).to.equal(301); - expect(res.headers.get('location')).to.equal('/some-base/one/'); + assert.equal(res.status, 301); + assert.equal(res.headers.get('location'), '/some-base/one/'); }); it('Can render prerendered route with redirect and query params', async () => { const res = await fetch(`http://${server.host}:${server.port}/some-base/one?foo=bar`, { redirect: 'manual', }); - expect(res.status).to.equal(301); - expect(res.headers.get('location')).to.equal('/some-base/one/?foo=bar'); + assert.equal(res.status, 301); + assert.equal(res.headers.get('location'), '/some-base/one/?foo=bar'); }); it('Can render prerendered route with query params', async () => { @@ -73,16 +74,16 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('One'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'One'); }); it('Does not add trailing slash to subresource urls', async () => { const res = await fetch(`http://${server.host}:${server.port}/some-base/one.css`); const css = await res.text(); - expect(res.status).to.equal(200); - expect(css).to.equal('h1 { color: red; }\n'); + assert.equal(res.status, 200); + assert.equal(css, 'h1 { color: red; }\n'); }) }); describe('Without base', async () => { @@ -113,24 +114,24 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('Index'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'Index'); }); it('Can render prerendered route with redirect', async () => { const res = await fetch(`http://${server.host}:${server.port}/one`, { redirect: 'manual', }); - expect(res.status).to.equal(301); - expect(res.headers.get('location')).to.equal('/one/'); + assert.equal(res.status, 301); + assert.equal(res.headers.get('location'), '/one/'); }); it('Can render prerendered route with redirect and query params', async () => { const res = await fetch(`http://${server.host}:${server.port}/one?foo=bar`, { redirect: 'manual', }); - expect(res.status).to.equal(301); - expect(res.headers.get('location')).to.equal('/one/?foo=bar'); + assert.equal(res.status, 301); + assert.equal(res.headers.get('location'), '/one/?foo=bar'); }); it('Can render prerendered route with query params', async () => { @@ -138,16 +139,16 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('One'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'One'); }); it('Does not add trailing slash to subresource urls', async () => { const res = await fetch(`http://${server.host}:${server.port}/one.css`); const css = await res.text(); - expect(res.status).to.equal(200); - expect(css).to.equal('h1 { color: red; }\n'); + assert.equal(res.status, 200); + assert.equal(css, 'h1 { color: red; }\n'); }) }); }); @@ -181,16 +182,16 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('Index'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'Index'); }); it('Can render prerendered route with redirect', async () => { const res = await fetch(`http://${server.host}:${server.port}/some-base/one/`, { redirect: 'manual', }); - expect(res.status).to.equal(301); - expect(res.headers.get('location')).to.equal('/some-base/one'); + assert.equal(res.status, 301); + assert.equal(res.headers.get('location'), '/some-base/one'); }); it('Can render prerendered route with redirect and query params', async () => { @@ -198,8 +199,8 @@ describe('Trailing slash', () => { redirect: 'manual', }); - expect(res.status).to.equal(301); - expect(res.headers.get('location')).to.equal('/some-base/one?foo=bar'); + assert.equal(res.status, 301); + assert.equal(res.headers.get('location'), '/some-base/one?foo=bar'); }); it('Can render prerendered route with query params', async () => { @@ -207,8 +208,8 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('One'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'One'); }); }); describe('Without base', async () => { @@ -239,16 +240,16 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('Index'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'Index'); }); it('Can render prerendered route with redirect', async () => { const res = await fetch(`http://${server.host}:${server.port}/one/`, { redirect: 'manual', }); - expect(res.status).to.equal(301); - expect(res.headers.get('location')).to.equal('/one'); + assert.equal(res.status, 301); + assert.equal(res.headers.get('location'), '/one'); }); it('Can render prerendered route with redirect and query params', async () => { @@ -256,8 +257,8 @@ describe('Trailing slash', () => { redirect: 'manual', }); - expect(res.status).to.equal(301); - expect(res.headers.get('location')).to.equal('/one?foo=bar'); + assert.equal(res.status, 301); + assert.equal(res.headers.get('location'), '/one?foo=bar'); }); it('Can render prerendered route and query params', async () => { @@ -265,8 +266,8 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('One'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'One'); }); }); }); @@ -300,8 +301,8 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('Index'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'Index'); }); it('Can render prerendered route with slash', async () => { @@ -311,8 +312,8 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('One'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'One'); }); it('Can render prerendered route without slash', async () => { @@ -322,8 +323,8 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('One'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'One'); }); it('Can render prerendered route with slash and query params', async () => { @@ -333,8 +334,8 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('One'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'One'); }); it('Can render prerendered route without slash and with query params', async () => { @@ -344,8 +345,8 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('One'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'One'); }); }); describe('Without base', async () => { @@ -376,8 +377,8 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('Index'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'Index'); }); it('Can render prerendered route with slash', async () => { @@ -385,8 +386,8 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('One'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'One'); }); it('Can render prerendered route without slash', async () => { @@ -394,8 +395,8 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('One'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'One'); }); it('Can render prerendered route with slash and query params', async () => { @@ -405,8 +406,8 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('One'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'One'); }); it('Can render prerendered route without slash and with query params', async () => { @@ -414,8 +415,8 @@ describe('Trailing slash', () => { const html = await res.text(); const $ = cheerio.load(html); - expect(res.status).to.equal(200); - expect($('h1').text()).to.equal('One'); + assert.equal(res.status, 200); + assert.equal($('h1').text(), 'One'); }); }); }); From 6c6637d9cab8c7ef60f8b6b9dffa60e251cee02b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Correa=20Casablanca?= Date: Tue, 19 Mar 2024 16:37:48 +0100 Subject: [PATCH 3/3] refactor: packages/integrations/node/src/serve-static.ts Co-authored-by: Arsh <69170106+lilnasy@users.noreply.github.com> --- packages/integrations/node/src/serve-static.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/integrations/node/src/serve-static.ts b/packages/integrations/node/src/serve-static.ts index 2c1d3853f9a5..811f748a47ca 100644 --- a/packages/integrations/node/src/serve-static.ts +++ b/packages/integrations/node/src/serve-static.ts @@ -6,6 +6,7 @@ import type { NodeApp } from 'astro/app/node'; import send from 'send'; import type { Options } from './types.js'; +// check for a dot followed by a extension made up of lowercase characters const isSubresourceRegex = /.+\.[a-z]+$/i /**