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: handle relative assets paths in server fetch correctly #12113

Merged
merged 8 commits into from
Oct 11, 2024
Merged
8 changes: 8 additions & 0 deletions .changeset/dirty-rivers-tickle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@sveltejs/adapter-cloudflare-workers": patch
"@sveltejs/adapter-cloudflare": patch
"@sveltejs/adapter-netlify": patch
"@sveltejs/kit": patch
---

fix: fix handling of relative paths in fetch on the server
eltigerchino marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 4 additions & 1 deletion packages/adapter-cloudflare-workers/files/entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ export default {
const filename = stripped_pathname.substring(1);
if (filename) {
is_static_asset =
manifest.assets.has(filename) || manifest.assets.has(filename + '/index.html');
manifest.assets.has(filename) ||
manifest.assets.has(filename + '/index.html') ||
filename in manifest._.server_assets ||
filename + '/index.html' in manifest._.server_assets;
}

let location = pathname.at(-1) === '/' ? stripped_pathname : pathname + '/';
Expand Down
5 changes: 4 additions & 1 deletion packages/adapter-cloudflare/src/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ const worker = {
const filename = stripped_pathname.substring(1);
if (filename) {
is_static_asset =
manifest.assets.has(filename) || manifest.assets.has(filename + '/index.html');
manifest.assets.has(filename) ||
manifest.assets.has(filename + '/index.html') ||
filename in manifest._.server_assets ||
filename + '/index.html' in manifest._.server_assets;
}

let location = pathname.at(-1) === '/' ? stripped_pathname : pathname + '/';
Expand Down
2 changes: 2 additions & 0 deletions packages/adapter-netlify/src/edge.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ function is_static_file(request) {
return (
manifest.assets.has(file) ||
manifest.assets.has(file + '/index.html') ||
file in manifest._.server_assets ||
file + '/index.html' in manifest._.server_assets ||
prerendered.has(pathname || '/')
);
}
8 changes: 7 additions & 1 deletion packages/kit/src/exports/vite/dev/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,13 @@ export async function dev(vite, vite_config, svelte_config) {
if (remoteAddress) return remoteAddress;
throw new Error('Could not determine clientAddress');
},
read: (file) => fs.readFileSync(path.join(svelte_config.kit.files.assets, file)),
read: (file) => {
if (file in manifest._.server_assets) {
return fs.readFileSync(from_fs(file));
}

return fs.readFileSync(path.join(svelte_config.kit.files.assets, file));
},
before_handle: (event, config, prerender) => {
async_local_storage.enterWith({ event, config, prerender });
},
Expand Down
8 changes: 7 additions & 1 deletion packages/kit/src/exports/vite/preview/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,13 @@ export async function preview(vite, vite_config, svelte_config) {
if (remoteAddress) return remoteAddress;
throw new Error('Could not determine clientAddress');
},
read: (file) => fs.readFileSync(join(svelte_config.kit.files.assets, file)),
read: (file) => {
if (file in manifest._.server_assets) {
return fs.readFileSync(join(dir, file));
}

return fs.readFileSync(join(svelte_config.kit.files.assets, file));
},
emulator
})
);
Expand Down
16 changes: 14 additions & 2 deletions packages/kit/src/runtime/server/fetch.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as set_cookie_parser from 'set-cookie-parser';
import { respond } from './respond.js';
import * as paths from '__sveltekit/paths';
import { read_implementation } from '__sveltekit/server';

/**
* @param {{
Expand Down Expand Up @@ -81,8 +82,9 @@ export function create_fetch({ event, options, manifest, state, get_cookie_heade
).slice(1);
const filename_html = `${filename}/index.html`; // path may also match path/index.html

const is_asset = manifest.assets.has(filename);
const is_asset_html = manifest.assets.has(filename_html);
const is_asset = manifest.assets.has(filename) || filename in manifest._.server_assets;
const is_asset_html =
manifest.assets.has(filename_html) || filename_html in manifest._.server_assets;

if (is_asset || is_asset_html) {
const file = is_asset ? filename : filename_html;
Expand All @@ -95,6 +97,16 @@ export function create_fetch({ event, options, manifest, state, get_cookie_heade
return new Response(state.read(file), {
headers: type ? { 'content-type': type } : {}
});
} else if (read_implementation) {
const length = manifest._.server_assets[file];
const type = manifest.mimeTypes[file.slice(file.lastIndexOf('.'))];

return new Response(read_implementation(file), {
headers: {
'Content-Length': '' + length,
'Content-Type': type
}
});
}

return await fetch(request);
Expand Down
eltigerchino marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import file_path from '../test.txt?url';

/** @type {import('@sveltejs/kit').RequestHandler} */
export async function GET({ url }) {
const absolute = new URL(file_path, url.origin);

const response = await fetch(absolute);

return new Response(response.body, {
headers: {
'Content-Type': response.headers.get('Content-Type')
}
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import file_path from '../test.txt?url';

/** @type {import('@sveltejs/kit').RequestHandler} */
export async function GET({ fetch }) {
const response = await fetch(file_path);

return new Response(response.body, {
headers: {
'Content-Type': response.headers.get('Content-Type')
}
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Cos sie konczy, cos zaczyna
14 changes: 14 additions & 0 deletions packages/kit/test/apps/basics/test/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,20 @@ test.describe('Endpoints', () => {
expect(response.status()).toBe(200);
expect(await response.text()).toBe('catch-all');
});

test('can get assets using absolute path', async ({ request }) => {
const response = await request.get('/endpoint-output/fetch-asset/absolute');
expect(response.status()).toBe(200);
expect(response.headers()['content-type']).toBe('text/plain');
expect(await response.text()).toBe('Cos sie konczy, cos zaczyna');
});

test('can get assets using relative path', async ({ request }) => {
const response = await request.get('/endpoint-output/fetch-asset/relative');
expect(response.status()).toBe(200);
expect(response.headers()['content-type']).toBe('text/plain');
expect(await response.text()).toBe('Cos sie konczy, cos zaczyna');
});
});

test.describe('Errors', () => {
Expand Down
Loading