deps: on-finished@2.4.1

expressjs · Apr 3, 2022 · 2e2d78c · 2e2d78c
1 parent 980d881
commit 2e2d78c
Show file tree

Hide file tree

Showing 4 changed files with 205 additions and 1 deletion.
diff --git a/History.md b/History.md
@@ -26,6 +26,8 @@ unreleased
     - Remove set content headers that break response
     - deps: on-finished@2.4.1
     - deps: statuses@2.0.1
+  * deps: on-finished@2.4.1
+    - Prevent loss of async hooks context
   * deps: qs@6.10.3
   * deps: send@0.18.0
     - Fix emitted 416 error missing headers property

diff --git a/package.json b/package.json
@@ -45,7 +45,7 @@
     "http-errors": "2.0.0",
     "merge-descriptors": "1.0.1",
     "methods": "~1.1.2",
-    "on-finished": "~2.3.0",
+    "on-finished": "2.4.1",
     "parseurl": "~1.3.3",
     "path-to-regexp": "0.1.7",
     "proxy-addr": "~2.0.7",

diff --git a/test/res.download.js b/test/res.download.js
@@ -1,6 +1,8 @@
 'use strict'
 
 var after = require('after');
+var assert = require('assert')
+var asyncHooks = tryRequire('async_hooks')
 var Buffer = require('safe-buffer').Buffer
 var express = require('..');
 var path = require('path')
@@ -9,6 +11,10 @@ var utils = require('./support/utils')
 
 var FIXTURES_PATH = path.join(__dirname, 'fixtures')
 
