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.

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 double prepended forward slash in certain cases #7091

Merged
merged 4 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/swift-moons-drop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fix double prepended forward slash in SSR
6 changes: 3 additions & 3 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { call as callEndpoint, createAPIContext } from '../endpoint/index.js';
import { consoleLogDestination } from '../logger/console.js';
import { error, type LogOptions } from '../logger/core.js';
import { callMiddleware } from '../middleware/callMiddleware.js';
import { removeTrailingForwardSlash } from '../path.js';
import { prependForwardSlash, removeTrailingForwardSlash } from '../path.js';
import {
createEnvironment,
createRenderContext,
Expand Down Expand Up @@ -101,7 +101,7 @@ export class App {
if (this.#manifest.assets.has(url.pathname)) {
return undefined;
}
let pathname = '/' + this.removeBase(url.pathname);
let pathname = prependForwardSlash(this.removeBase(url.pathname));
let routeData = matchRoute(pathname, this.#manifestData);

if (routeData) {
Expand Down Expand Up @@ -178,7 +178,7 @@ export class App {
status = 200
): Promise<Response> {
const url = new URL(request.url);
const pathname = '/' + this.removeBase(url.pathname);
const pathname = prependForwardSlash(this.removeBase(url.pathname));
const info = this.#routeDataToRouteInfo.get(routeData!)!;
// may be used in the future for handling rel=modulepreload, rel=icon, rel=manifest etc.
const links = new Set<never>();
Expand Down
133 changes: 90 additions & 43 deletions packages/integrations/node/test/prerender.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,61 +9,108 @@ describe('Prerendering', () => {
let fixture;
let server;

before(async () => {
process.env.ASTRO_NODE_AUTOSTART = 'disabled';
fixture = await loadFixture({
base: '/some-base',
root: './fixtures/prerender/',
output: 'server',
adapter: nodejs({ mode: 'standalone' }),
});
await fixture.build();
const { startServer } = await await load();
let res = startServer();
server = res.server;
});

after(async () => {
await server.stop();
});

async function load() {
const mod = await import('./fixtures/prerender/dist/server/entry.mjs');
return mod;
}

it('Can render SSR route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/one`);
const html = await res.text();
const $ = cheerio.load(html);
describe('With base', () => {
before(async () => {
process.env.ASTRO_NODE_AUTOSTART = 'disabled';
fixture = await loadFixture({
base: '/some-base',
root: './fixtures/prerender/',
output: 'server',
adapter: nodejs({ mode: 'standalone' }),
});
await fixture.build();
const { startServer } = await await load();
let res = startServer();
server = res.server;
});

expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('One');
});
after(async () => {
await server.stop();
});

it('Can render prerendered route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/two`);
const html = await res.text();
const $ = cheerio.load(html);
it('Can render SSR route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/one`);
const html = await res.text();
const $ = cheerio.load(html);

expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Two');
});
expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('One');
});

it('Can render prerendered route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/two`);
const html = await res.text();
const $ = cheerio.load(html);

expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Two');
});

it('Can render prerendered route with query params', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/two/?foo=bar`);
const html = await res.text();
const $ = cheerio.load(html);
it('Can render prerendered route with query params', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/two/?foo=bar`);
const html = await res.text();
const $ = cheerio.load(html);

expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Two');
expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Two');
});

it('Omitting the trailing slash results in a redirect that includes the base', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/two`, {
redirect: 'manual',
});
expect(res.status).to.equal(301);
expect(res.headers.get('location')).to.equal('/some-base/two/');
});
});
describe('Without base', () => {
before(async () => {
process.env.ASTRO_NODE_AUTOSTART = 'disabled';
fixture = await loadFixture({
root: './fixtures/prerender/',
output: 'server',
adapter: nodejs({ mode: 'standalone' }),
});
await fixture.build();
const { startServer } = await await load();
let res = startServer();
server = res.server;
});

after(async () => {
await server.stop();
});

it('Can render SSR route', 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');
});

it('Can render prerendered route', async () => {
const res = await fetch(`http://${server.host}:${server.port}/two`);
const html = await res.text();
const $ = cheerio.load(html);

expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Two');
});

it('Can render prerendered route with query params', async () => {
const res = await fetch(`http://${server.host}:${server.port}/two/?foo=bar`);
const html = await res.text();
const $ = cheerio.load(html);

it('Omitting the trailing slash results in a redirect that includes the base', async () => {
const res = await fetch(`http://${server.host}:${server.port}/some-base/two`, {
redirect: 'manual',
expect(res.status).to.equal(200);
expect($('h1').text()).to.equal('Two');
});
expect(res.status).to.equal(301);
expect(res.headers.get('location')).to.equal('/some-base/two/');
});
});