Skip to content
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/busy-trains-see.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes an issue where renaming an image file while the dev server is running triggers a build error. Now Astro correctly hot-reloads the image without crashing.
5 changes: 5 additions & 0 deletions .changeset/hot-walls-grin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Hardens `addAttribute` to drop attribute names containing characters that are invalid per the HTML spec (`"`, `'`, `>`, `/`, `=`, whitespace)
5 changes: 5 additions & 0 deletions .changeset/yummy-hoops-grin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/netlify': patch
---

Hardens `remotePatterns` regex generation to match canonical wildcard semantics more strictly
29 changes: 28 additions & 1 deletion packages/astro/src/content/vite-plugin-content-virtual-mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,26 @@ interface AstroContentVirtualModPluginParams {
fs: typeof nodeFs;
}

function invalidateAssetImports(viteServer: ViteDevServer, filePath: string) {
const timestamp = Date.now();
for (const environment of Object.values(viteServer.environments)) {
const modules = environment.moduleGraph.getModulesByFile(filePath);
if (modules) {
for (const module of modules) {
environment.moduleGraph.invalidateModule(module, undefined, timestamp, true);
}
}
if (isRunnableDevEnvironment(environment)) {
const runnerModules = environment.runner.evaluatedModules.getModulesByFile(filePath);
if (runnerModules) {
for (const runnerModule of runnerModules) {
environment.runner.evaluatedModules.invalidateModule(runnerModule);
}
}
}
}
}

