Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not append trailing slash to subresource urls #10491

Merged
merged 3 commits into from
Mar 19, 2024
Merged

fix: do not append trailing slash to subresource urls #10491

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0abf851
fix: do not append traling slash to subresource urls
castarco Mar 19, 2024
2184d75
test: fix broken test
castarco Mar 19, 2024
6c6637d
refactor: packages/integrations/node/src/serve-static.ts
castarco Mar 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tender-snails-accept.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@astrojs/node": patch
---

Fixes a bug where the preview server wrongly appends trailing slashes to subresource URLs.
5 changes: 4 additions & 1 deletion packages/integrations/node/src/serve-static.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
castarco marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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 +50,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
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Loading