Skip to content

Commit

Permalink
fix(@angular/build): redirect to path with trailing slash for asset d…
Browse files Browse the repository at this point in the history 
…irectories

Prior to this commit, accessing a static asset directory without a trailing slash resulted in a 404 error. With this change, we now redirect to the path with a trailing slash, aligning with the behavior of express static.

Closes #27949

(cherry picked from commit 9a1c059)
  • Loading branch information
@alan-agius4
alan-agius4 committed Jun 28, 2024
1 parent 67bf901 commit 2d51e86
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 33 deletions.
74 changes: 47 additions & 27 deletions packages/angular/build/src/builders/dev-server/tests/behavior/build-assets_spec.ts
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,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 @@ -146,12 +147,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 @@ -180,6 +180,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>`);
}

0 comments on commit 2d51e86

Please sign in to comment.