function invalidateDataStore(viteServer: ViteDevServer) {
const environment = viteServer.environments[ASTRO_VITE_ENVIRONMENT_NAMES.ssr];
const module = environment.moduleGraph.getModuleById(RESOLVED_DATA_STORE_VIRTUAL_ID);
Expand Down Expand Up @@ -84,10 +104,13 @@ export function astroContentVirtualModPlugin({
},
buildStart() {
if (devServer) {
const assetImportsPath = fileURLToPath(new URL(ASSET_IMPORTS_FILE, settings.dotAstroDir));
// We defer adding the data store file to the watcher until the server is ready
devServer.watcher.add(fileURLToPath(dataStoreFile));
devServer.watcher.add(assetImportsPath);
// Manually invalidate the data store to avoid a race condition in file watching
invalidateDataStore(devServer);
invalidateAssetImports(devServer, assetImportsPath);
}
},
resolveId: {
Expand Down Expand Up @@ -209,17 +232,21 @@ export function astroContentVirtualModPlugin({
configureServer(server) {
devServer = server;
const dataStorePath = fileURLToPath(dataStoreFile);
// If the datastore file changes, invalidate the virtual module
const assetImportsPath = fileURLToPath(new URL(ASSET_IMPORTS_FILE, settings.dotAstroDir));

server.watcher.on('add', (addedPath) => {
if (addedPath === dataStorePath) {
invalidateDataStore(server);
invalidateAssetImports(server, assetImportsPath);
}
});

server.watcher.on('change', (changedPath) => {
if (changedPath === dataStorePath) {
invalidateDataStore(server);
invalidateAssetImports(server, assetImportsPath);
} else if (changedPath === assetImportsPath) {
invalidateAssetImports(server, assetImportsPath);
}
});
},
Expand Down
8 changes: 8 additions & 0 deletions packages/astro/src/runtime/server/render/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ const DOUBLE_QUOTE_REGEX = /"/g;

const STATIC_DIRECTIVES = new Set(['set:html', 'set:text']);

// Per the HTML spec, attribute names must not contain ASCII whitespace, ", ', >, /, or =.
const INVALID_ATTR_NAME_CHAR = /[\s"'>/=]/;

// converts (most) arbitrary strings to valid JS identifiers
const toIdent = (k: string) =>
k.trim().replace(/(?!^)\b\w|\s+|\W+/g, (match, index) => {
Expand Down Expand Up @@ -83,6 +86,11 @@ export function addAttribute(value: any, key: string, shouldEscape = true, tagNa
return '';
}

// Reject attribute names with characters that could break out of the attribute context.
if (INVALID_ATTR_NAME_CHAR.test(key)) {
return '';
}

// compiler directives cannot be applied dynamically, log a warning and ignore.
if (STATIC_DIRECTIVES.has(key)) {
console.warn(`[astro] The "${key}" directive cannot be applied dynamically at runtime. It will not be rendered as an attribute.
Expand Down
57 changes: 57 additions & 0 deletions packages/astro/test/content-collections-image-hmr.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import assert from 'node:assert/strict';
import fs from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { after, before, describe, it } from 'node:test';
import { type DevServer, type Fixture, isWindows, loadFixture } from './test-utils.ts';

const assetsDir = fileURLToPath(
new URL('./fixtures/content-collections-image-hmr/src/assets/', import.meta.url),
);

describe('HMR: Content Collections image url rename test', () => {
let fixture: Fixture;
let devServer: DevServer;

before(async () => {
fixture = await loadFixture({
root: './fixtures/content-collections-image-hmr/',
outDir: './dist/content-collections-image-hmr/',
});
devServer = await fixture.startDevServer();
});

after(async () => {
const renamedPath = path.join(assetsDir, 'shuttle-renamed.jpg');
const originalPath = path.join(assetsDir, 'shuttle.jpg');
if (fs.existsSync(renamedPath) && !fs.existsSync(originalPath)) {
try {
await fs.promises.rename(renamedPath, originalPath);
} catch {
// ignore
}
}
fixture.resetAllFiles();
await devServer.stop();
});

it('should recover after renaming the primary image and updating the markdown reference', {
skip: isWindows,
}, async () => {
await fixture.fetch('/');

const originalImagePath = path.join(assetsDir, 'shuttle.jpg');
const renamedImagePath = path.join(assetsDir, 'shuttle-renamed.jpg');

await fs.promises.rename(originalImagePath, renamedImagePath);
await fixture.editFile('/src/content/blog/post.md', (content) =>
content.replace('shuttle.jpg', 'shuttle-renamed.jpg'),
);
await fixture.onNextDataStoreChange();

const response = await fixture.fetch('/');
assert.equal(response.status, 200);
const html = await response.text();
assert.ok(html.includes('data-image="primary"'));
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { defineConfig } from 'astro/config';

export default defineConfig({});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@test/content-collections-image-hmr",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { defineCollection } from 'astro:content';
import { glob } from 'astro/loaders';
import { z } from 'astro/zod';

const blog = defineCollection({
loader: glob({ pattern: '**/*.md', base: './src/content/blog' }),
schema: ({ image }) =>
z.object({
title: z.string(),
image: image(),
}),
});

export const collections = { blog };
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
title: Test Post
image: ../../assets/shuttle.jpg
---

Post content
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
import { Image } from 'astro:assets';
import { getCollection } from 'astro:content';

const posts = await getCollection('blog');
---

<html>
<head>
<title>Content Collections Image HMR</title>
</head>
<body>
{posts.map((post) => (
<div>
<h2>{post.data.title}</h2>
<Image src={post.data.image} alt={post.data.title} data-image="primary" />
</div>
))}
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"extends": "astro/tsconfigs/strict",
"include": [".astro/types.d.ts", "**/*"],
"exclude": ["dist"]
}
54 changes: 54 additions & 0 deletions packages/astro/test/units/render/html-primitives.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,60 @@ describe('renderElement', () => {
// createComponent/createTestApp — no Vite build needed.
// ---------------------------------------------------------------------------

describe('addAttribute rejects invalid attribute keys', () => {
it('drops keys containing double quotes', () => {
const result = String(addAttribute('val', 'x" onmousemove="alert(1)" y'));
assert.equal(result, '');
});

it('drops keys containing spaces', () => {
const result = String(addAttribute('val', ' onerror=alert(1) '));
assert.equal(result, '');
});

it('drops keys containing >', () => {
const result = String(addAttribute('val', 'x><script>alert(1)</script'));
assert.equal(result, '');
});

it('drops keys containing single quotes', () => {
const result = String(addAttribute('val', "x' onclick='alert(1)"));
assert.equal(result, '');
});

it('drops keys containing =', () => {
const result = String(addAttribute('val', 'x=y'));
assert.equal(result, '');
});

it('allows normal attribute names', () => {
assert.equal(String(addAttribute('v', 'id')), ' id="v"');
assert.equal(String(addAttribute('v', 'data-foo')), ' data-foo="v"');
assert.equal(String(addAttribute('v', 'aria-label')), ' aria-label="v"');
});

it('allows namespaced attributes', () => {
assert.equal(String(addAttribute('v', 'on:click')), ' on:click="v"');
assert.equal(String(addAttribute('v', 'xmlns:happy')), ' xmlns:happy="v"');
});
});

describe('spreadAttributes rejects invalid attribute keys', () => {
it('drops malicious keys while keeping valid ones', () => {
const result = String(
internalSpreadAttributes(
{ class: 'safe', 'x" onclick="alert(1)" y': 'bad', id: 'ok' },
true,
'div',
),
);
assert.ok(result.includes('class="safe"'));
assert.ok(result.includes('id="ok"'));
assert.ok(!result.includes('onclick'));
assert.ok(!result.includes('alert'));
});
});

describe('Correctly serializes boolean attributes (#astro-basic)', async () => {
// h1 data-something and h2 not-data-ok are both empty-string boolean-ish attrs
it('renders empty-value attribute for data-* attr with no value', () => {
Expand Down
13 changes: 7 additions & 6 deletions packages/integrations/netlify/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ export function remotePatternToRegex(

if (hostname) {
if (hostname.startsWith('**.')) {
// match any number of subdomains
regexStr += '([a-z0-9-]+\\.)*';
// match one or more subdomains
regexStr += '([a-z0-9-]+\\.)+';
hostname = hostname.substring(3);
} else if (hostname.startsWith('*.')) {
// match one subdomain
regexStr += '([a-z0-9-]+\\.)?';
// match exactly one subdomain
regexStr += '([a-z0-9-]+\\.)';
hostname = hostname.substring(2); // Remove '*.' from the beginning
}
// Escape dots in the hostname
Expand All @@ -78,8 +78,7 @@ export function remotePatternToRegex(
if (pathname.endsWith('/**')) {
// Match any path.
regexStr += `(\\${pathname.replace('/**', '')}.*)`;
}
if (pathname.endsWith('/*')) {
} else if (pathname.endsWith('/*')) {
// Match one level of path
regexStr += `(\\${pathname.replace('/*', '')}\/[^/?#]+)\/?`;
} else {
Expand All @@ -94,6 +93,8 @@ export function remotePatternToRegex(
// Match query, but only if it's not already matched by the pathname
regexStr += '([?][^#]*)?';
}
// Anchor to end of string so .test() can't match a prefix
regexStr += '$';
try {
// nosemgrep: javascript.lang.security.audit.detect-non-literal-regexp.detect-non-literal-regexp
// This only validates the generated pattern before handing it to Netlify.
Expand Down
11 changes: 10 additions & 1 deletion packages/integrations/netlify/test/functions/image-cdn.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,11 @@ describe('Image CDN', { timeout: 120000 }, () => {

it('generates correct config for remotePatterns', async () => {
const patterns = regexes[2]!;
assert.equal(patterns.test('https://example.org/images/1.jpg'), true, 'should match domain');
assert.equal(
patterns.test('https://example.org/images/1.jpg'),
false,
'apex hostname should not match for *.example.org',
);
assert.equal(
patterns.test('https://www.example.org/images/2.jpg'),
true,
Expand All @@ -105,6 +109,11 @@ describe('Image CDN', { timeout: 120000 }, () => {
false,
'wrong path should not match',
);
assert.equal(
patterns.test('https://www.example.org/images/a/b.jpg'),
false,
'deeper path should not match for /images/*',
);
});

it('warns when remotepatterns generates an invalid regex', async () => {
Expand Down
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading