Dont pass in a second arg to buildReq(), and stub out req.file() for …

…virtual requests in the virtual request body parser. Fixes #3656.

lib/router/index.js

@@ -180,16 +180,19 @@ Router.prototype.route = function(req, res) {
     return;
   }
 
-  // Provide access to `sails` object
+  // Provide access to SailsApp instance as `req._sails`.
   req._sails = req._sails || sails;
 
+  // Note that, at this point, `req` and `res` are just dictionaries containing
+  // the properties of each object that have been built up _so far_.
+  //
   // Use base req and res definitions to ensure the specified
   // objects are at least ducktype-able as standard node HTTP
   // req and req streams.
   //
   // Make sure request and response objects have reasonable defaults
   // (will use the supplied definitions if possible)
-  req = buildReq(req, res);
+  req = buildReq(req);
   res = buildRes(req, res);
 
   // console.log('\n\n\n\n=======================\nReceived request to %s %s\nwith req.body:\n',req.method,req.url, req.body);
@@ -296,7 +299,7 @@ Router.prototype.unbind = function(route) {
   // Remove route in internal router
   var newRoutes = [];
   _.each(this._privateRouter.routes[route.method], function(expressRoute) {
-    if (expressRoute.path != route.path) {
+    if (expressRoute.path !== route.path) {
       newRoutes.push(expressRoute);
     }
   });
@@ -403,6 +406,14 @@ function qsParser(req,res,next) {
 }
 // Extremely simple body parser (`req.body`)
 function bodyParser (req, res, next) {
+
+  // Set up a mock `req.file()` clarifying that req.file() is not available
+  // outside of the context of Skipper (i.e. in this case, most commonly from
+  // socket.io virtual requests).
+  req.file = function fileUploadsNotAvailable(){
+    return res.send(500, 'Streaming file uploads via `req.file()` are only available over HTTP with Skipper.');
+  };
+
   var bodyBuffer='';
   if (req.method === 'GET' || req.method === 'HEAD' || req.method === 'DELETE'){
     req.body = _.extend({}, req.body);

lib/router/req.js

@@ -15,6 +15,10 @@ var parseurl = require('parseurl');
  * middleware.  Used by hooks (i.e. sockets) but also for
  * tests-- both at the app-level and in Sails core.
  *
+ * @param {Dictionary} _req
+ *        the properties of this simulated request object that
+ *        have been built up _so far_.
+ *
  * @return {Request} simulated HTTP request object
  * @idempotent
  */
@@ -92,7 +96,19 @@ module.exports = function buildRequest (_req) {
   // Track request start time
   req._startTime = new Date();
 
-  // TODO: add all the other methods in core
+  ////////////////////////////////////////////////////////////////////////////////
+  // Note that other core methods _could_ be added here for use w/ the virtual
+  // router.  But as per convo w/ dougwilson, the same _cannot_ be done for HTTP
+  // requests coming out of Express.  They would either have to (a) rely on modifying
+  // the HTTP request (IncomingMessage) prototype, or (B) rely on context (i.e. `this`),
+  // which would require `_.bind()`-ing them to avoid issues when triggered from
+  // userland code. And re: (B) at that point, the performance impact is effectively
+  // the same as if they were attached on the fly on a per-request basis.
+  //
+  // So we only initially attach `req.*` methods & properties here which are _not_
+  // already built-in to the mock request, and which are _not_ already taken care of
+  // by hooks, AND which don't rely on `res` (because it hasn't been built yet).
+  ////////////////////////////////////////////////////////////////////////////////
 
   // Provide defaults for other request state and methods
   req = defaultsDeep(req, {
@@ -121,9 +137,6 @@ module.exports = function buildRequest (_req) {
       }
 
     },
-    file: function (){
-      return res.send(500, 'Streaming file uploads via `req.file()` are only available over HTTP with Skipper.');
-    },
     wantsJSON: (_req && _req.wantsJSON === false) ? false : true,
     method: 'GET',
     originalUrl: _req.originalUrl || _req.url,