Use ctx.request.files instead of ctx.request.body.files (#89)

Use `ctx.request.files` instead of `ctx.request.body.files` - Fixes #75
koajs · Jun 4, 2018 · 73fc2a9 · 73fc2a9
1 parent ff2b222
commit 73fc2a9
Show file tree

Hide file tree

Showing 7 changed files with 137 additions and 101 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,3 +1,3 @@
 language: node_js
 node_js:
-  - "4.0"
+  - "6"
diff --git a/README.md b/README.md
@@ -14,11 +14,16 @@ koa-body [![Build Status](https://travis-ci.org/dlau/koa-body.png)](https://trav
 $ npm install koa-body
 ```
 
-## Koa2 support
+## Legacy Koa v1 support
 ```
-$ npm install koa-body@2
+$ npm install koa-body@3
 ```
 
+## Breaking Changes in v3/4
+To address a potential security issue, the `files` property has been moved to `ctx.request.files`. In prior versions, `files` was a property of `ctx.request.body`. If you do not use multipart uploads, no changes to your code need to be made.
+
+Versions 1 and 2 of `koa-body` are deprecated and replaced with versions 3 and 4, respectively.
+
 ## Features
 - 15 tests
 - can handle three type requests

diff --git a/examples/koa-router.js b/examples/koa-router.js
@@ -49,7 +49,7 @@ router.post('/post/upload',
     log(this.request.body.fields);
     // => {username: ""} - if empty
 
-    log(this.request.body.files);
+    log(this.request.files);
     /* => {uploads: [
             {
               "size": 748831,
@@ -67,7 +67,7 @@ router.post('/post/upload',
             }
           ]}
     */
-   this.body = JSON.stringify(this.request.body, null, 2)
+   this.body = JSON.stringify({ body: this.request.body, files: this.request.files }, null, 2);
    yield next;
   }
 )

diff --git a/examples/multipart.js b/examples/multipart.js
@@ -40,7 +40,7 @@ router.post('/', koaBody,
     console.log(this.request.body.fields);
     // => {username: ""} - if empty
 
-    console.log(this.request.body.files);
+    console.log(this.request.files);
     /* => {uploads: [
             {
               "size": 748831,

diff --git a/index.js b/index.js
@@ -47,23 +47,23 @@ function requestbody(opts) {
   opts.strict = 'strict' in opts ? opts.strict : true;
 
   return function *(next){
-    var body = {};
+    var bodyPromise;
     // so don't parse the body in strict mode
     if (!opts.strict || ["GET", "HEAD", "DELETE"].indexOf(this.method.toUpperCase()) === -1) {
       try {
         if (opts.json && this.is('json'))  {
-          body = yield buddy.json(this, {encoding: opts.encoding, limit: opts.jsonLimit});
+          bodyPromise = buddy.json(this, {encoding: opts.encoding, limit: opts.jsonLimit});
         }
         else if (opts.urlencoded && this.is('urlencoded')) {
-          body = yield buddy.form(this, {encoding: opts.encoding, limit: opts.formLimit, queryString: opts.queryString});
+          bodyPromise = buddy.form(this, {encoding: opts.encoding, limit: opts.formLimit, queryString: opts.queryString});
         }
         else if (opts.text && this.is('text')) {
-          body = yield buddy.text(this, {encoding: opts.encoding, limit: opts.textLimit});
+          bodyPromise = buddy.text(this, {encoding: opts.encoding, limit: opts.textLimit});
         }
         else if (opts.multipart && this.is('multipart')) {
-          body = yield formy(this, opts.formidable);
+          bodyPromise = formy(this, opts.formidable);
         }
-        
+
       } catch(parsingError) {
         if (typeof(opts.onError) === 'function') {
           opts.onError(parsingError, this);
@@ -73,61 +73,94 @@ function requestbody(opts) {
       }
     }
 
-    if (opts.patchNode) {
-      this.req.body = body;
-    }
-    if (opts.patchKoa) {
-      this.request.body = body;
-    }
+    bodyPromise = bodyPromise || Promise.resolve({});
+    bodyPromise = bodyPromise.catch((parsingError) => {
+      if (typeof opts.onError === 'function') {
+        opts.onError(parsingError, this);
+      } else {
+        throw parsingError;
+      }
+    })
+    .then((body) => {
+      if (opts.patchNode) {
+        if (isMultiPart(this, opts)) {
+          this.req.body = body.fields;
+          this.req.files = body.files;
+        } else {
+          this.req.body = body;
+        }
+      }
+      if (opts.patchKoa) {
+        if (isMultiPart(this, opts)) {
+          this.request.body = body.fields;
+          this.request.files = body.files;
+        } else {
+          this.request.body = body;
+        }
+      }
+    });
+
+    yield bodyPromise;
     yield next;
   };
 }
 
+/**
+ * Check if multipart handling is enabled and that this is a multipart request
+ *
+ * @param  {Object} ctx
+ * @param  {Object} opts
+ * @return {Boolean} true if request is multipart and being treated as so
+ * @api private
+ */
+function isMultiPart(ctx, opts) {
+  return opts.multipart && ctx.is('multipart');
+}
+
 /**
  * Donable formidable
  *
  * @param  {Stream} ctx
  * @param  {Object} opts
- * @return {Object}
+ * @return {Promise}
  * @api private
  */
 function formy(ctx, opts) {
-  return function(done) {
+  return new Promise(function (resolve, reject) {
     var fields = {};
     var files = {};
-    var form = new forms.IncomingForm(opts)
-    form
-      .on('end', function() {
-        done(null, {fields: fields, files: files});
-      })
-      .on('error', function(err) {
-        done(err);
-      })
-      .on('field', function(field, value) {
-        if (fields[field]) {
-          if (Array.isArray(fields[field])) {
-            fields[field].push(value);
-          } else {
-            fields[field] = [fields[field], value];
-          }
+    var form = new forms.IncomingForm(opts);
+    form.on('end', function () {
+      return resolve({
+        fields: fields,
+        files: files
+      });
+    }).on('error', function (err) {
+      return reject(err);
+    }).on('field', function (field, value) {
+      if (fields[field]) {
+        if (Array.isArray(fields[field])) {
+          fields[field].push(value);
         } else {
-          fields[field] = value;
+          fields[field] = [fields[field], value];
         }
-      })
-      .on('file', function(field, file) {
-        if (files[field]) {
-          if (Array.isArray(files[field])) {
-            files[field].push(file);
-          } else {
-            files[field] = [files[field], file];
-          }
+      } else {
+        fields[field] = value;
+      }
+    }).on('file', function (field, file) {
+      if (files[field]) {
+        if (Array.isArray(files[field])) {
+          files[field].push(file);
         } else {
-          files[field] = file;
+          files[field] = [files[field], file];
         }
-      });
-    if(opts.onFileBegin) {
+      } else {
+        files[field] = file;
+      }
+    });
+    if (opts.onFileBegin) {
       form.on('fileBegin', opts.onFileBegin);
     }
     form.parse(ctx.req);
-  };
+  });
 }
diff --git a/package.json b/package.json
@@ -4,9 +4,9 @@
   "description": "A koa body parser middleware. Support multipart, urlencoded and json request bodies.",
   "main": "index.js",
   "scripts": {
-    "test": "node_modules/.bin/mocha --harmony",
-    "examples-multer": "node --harmony examples/multer.js",
-    "examples-koa-router": "node --harmony examples/koa-router.js"
+    "test": "node_modules/.bin/mocha",
+    "examples-multer": "node examples/multer.js",
+    "examples-koa-router": "node examples/koa-router.js"
   },
   "author": {
     "name": "Daryl Lau",
@@ -27,15 +27,11 @@
     "form"
   ],
   "files": [
-    ".gitignore",
-    ".npmignore",
-    ".travis.yml",
     "LICENSE",
-    "Makefile",
     "README.md",
-    "example.js",
     "index.js",
-    "test.js"
+    "index.d.ts",
+    "package.json"
   ],
   "dependencies": {
     "co-body": "^5.1.1",