fix: normalize module path passed to runfiles helper for robustness

bazel-contrib · Nov 22, 2021 · 734a24c · 734a24c
1 parent 85094e4
commit 734a24c
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 1 deletion.
diff --git a/internal/runfiles/index.js b/internal/runfiles/index.js
@@ -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;
         }
@@ -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');
         }

diff --git a/packages/runfiles/runfiles.ts b/packages/runfiles/runfiles.ts
@@ -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;
     }
@@ -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');
@@ -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');

diff --git a/packages/runfiles/test/runfile_resolution.spec.js b/packages/runfiles/test/runfile_resolution.spec.js
@@ -1,4 +1,4 @@
-const {join} = require('path')
+const {join, dirname} = require('path')
 const {runfiles} = require('@bazel/runfiles');
 
 describe('runfile resolution', () => {
@@ -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.');
+  });
 });
 
 /**