Skip to content

fix(@angular/build): redirect to path with trailing slash for asset directories #27958

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

Merged
merged 1 commit into from
Jun 28, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { executeOnceAndFetch } from '../execute-fetch';
import { describeServeBuilder } from '../jasmine-helpers';
import { BASE_OPTIONS, DEV_SERVER_BUILDER_INFO } from '../setup';

describeServeBuilder(executeDevServer, DEV_SERVER_BUILDER_INFO, (harness, setupTarget, isVite) => {
describeServeBuilder(executeDevServer, DEV_SERVER_BUILDER_INFO, (harness, setupTarget) => {
const javascriptFileContent =
"import {foo} from 'unresolved'; /* a comment */const foo = `bar`;\n\n\n";

Expand Down Expand Up @@ -95,31 +95,51 @@ describeServeBuilder(executeDevServer, DEV_SERVER_BUILDER_INFO, (harness, setupT
expect(await response?.text()).toContain('<h1>Login page</h1>');
});

(isVite ? it : xit)(
`should return the asset that matches '.html' when path has no trailing '/'`,
async () => {
await harness.writeFile(
'src/login/new.html',
'<html><body><h1>Login page</h1></body><html>',
);

setupTarget(harness, {
assets: ['src/login'],
optimization: {
scripts: true,
},
});

harness.useTarget('serve', {
...BASE_OPTIONS,
});

const { result, response } = await executeOnceAndFetch(harness, 'login/new');

expect(result?.success).toBeTrue();
expect(await response?.status).toBe(200);
expect(await response?.text()).toContain('<h1>Login page</h1>');
},
);
it(`should return the asset that matches '.html' when path has no trailing '/'`, async () => {
await harness.writeFile('src/login/new.html', '<html><body><h1>Login page</h1></body><html>');

setupTarget(harness, {
assets: ['src/login'],
optimization: {
scripts: true,
},
});

harness.useTarget('serve', {
...BASE_OPTIONS,
});

const { result, response } = await executeOnceAndFetch(harness, 'login/new');

expect(result?.success).toBeTrue();
expect(await response?.status).toBe(200);
expect(await response?.text()).toContain('<h1>Login page</h1>');
});

it(`should return a redirect when an asset directory is accessed without a trailing '/'`, async () => {
await harness.writeFile(
'src/login/index.html',
'<html><body><h1>Login page</h1></body><html>',
);

setupTarget(harness, {
assets: ['src/login'],
optimization: {
scripts: true,
},
});

harness.useTarget('serve', {
...BASE_OPTIONS,
});

const { result, response } = await executeOnceAndFetch(harness, 'login', {
request: { redirect: 'manual' },
});

expect(result?.success).toBeTrue();
expect(await response?.status).toBe(301);
expect(await response?.headers.get('Location')).toBe('/login/');
});
});
});
42 changes: 36 additions & 6 deletions packages/angular/build/src/tools/vite/angular-memory-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export function createAngularMemoryPlugin(options: AngularMemoryPluginOptions):
// The base of the URL is unused but required to parse the URL.
const pathname = pathnameWithoutBasePath(req.url, server.config.base);
const extension = extname(pathname);
const pathnameHasTrailingSlash = pathname[pathname.length - 1] === '/';

// Rewrite all build assets to a vite raw fs URL
const assetSourcePath = assets.get(pathname);
Expand All @@ -141,12 +142,11 @@ export function createAngularMemoryPlugin(options: AngularMemoryPluginOptions):
// HTML fallbacking
// This matches what happens in the vite html fallback middleware.
// ref: https://github.com/vitejs/vite/blob/main/packages/vite/src/node/server/middlewares/htmlFallback.ts#L9
const htmlAssetSourcePath =
pathname[pathname.length - 1] === '/'
? // Trailing slash check for `index.html`.
assets.get(pathname + 'index.html')
: // Non-trailing slash check for fallback `.html`
assets.get(pathname + '.html');
const htmlAssetSourcePath = pathnameHasTrailingSlash
? // Trailing slash check for `index.html`.
assets.get(pathname + 'index.html')
: // Non-trailing slash check for fallback `.html`
assets.get(pathname + '.html');

if (htmlAssetSourcePath) {
req.url = `${server.config.base}@fs/${encodeURI(htmlAssetSourcePath)}`;
Expand Down Expand Up @@ -175,6 +175,19 @@ export function createAngularMemoryPlugin(options: AngularMemoryPluginOptions):
}
}

// If the path has no trailing slash and it matches a servable directory redirect to the same path with slash.
// This matches the default express static behaviour.
// See: https://github.com/expressjs/serve-static/blob/89fc94567fae632718a2157206c52654680e9d01/index.js#L182
if (!pathnameHasTrailingSlash) {
for (const assetPath of assets.keys()) {
if (pathname === assetPath.substring(0, assetPath.lastIndexOf('/'))) {
redirect(res, req.url + '/');

return;
}
}
}

next();
});

Expand Down Expand Up @@ -362,3 +375,20 @@ function lookupMimeTypeFromRequest(url: string): string | undefined {

return extension && lookupMimeType(extension);
}

function redirect(res: ServerResponse, location: string): void {
res.statusCode = 301;
res.setHeader('Content-Type', 'text/html');
res.setHeader('Location', location);
res.end(`
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Redirecting</title>
</head>
<body>
<pre>Redirecting to <a href="${location}">${location}</a></pre>
</body>
</html>`);
}