fix(builtin): strip BOM when parsing package.json (#1453)

Fixes #1448
bazel-contrib · Dec 14, 2019 · c65d9b7 · c65d9b7
1 parent aacd924
commit c65d9b7
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 19 deletions.
diff --git a/internal/npm_install/generate_build_file.js b/internal/npm_install/generate_build_file.js
@@ -102,11 +102,7 @@ package(default_visibility = ["//visibility:public"])
         // generate all BUILD files
         generateBuildFiles(pkgs);
     }
-    module.exports = {
-        main,
-        printPackageBin,
-        printIndexBzl,
-    };
+    exports.main = main;
     /**
      * Generates all build files
      */
@@ -468,7 +464,9 @@ def _maybe(repo_rule, name, **kwargs):
     function parsePackage(p, hide = true) {
         // Parse the package.json file of this package
         const packageJson = path.posix.join(p, 'package.json');
-        const pkg = isFile(packageJson) ? JSON.parse(fs.readFileSync(packageJson, { encoding: 'utf8' })) :
+        const stripBom = (s) => s.charCodeAt(0) === 0xFEFF ? s.slice(1) : s;
+        const pkg = isFile(packageJson) ?
+            JSON.parse(stripBom(fs.readFileSync(packageJson, { encoding: 'utf8' }))) :
             { version: '0.0.0' };
         // Trim the leading node_modules from the path and
         // assign to _dir for future use
@@ -492,6 +490,7 @@ def _maybe(repo_rule, name, **kwargs):
             hideBazelFiles(pkg);
         return pkg;
     }
+    exports.parsePackage = parsePackage;
     /**
      * Check if a bin entry is a non-empty path
      */
@@ -958,6 +957,7 @@ nodejs_binary(
         }
         return result;
     }
+    exports.printPackageBin = printPackageBin;
     function printIndexBzl(pkg) {
         let result = '';
         const executables = _findExecutables(pkg);
@@ -1001,6 +1001,7 @@ def ${name.replace(/-/g, '_')}_test(**kwargs):
         }
         return result;
     }
+    exports.printIndexBzl = printIndexBzl;
     /**
      * Given a scope, return the skylark `node_module_library` target for the scope.
      */

diff --git a/internal/npm_install/generate_build_file.ts b/internal/npm_install/generate_build_file.ts
@@ -90,7 +90,7 @@ function writeFileSync(p: string, content: string) {
 /**
  * Main entrypoint.
  */
-function main() {
+export function main() {
   // find all packages (including packages in nested node_modules)
   const pkgs = findPackages();
 
@@ -104,12 +104,6 @@ function main() {
   generateBuildFiles(pkgs)
 }
 
-module.exports = {
-  main,
-  printPackageBin,
-  printIndexBzl,
-};
-
 /**
  * Generates all build files
  */
@@ -515,11 +509,13 @@ function findScopes() {
  * package json and return it as an object along with
  * some additional internal attributes prefixed with '_'.
  */
-function parsePackage(p: string, hide: boolean = true): Dep {
+export function parsePackage(p: string, hide: boolean = true): Dep {
   // Parse the package.json file of this package
   const packageJson = path.posix.join(p, 'package.json');
-  const pkg = isFile(packageJson) ? JSON.parse(fs.readFileSync(packageJson, {encoding: 'utf8'})) :
-                                    {version: '0.0.0'};
+  const stripBom = (s: string) => s.charCodeAt(0) === 0xFEFF ? s.slice(1) : s;
+  const pkg = isFile(packageJson) ?
+      JSON.parse(stripBom(fs.readFileSync(packageJson, {encoding: 'utf8'}))) :
+      {version: '0.0.0'};
 
   // Trim the leading node_modules from the path and
   // assign to _dir for future use
@@ -1031,7 +1027,7 @@ function additionalAttributes(pkg: Dep, name: string) {
 /**
  * Given a pkg, return the skylark nodejs_binary targets for the package.
  */
-function printPackageBin(pkg: Dep) {
+export function printPackageBin(pkg: Dep) {
   let result = '';
   const executables = _findExecutables(pkg);
   if (executables.size) {
@@ -1058,7 +1054,7 @@ nodejs_binary(
   return result;
 }
 
-function printIndexBzl(pkg: Dep) {
+export function printIndexBzl(pkg: Dep) {
   let result = '';
   const executables = _findExecutables(pkg);
   if (executables.size) {

diff --git a/internal/npm_install/test/generate_build_file.spec.js b/internal/npm_install/test/generate_build_file.spec.js
@@ -1,5 +1,7 @@
+const fs = require('fs');
+const path = require('path');
 const {check, files} = require('./check');
-const {printPackageBin, printIndexBzl} = require('../generate_build_file');
+const {parsePackage, printPackageBin, printIndexBzl} = require('../generate_build_file');
 
 describe('build file generator', () => {
   describe('integration test', () => {
@@ -10,6 +12,14 @@ describe('build file generator', () => {
     });
   });
 
+  describe('parsing package.json', () => {
+    it('should strip leading Byte-Order Mark character', () => {
+      const pkgPath = path.join(process.env['TEST_TMPDIR'], 'package.json');
+      fs.writeFileSync(pkgPath, `\uFEFF{"name": "foo"}`, 'utf-8');
+      expect(parsePackage(path.dirname(pkgPath)).name).toBe('foo');
+    });
+  });
+
   describe('should exclude nodejs_binary rules when', () => {
     const pkg = {_name: 'some_name', _dir: 'some_dir', _dependencies: [], _files: []};