Fix for #322 - require wrapper now serves only to check against depen…

…dency whitelist

package.json

@@ -72,6 +72,7 @@
     "opn": "4.0.2",
     "parse-author": "1.0.0",
     "read": "1.0.7",
+    "require-package-name": "2.0.1",
     "semver": "5.1.1",
     "stringformat": "0.0.5",
     "targz": "1.0.1",

src/cli/domain/package-server-script.js

@@ -7,6 +7,7 @@ var path = require('path');
 var uglifyJs = require('uglify-js');
 var falafel = require('falafel');
 var _ = require('underscore');
+var requirePackageName = require('require-package-name');
 
 var config = require('../../resources/settings');
 var hashBuilder = require('../../utils/hash-builder');
@@ -75,7 +76,8 @@ var getLocalDependencies = function(componentPath, serverContent, fileName){
     if(isLocalFile(required)) {
       requires.files[required] = getRequiredContent(componentPath, required);
     } else {
-      requires.modules.push(required);
+      var packageName = requirePackageName(required);
+      requires.modules.push(packageName);
     }
   });
 

src/registry/domain/options-sanitiser.js

@@ -3,7 +3,6 @@
 var express = require('express');
 var _ = require('underscore');
 
-var dependenciesResolver = require('./dependencies-resolver');
 var settings = require('../../resources/settings');
 var auth = require('./authentication');
 
@@ -30,10 +29,6 @@ module.exports = function(input){
     options.tempDir = settings.registry.defaultTempPath;
   }
 
-  if(!!options.dependencies){
-    options.dependencies = dependenciesResolver(options);
-  }
-
   if(!_.isBoolean(options.hotReloading)){
     options.hotReloading = !!options.local;
   }

src/registry/domain/require-wrapper.js

@@ -1,18 +1,33 @@
 'use strict';
 
+var path = require('path');
+var fs = require('fs-extra');
 var _ = require('underscore');
+var requirePackageName = require('require-package-name');
 
 var strings = require('../../resources');
 
 module.exports = function(injectedDependencies){
-  return function(moduleName){
-    if(!!injectedDependencies && _.has(injectedDependencies, moduleName)){
-      return injectedDependencies[moduleName];
-    } else {
+  return function(requirePath){
+    var moduleName = requirePackageName(requirePath);
+
+    if(!_.contains(injectedDependencies, moduleName)){
       throw {
         code: strings.errors.registry.DEPENDENCY_NOT_FOUND_CODE,
         missing: [moduleName]
-      };
+      }; 
+    }
+
+    var nodeModulesPath = path.resolve('.', 'node_modules');
+    var modulePath = path.resolve(nodeModulesPath, requirePath);
+
+    try {
+      return require(modulePath);
+    } catch (e) {
+      throw {
+        code: strings.errors.registry.DEPENDENCY_NOT_FOUND_CODE,
+        missing: [modulePath]
+      }; 
     }
   };
-};
+};

src/resources/index.js

@@ -27,7 +27,6 @@ module.exports = {
       COMPONENT_VERSION_NOT_VALID: 'Version "{0}" is not a valid semantic version.',
       COMPONENT_VERSION_NOT_VALID_CODE: 'version_not_valid',
       CONFIGURATION_DEPENDENCIES_MUST_BE_ARRAY: 'Registry configuration is not valid: dependencies must be an array',
-      CONFIGURATION_A_DEPENDENCY_NOT_FOUND: 'Registry configuration is not valid: a dependency is not valid.',
       CONFIGURATION_EMPTY: 'Registry configuration is empty',
       CONFIGURATION_ONREQUEST_MUST_BE_FUNCTION: 'Registry configuration is not valid: registry.on\'s callback must be a function',
       CONFIGURATION_PUBLISH_BASIC_AUTH_CREDENTIALS_MISSING: 'Registry configuration is not valid: basic auth requires username and password',
@@ -44,7 +43,6 @@ module.exports = {
       LOCAL_PUBLISH_NOT_ALLOWED: 'Components can\'t be published to local repository',
       LOCAL_PUBLISH_NOT_ALLOWED_CODE: 'not_allowed',
       GENERIC_ERROR: 'error!',
-      GENERIC_NOT_FOUND: 'not found!',
       MANDATORY_PARAMETER_MISSING: 'Expected mandatory parameters are missing: {0}',
       MANDATORY_PARAMETER_MISSING_CODE: 'missing',
       NESTED_RENDERER_CALLBACK_IS_NOT_VALID: 'callback is not valid',
@@ -120,9 +118,6 @@ module.exports = {
       REGISTRY_STARTING: 'Starting dev registry on {0} ...',
       RETRYING_10_SECONDS: 'Retrying in 10 seconds...',
       SCANNING_COMPONENTS: 'Looking for components...'
-    },
-    registry: {
-      RESOLVING_DEPENDENCIES: 'Resolving dependencies...'
     }
   }
 };

