Skip to content

Commit

Permalink
fix: do not append trailing slash to subresource urls (#10491)
Browse files Browse the repository at this point in the history 
* fix: do not append traling slash to subresource urls

Signed-off-by: Andres Correa Casablanca <andreu@kindspells.dev>

* test: fix broken test

Signed-off-by: Andres Correa Casablanca <andreu@kindspells.dev>

* refactor: packages/integrations/node/src/serve-static.ts

Co-authored-by: Arsh <69170106+lilnasy@users.noreply.github.com>

---------

Signed-off-by: Andres Correa Casablanca <andreu@kindspells.dev>
Co-authored-by: Arsh <69170106+lilnasy@users.noreply.github.com>
  • Loading branch information
@castarco @lilnasy
castarco and lilnasy committed Mar 19, 2024
1 parent e4a6462 commit 28e33a2
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 54 deletions.
5 changes: 5 additions & 0 deletions .changeset/tender-snails-accept.md
View file
@@ -0,0 +1,5 @@
---
"@astrojs/node": patch
---

Fixes a bug where the preview server wrongly appends trailing slashes to subresource URLs.
6 changes: 5 additions & 1 deletion packages/integrations/node/src/serve-static.ts
View file
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions packages/integrations/node/test/fixtures/trailing-slash/public/one.css
View file
@@ -0,0 +1 @@
h1 { color: red; }
123 changes: 70 additions & 53 deletions .../integrations/node/test/trailing-slash.js → ...grations/node/test/trailing-slash.test.js
View file
@@ -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';
Expand Down Expand Up @@ -48,34 +49,42 @@ 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 () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/one/?foo=bar`);
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 () => {
Expand Down Expand Up @@ -105,34 +114,42 @@ 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 () => {
const res = await fetch(`http://${server.host}:${server.port}/one/?foo=bar`);
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 () => {
Expand Down Expand Up @@ -165,34 +182,34 @@ 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 () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/one?foo=bar`);
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 () => {
Expand Down Expand Up @@ -223,34 +240,34 @@ 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 and query params', async () => {
const res = await fetch(`http://${server.host}:${server.port}/one?foo=bar`);
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');
});
});
});
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand Down Expand Up @@ -360,26 +377,26 @@ 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 () => {
const res = await fetch(`http://${server.host}:${server.port}/one/`);
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 () => {
const res = await fetch(`http://${server.host}:${server.port}/one`);
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 () => {
Expand All @@ -389,17 +406,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('Can render prerendered route without slash and with query params', async () => {
const res = await fetch(`http://${server.host}:${server.port}/one?foo=bar`);
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');
});
});
});
Expand Down

0 comments on commit 28e33a2

Please sign in to comment.