+var describeAsyncHooks = typeof asyncHooks.AsyncLocalStorage === 'function'
+  ? describe
+  : describe.skip
+
 describe('res', function(){
   describe('.download(path)', function(){
     it('should transfer as an attachment', function(done){
@@ -84,6 +90,65 @@ describe('res', function(){
       .expect('Content-Disposition', 'attachment; filename="user.html"')
       .expect(200, cb);
     })
+
+    describeAsyncHooks('async local storage', function () {
+      it('should presist store', function (done) {
+        var app = express()
+        var cb = after(2, done)
+        var store = { foo: 'bar' }
+
+        app.use(function (req, res, next) {
+          req.asyncLocalStorage = new asyncHooks.AsyncLocalStorage()
+          req.asyncLocalStorage.run(store, next)
+        })
+
+        app.use(function (req, res) {
+          res.download('test/fixtures/name.txt', function (err) {
+            if (err) return cb(err)
+
+            var local = req.asyncLocalStorage.getStore()
+
+            assert.strictEqual(local.foo, 'bar')
+            cb()
+          })
+        })
+
+        request(app)
+          .get('/')
+          .expect('Content-Type', 'text/plain; charset=UTF-8')
+          .expect('Content-Disposition', 'attachment; filename="name.txt"')
+          .expect(200, 'tobi', cb)
+      })
+
+      it('should presist store on error', function (done) {
+        var app = express()
+        var store = { foo: 'bar' }
+
+        app.use(function (req, res, next) {
+          req.asyncLocalStorage = new asyncHooks.AsyncLocalStorage()
+          req.asyncLocalStorage.run(store, next)
+        })
+
+        app.use(function (req, res) {
+          res.download('test/fixtures/does-not-exist', function (err) {
+            var local = req.asyncLocalStorage.getStore()
+
+            if (local) {
+              res.setHeader('x-store-foo', String(local.foo))
+            }
+
+            res.send(err ? 'got ' + err.status + ' error' : 'no error')
+          })
+        })
+
+        request(app)
+          .get('/')
+          .expect(200)
+          .expect('x-store-foo', 'bar')
+          .expect('got 404 error')
+          .end(done)
+      })
+    })
   })
 
   describe('.download(path, options)', function () {
@@ -423,3 +488,11 @@ describe('res', function(){
     })
   })
 })
+
+function tryRequire (name) {
+  try {
+    return require(name)
+  } catch (e) {
+    return {}
+  }
+}
diff --git a/test/res.sendFile.js b/test/res.sendFile.js
@@ -1,6 +1,7 @@
 'use strict'
 
 var after = require('after');
+var asyncHooks = tryRequire('async_hooks')
 var Buffer = require('safe-buffer').Buffer
 var express = require('../')
   , request = require('supertest')
@@ -10,6 +11,10 @@ var path = require('path');
 var fixtures = path.join(__dirname, 'fixtures');
 var utils = require('./support/utils');
 
+var describeAsyncHooks = typeof asyncHooks.AsyncLocalStorage === 'function'
+  ? describe
+  : describe.skip
+
 describe('res', function(){
   describe('.sendFile(path)', function () {
     it('should error missing path', function (done) {
@@ -261,6 +266,64 @@ describe('res', function(){
         .get('/')
         .expect(200, 'got 404 error', done)
     })
+
+    describeAsyncHooks('async local storage', function () {
+      it('should presist store', function (done) {
+        var app = express()
+        var cb = after(2, done)
+        var store = { foo: 'bar' }
+
+        app.use(function (req, res, next) {
+          req.asyncLocalStorage = new asyncHooks.AsyncLocalStorage()
+          req.asyncLocalStorage.run(store, next)
+        })
+
+        app.use(function (req, res) {
+          res.sendFile(path.resolve(fixtures, 'name.txt'), function (err) {
+            if (err) return cb(err)
+
+            var local = req.asyncLocalStorage.getStore()
+
+            assert.strictEqual(local.foo, 'bar')
+            cb()
+          })
+        })
+
+        request(app)
+          .get('/')
+          .expect('Content-Type', 'text/plain; charset=UTF-8')
+          .expect(200, 'tobi', cb)
+      })
+
+      it('should presist store on error', function (done) {
+        var app = express()
+        var store = { foo: 'bar' }
+
+        app.use(function (req, res, next) {
+          req.asyncLocalStorage = new asyncHooks.AsyncLocalStorage()
+          req.asyncLocalStorage.run(store, next)
+        })
+
+        app.use(function (req, res) {
+          res.sendFile(path.resolve(fixtures, 'does-not-exist'), function (err) {
+            var local = req.asyncLocalStorage.getStore()
+
+            if (local) {
+              res.setHeader('x-store-foo', String(local.foo))
+            }
+
+            res.send(err ? 'got ' + err.status + ' error' : 'no error')
+          })
+        })
+
+        request(app)
+          .get('/')
+          .expect(200)
+          .expect('x-store-foo', 'bar')
+          .expect('got 404 error')
+          .end(done)
+      })
+    })
   })
 
   describe('.sendFile(path, options)', function () {
@@ -999,6 +1062,64 @@ describe('res', function(){
       .get('/')
       .end(function(){});
     })
+
+    describeAsyncHooks('async local storage', function () {
+      it('should presist store', function (done) {
+        var app = express()
+        var cb = after(2, done)
+        var store = { foo: 'bar' }
+
+        app.use(function (req, res, next) {
+          req.asyncLocalStorage = new asyncHooks.AsyncLocalStorage()
+          req.asyncLocalStorage.run(store, next)
+        })
+
+        app.use(function (req, res) {
+          res.sendfile('test/fixtures/name.txt', function (err) {
+            if (err) return cb(err)
+
+            var local = req.asyncLocalStorage.getStore()
+
+            assert.strictEqual(local.foo, 'bar')
+            cb()
+          })
+        })
+
+        request(app)
+          .get('/')
+          .expect('Content-Type', 'text/plain; charset=UTF-8')
+          .expect(200, 'tobi', cb)
+      })
+
+      it('should presist store on error', function (done) {
+        var app = express()
+        var store = { foo: 'bar' }
+
+        app.use(function (req, res, next) {
+          req.asyncLocalStorage = new asyncHooks.AsyncLocalStorage()
+          req.asyncLocalStorage.run(store, next)
+        })
+
+        app.use(function (req, res) {
+          res.sendfile('test/fixtures/does-not-exist', function (err) {
+            var local = req.asyncLocalStorage.getStore()
+
+            if (local) {
+              res.setHeader('x-store-foo', String(local.foo))
+            }
+
+            res.send(err ? 'got ' + err.status + ' error' : 'no error')
+          })
+        })
+
+        request(app)
+          .get('/')
+          .expect(200)
+          .expect('x-store-foo', 'bar')
+          .expect('got 404 error')
+          .end(done)
+      })
+    })
   })
 
   describe('.sendfile(path)', function(){
@@ -1280,3 +1401,11 @@ function createApp(path, options, fn) {
 
   return app;
 }
+
+function tryRequire (name) {
+  try {
+    return require(name)
+  } catch (e) {
+    return {}
+  }
+}