test/unit/cli-domain-package-server-script.js

@@ -127,7 +127,7 @@ describe('cli : domain : package-server-script', function(){
       });
     });
 
-    describe('when component requires a module', function(){
+    describe('when component requires an npm module', function(){
 
       var error,
           serverjs = 'var data=require(\'request\');module.exports.data=function(context,cb){return cb(null,data); };';
@@ -155,6 +155,34 @@ describe('cli : domain : package-server-script', function(){
       });
     });
 
+    describe('when component requires a relative path from an npm module', function(){
+
+      var error,
+          serverjs = 'var data=require(\'react-dom/server\');module.exports.data=function(context,cb){return cb(null,data); };';
+
+      beforeEach(function(done){
+
+        initialise({ readFileSync: sinon.stub().returns(serverjs) });
+
+        packageServerScript({
+          componentPath: '/path/to/component/',
+          ocOptions: {
+            files: {
+              data: 'server.js'
+            }
+          },
+          publishPath: '/path/to/component/_package/'
+        }, function(e, r){
+          error = e;
+          done();
+        });
+      });
+
+      it('should throw an error when the dependency is not present in the package.json', function(){
+        expect(error.toString()).to.equal('Error: Missing dependencies from package.json => ["react-dom"]');
+      });
+    });
+
     describe('when component requires a js file', function(){
 
       var serverjs = 'var data=require(\'./hi.js\');module.exports.data=function(context,cb){return cb(null,data); };',

test/unit/registry-domain-require-wrapper.js

@@ -9,16 +9,31 @@ describe('registry : domain : require-wrapper', function(){
 
   describe('when using the require wrapper in a clear context', function(){
 
-    describe('when injecting a dependency', function(){
-
-      var dependencies = {
-        'some-module': {
-          someFunction: function(name){
-            return 'hello ' + name;
-          }
-        }
+    describe('when requiring a dependency', function(){
+
+      var dependencies = [
+        'underscore'
+      ];
+
+      var context = {
+        require: new RequireWrapper(dependencies),
+        result: null
       };
 
+      var script = 'var _ = require(\'underscore\');\n' +
+                   'result = _.first([5, 4, 3, 2, 1]);';
+
+      vm.runInNewContext(script, context);
+
+      it('should correctly make the dependency require-able', function(){
+        expect(context.result).to.eql(5);
+      });
+    });
+
+    describe('when requiring an unrecognised dependency', function(){
+
+      var dependencies = [];
+
       var context = {
         require: new RequireWrapper(dependencies),
         result: null
@@ -27,24 +42,50 @@ describe('registry : domain : require-wrapper', function(){
       var script = 'var someModule = require(\'some-module\');\n' +
                    'result = someModule.someFunction(\'John Doe\');';
 
+      it('should correctly throw an error', function(){
+        expect(function(){
+          return vm.runInNewContext(script, context);
+        }).to.throw({
+          code: 'DEPENDENCY_MISSING_FROM_REGISTRY',
+          missing: ['someModule']
+        });
+      });
+    });
+
+    describe('when requiring a dependency with a relative path', function(){
+
+      var dependencies = [
+        'underscore'
+      ];
+
+      var context = {
+        require: new RequireWrapper(dependencies),
+        result: null
+      };
+
+      var script = 'var _ = require(\'underscore/underscore\');\n' +
+                   'result = _.first([5, 4, 3, 2, 1]);';
+
       vm.runInNewContext(script, context);
 
       it('should correctly make the dependency require-able', function(){
-        expect(context.result).to.eql('hello John Doe');
+        expect(context.result).to.eql(5);
       });
     });
 
-    describe('when requiring un-injected dependency', function(){
+    describe('when requiring a dependency with a relative path that does not exist', function(){
 
-      var dependencies = {};
+      var dependencies = [
+        'underscore'
+      ];
 
       var context = {
         require: new RequireWrapper(dependencies),
         result: null
       };
 
-      var script = 'var someModule = require(\'some-module\');\n' +
-                   'result = someModule.someFunction(\'John Doe\');';
+      var script = 'var _ = require(\'underscore/foo\');\n' +
+                   'result = _.first([5, 4, 3, 2, 1]);';
 
       it('should correctly throw an error', function(){
         expect(function(){

test/unit/registry-routes-component.js

@@ -356,9 +356,9 @@ describe('registry : routes : component', function(){
             local: true, //needed to invalidate the cache
             baseUrl: 'http://components.com/',
             plugins: {},
-            dependencies: {
-              underscore: require('underscore')
-            }
+            dependencies: [
+              'underscore'
+            ]
           },
           json: resJsonStub
         });