From 28e33a2f9c04373eae5da2e6edb0dc2981bce790 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Correa=20Casablanca?= Date: Tue, 19 Mar 2024 16:43:08 +0100 Subject: [PATCH] fix: do not append trailing slash to subresource urls (#10491) * fix: do not append traling slash to subresource urls Signed-off-by: Andres Correa Casablanca * test: fix broken test Signed-off-by: Andres Correa Casablanca * refactor: packages/integrations/node/src/serve-static.ts Co-authored-by: Arsh <69170106+lilnasy@users.noreply.github.com> --------- Signed-off-by: Andres Correa Casablanca Co-authored-by: Arsh <69170106+lilnasy@users.noreply.github.com> --- .changeset/tender-snails-accept.md | 5 + .../integrations/node/src/serve-static.ts | 6 +- .../fixtures/trailing-slash/public/one.css | 1 + ...ailing-slash.js => trailing-slash.test.js} | 123 ++++++++++-------- 4 files changed, 81 insertions(+), 54 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} (77%) diff --git a/.changeset/tender-snails-accept.md b/.changeset/tender-snails-accept.md new file mode 100644 index 0000000000..c29b04123b --- /dev/null +++ b/.changeset/tender-snails-accept.md @@ -0,0 +1,5 @@ +--- +"@astrojs/node": patch +--- + +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 a65e52e96c..811f748a47 100644 --- a/packages/integrations/node/src/serve-static.ts +++ b/packages/integrations/node/src/serve-static.ts @@ -6,6 +6,9 @@ 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 + /** * 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. @@ -48,7 +51,8 @@ export function createStaticHandler(app: NodeApp, options: Options) { } break; case 'always': - if (!hasSlash) { + // trailing slash is not added to "subresources" + if (!hasSlash && !urlPath.match(isSubresourceRegex)) { 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 0000000000..5ce768ca55 --- /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 77% rename from packages/integrations/node/test/trailing-slash.js rename to packages/integrations/node/test/trailing-slash.test.js index 28bfe0f5d2..64fff19645 100644 --- a/packages/integrations/node/test/trailing-slash.js +++ b/packages/integrations/node/test/trailing-slash.test.js @@ -1,4 +1,5 @@ -import { expect } from 'chai'; +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,9 +74,17 @@ 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(); + + assert.equal(res.status, 200); + assert.equal(css, 'h1 { color: red; }\n'); + }) }); describe('Without base', async () => { before(async () => { @@ -105,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 () => { @@ -130,9 +139,17 @@ 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(); + + assert.equal(res.status, 200); + assert.equal(css, 'h1 { color: red; }\n'); + }) }); }); describe('Never', async () => { @@ -165,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 () => { @@ -182,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 () => { @@ -191,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 () => { @@ -223,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 () => { @@ -240,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 () => { @@ -249,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'); }); }); }); @@ -284,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 () => { @@ -295,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 () => { @@ -306,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 () => { @@ -317,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 () => { @@ -328,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 () => { @@ -360,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 () => { @@ -369,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 () => { @@ -378,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 () => { @@ -389,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 () => { @@ -398,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'); }); }); });