test: improve code coverage (#12)

app.js

@@ -17,8 +17,7 @@ module.exports = app => {
     checkExt = options.whitelist;
   } else if (Array.isArray(options.whitelist)) {
     options.whitelist = options.whitelist.map(extname => extname.toLowerCase());
-
-    checkExt = filename => options.whitelist.includes(path.extname(filename).toLowerCase() || '');
+    checkExt = filename => options.whitelist.includes(path.extname(filename).toLowerCase());
   } else {
     // default extname whitelist
     const whitelist = [
@@ -49,7 +48,7 @@ module.exports = app => {
       .concat(options.fileExtensions || [])
       .map(extname => extname.toLowerCase());
 
-    checkExt = filename => whitelist.includes(path.extname(filename).toLowerCase() || '');
+    checkExt = filename => whitelist.includes(path.extname(filename).toLowerCase());
   }
 
   // https://github.com/mscdex/busboy#busboy-methods

app/extend/context.js

@@ -47,6 +47,7 @@ module.exports = {
         err.name = 'MultipartFileTooLargeError';
         err.status = 413;
         err.fields = stream.fields;
+        err.filename = stream.filename;
         if (stream.listenerCount('error') > 0) {
           stream.emit('error', err);
           ctx.coreLogger.warn(err);

package.json

@@ -54,7 +54,7 @@
     "egg-bin": "^4.0.4",
     "egg-ci": "^1.8.0",
     "egg-mock": "^3.8.0",
-    "eslint": "^3.19.0",
+    "eslint": "^4.1.0",
     "eslint-config-egg": "^4.2.1",
     "formstream": "^1.1.0",
     "is-type-of": "^1.0.0",

test/fixtures/apps/upload-limit/app/router.js

@@ -16,10 +16,13 @@ module.exports = app => {
         const writeStream = fs.createWriteStream(storefile);
         stream.pipe(writeStream);
 
-        stream.on('error', err => {
-          console.log('read stream error: %s', err);
-          reject(err);
-        });
+        if (!name.includes('not-handle-error-event')) {
+          stream.on('error', err => {
+            console.log('read stream error: %s', err);
+            reject(err);
+          });
+        }
+
         writeStream.on('error', err => {
           console.log('write stream error: %s', err);
           reject(err);
@@ -46,6 +49,9 @@ module.exports = app => {
     const stream = yield this.getFileStream();
     const name = 'egg-multipart-test/' + process.version + '-' + Date.now() + '-' + path.basename(stream.filename);
     const result = yield this.oss.put(name, stream);
+    if (name.includes('not-handle-error-event-and-mock-stream-error')) {
+      process.nextTick(() => stream.emit('error', new Error('mock stream unhandle error')));
+    }
     this.body = {
       name: result.name,
       url: result.url,

test/fixtures/apps/upload-limit/config/config.default.js

@@ -4,4 +4,5 @@ exports.keys = 'multipart';
 
 exports.multipart = {
   fileSize: '1mb',
+  fileExtensions: null,
 };

test/fixtures/apps/whitelist-function/config/config.default.js

@@ -2,7 +2,11 @@
 
 exports.multipart = {
   fileExtensions: [ '.foo' ],
-  whitelist: filename => filename === 'bar',
+  whitelist(filename) {
+    if (filename === 'bar') return true;
+    if (filename === 'error') throw new Error('mock checkExt error');
+    return false;
+  }
 };
 
 exports.keys = 'multipart';

test/multipart.test.js

@@ -1,7 +1,6 @@
 'use strict';
 
 const assert = require('assert');
-const request = require('supertest');
 const formstream = require('formstream');
 const urllib = require('urllib');
 const path = require('path');
@@ -295,6 +294,21 @@ describe('test/multipart.test.js', () => {
       const data = JSON.parse(res.data);
       assert(data.message === 'Invalid filename: foo.png');
     });
+
+    it('should throw 400 when whitelist function throw error', function* () {
+      const form = formstream();
+      form.file('file', __filename, 'error');
+      const headers = form.headers();
+      const res = yield urllib.request(host + '/upload.json', {
+        method: 'POST',
+        headers,
+        stream: form,
+      });
+
+      assert(res.status === 400);
+      const data = JSON.parse(res.data);
+      assert(data.message === 'mock checkExt error');
+    });
   });
 
   describe('upload one file', () => {
@@ -310,7 +324,7 @@ describe('test/multipart.test.js', () => {
     before(function* () {
       server = app.listen();
       host = 'http://127.0.0.1:' + server.address().port;
-      yield request(server)
+      yield app.httpRequest()
         .get('/upload')
         .expect(200);
     });
@@ -436,7 +450,7 @@ describe('test/multipart.test.js', () => {
       yield fs.writeFile(bigfile, new Buffer(1024 * 1024 * 2));
       server = app.listen();
       host = 'http://127.0.0.1:' + server.address().port;
-      yield request(server)
+      yield app.httpRequest()
         .get('/upload')
         .expect(200);
     });
@@ -470,5 +484,53 @@ describe('test/multipart.test.js', () => {
       const content = yield fs.readFile(coreLogPath, 'utf8');
       assert(content.includes('nodejs.MultipartFileTooLargeError: Request file too large'));
     });
+
+    it('should ignore error when stream not handle error event', function* () {
+      const form = formstream();
+      form.field('foo', 'bar').field('[', 'toString').field(']', 'toString');
+      form.file('file', bigfile, 'not-handle-error-event.js');
+
+      const headers = form.headers();
+      const url = host + '/upload';
+      const res = yield urllib.request(url, {
+        method: 'POST',
+        headers,
+        stream: form,
+        dataType: 'json',
+      });
+
+      const data = res.data;
+      assert(res.status === 200);
+      assert(data.url);
+
+      const coreLogPath = path.join(app.baseDir, 'logs/oss/common-error.log');
+      const content = yield fs.readFile(coreLogPath, 'utf8');
+      assert(content.includes('nodejs.MultipartFileTooLargeError: Request file too large'));
+      assert(content.includes("filename: 'not-handle-error-event.js'"));
+    });
+
+    it('should ignore stream next errors after limit event fire', function* () {
+      const form = formstream();
+      form.field('foo', 'bar').field('[', 'toString').field(']', 'toString');
+      form.file('file', bigfile, 'not-handle-error-event-and-mock-stream-error.js');
+
+      const headers = form.headers();
+      const url = host + '/upload';
+      const res = yield urllib.request(url, {
+        method: 'POST',
+        headers,
+        stream: form,
+        dataType: 'json',
+      });
+
+      const data = res.data;
+      assert(res.status === 200);
+      assert(data.url);
+
+      const coreLogPath = path.join(app.baseDir, 'logs/oss/common-error.log');
+      const content = yield fs.readFile(coreLogPath, 'utf8');
+      assert(content.includes('nodejs.MultipartFileTooLargeError: Request file too large'));
+      assert(content.includes("filename: 'not-handle-error-event-and-mock-stream-error.js'"));
+    });
   });
 });