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: normalize module path passed to runfiles helper for robustness #3094

Merged
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
9 changes: 9 additions & 0 deletions internal/runfiles/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ class Runfiles {
}
/** Resolves the given module path. */
resolve(modulePath) {
// Normalize path by converting to forward slashes and removing all trailing
// forward slashes
modulePath = modulePath.replace(/\\/g, '/').replace(/\/+$/g, '');
if (path__default['default'].isAbsolute(modulePath)) {
return modulePath;
}
Expand All @@ -143,13 +146,19 @@ class Runfiles {
}
/** Resolves the given path relative to the current Bazel workspace. */
resolveWorkspaceRelative(modulePath) {
// Normalize path by converting to forward slashes and removing all trailing
// forward slashes
modulePath = modulePath.replace(/\\/g, '/').replace(/\/+$/g, '');
if (!this.workspace) {
throw new Error('workspace could not be determined from the environment; make sure BAZEL_WORKSPACE is set');
}
return this.resolve(path__default['default'].posix.join(this.workspace, modulePath));
}
/** Resolves the given path relative to the current Bazel package. */
resolvePackageRelative(modulePath) {
// Normalize path by converting to forward slashes and removing all trailing
// forward slashes
modulePath = modulePath.replace(/\\/g, '/').replace(/\/+$/g, '');
if (!this.workspace) {
throw new Error('workspace could not be determined from the environment; make sure BAZEL_WORKSPACE is set');
}
Expand Down
9 changes: 9 additions & 0 deletions packages/runfiles/runfiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ export class Runfiles {

/** Resolves the given module path. */
resolve(modulePath: string) {
// Normalize path by converting to forward slashes and removing all trailing
// forward slashes
modulePath = modulePath.replace(/\\/g, '/').replace(/\/+$/g, '')
if (path.isAbsolute(modulePath)) {
return modulePath;
}
Expand All @@ -128,6 +131,9 @@ export class Runfiles {

/** Resolves the given path relative to the current Bazel workspace. */
resolveWorkspaceRelative(modulePath: string) {
// Normalize path by converting to forward slashes and removing all trailing
// forward slashes
modulePath = modulePath.replace(/\\/g, '/').replace(/\/+$/g, '')
if (!this.workspace) {
throw new Error(
'workspace could not be determined from the environment; make sure BAZEL_WORKSPACE is set');
Expand All @@ -137,6 +143,9 @@ export class Runfiles {

/** Resolves the given path relative to the current Bazel package. */
resolvePackageRelative(modulePath: string) {
// Normalize path by converting to forward slashes and removing all trailing
// forward slashes
modulePath = modulePath.replace(/\\/g, '/').replace(/\/+$/g, '')
if (!this.workspace) {
throw new Error(
'workspace could not be determined from the environment; make sure BAZEL_WORKSPACE is set');
Expand Down
34 changes: 33 additions & 1 deletion packages/runfiles/test/runfile_resolution.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const {join} = require('path')
const {join, dirname} = require('path')
const {runfiles} = require('@bazel/runfiles');

describe('runfile resolution', () => {
Expand All @@ -10,6 +10,38 @@ describe('runfile resolution', () => {
expect(normalizePath(testFixturePath)).toEqual(normalizePath(expectedPath),
'Expected the test fixture to be resolved next to the spec source file.');
});

it('should properly resolve with forward slashes', () => {
const testFixturePath = runfiles.resolve('build_bazel_rules_nodejs\\packages\\runfiles\\test\\test_fixture.md');
const expectedPath = join(__dirname, 'test_fixture.md');

expect(normalizePath(testFixturePath)).toEqual(normalizePath(expectedPath),
'Expected the test fixture to be resolved next to the spec source file.');
});

it('should properly resolve a subdirectory of a runfile', () => {
const packagePath = runfiles.resolve('build_bazel_rules_nodejs/packages');
// Alternate with trailing slash
const packagePath2 = runfiles.resolve('build_bazel_rules_nodejs/packages/');
const expectedPath = dirname(dirname(dirname(runfiles.resolve('build_bazel_rules_nodejs/packages/runfiles/test/test_fixture.md.generated_file_suffix'))));

expect(normalizePath(packagePath)).toEqual(normalizePath(expectedPath),
'Expected to resolve a subdirectory of a runfile.');
expect(normalizePath(packagePath2)).toEqual(normalizePath(expectedPath),
'Expected to resolve a subdirectory of a runfile.');
});

it('should properly resolve the workspace root of a runfile', () => {
const packagePath = runfiles.resolve('build_bazel_rules_nodejs');
// Alternate with trailing slash
const packagePath2 = runfiles.resolve('build_bazel_rules_nodejs/');
const expectedPath = dirname(dirname(dirname(dirname(runfiles.resolve('build_bazel_rules_nodejs/packages/runfiles/test/test_fixture.md.generated_file_suffix')))));

expect(normalizePath(packagePath)).toEqual(normalizePath(expectedPath),
'Expected to resolve the workspace root of a runfile.');
expect(normalizePath(packagePath2)).toEqual(normalizePath(expectedPath),
'Expected to resolve the workspace root of a runfile.');
});
});

/**
Expand Down