From 6e37feab4b2bf1aed2d78e088497e3c47ac5dede Mon Sep 17 00:00:00 2001 From: Stephen Davies Date: Tue, 14 May 2024 05:58:50 +1000 Subject: [PATCH 1/5] Fix upstream proxied POST request being aborted early on Node 16+ Due to change in meaning of 'close' event, which was changed to match the Stream interface instead of events of the socket --- CHANGES.md | 4 ++++ lib/controllers/proxy.js | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 88af57b4..dce35c42 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,9 @@ ### Next version +### 4.0.1 + +* Fixed proxied upstream POST request being aborted when the stream associated with the downstream request is closed on Node v16+. This will now again correctly be triggered only when the socket is closed early. + ### 4.0.0 * Removed conversion service (no longer used in TerriaJS 8+). diff --git a/lib/controllers/proxy.js b/lib/controllers/proxy.js index 93ed1ec5..595aa5c4 100644 --- a/lib/controllers/proxy.js +++ b/lib/controllers/proxy.js @@ -198,8 +198,8 @@ module.exports = function(options) { // encoding : null means "body" passed to the callback will be raw bytes var proxiedRequest; - req.on('close', function() { - if (proxiedRequest) { + req.socket.once('close', function() { + if (proxiedRequest && !res.writableEnded) { proxiedRequest.abort(); } }); From dcd4824c1fcc8228597aba43dcf1eef2dc9e168e Mon Sep 17 00:00:00 2001 From: Stephen Davies Date: Tue, 14 May 2024 05:59:55 +1000 Subject: [PATCH 2/5] Include node 16 in tests --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5bb3a818..dffc0bb7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,7 +18,7 @@ jobs: strategy: fail-fast: false matrix: - node-version: [18.x, 20.x, 21.x] + node-version: [16.x, 18.x, 20.x, 22.x] steps: - uses: actions/checkout@v4 From a7e8cbd10f3f7a183157bd3cf7398a81a6d6d438 Mon Sep 17 00:00:00 2001 From: Stephen Davies Date: Tue, 14 May 2024 06:00:01 +1000 Subject: [PATCH 3/5] Fix warning --- spec/proxy.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/proxy.spec.js b/spec/proxy.spec.js index 57347c17..67a031b4 100644 --- a/spec/proxy.spec.js +++ b/spec/proxy.spec.js @@ -228,7 +228,7 @@ describe('proxy', function() { request(buildApp(openProxyOptions)) [methodName]('/_2h/example.com') .set('Cache-Control', 'no-cache') - .set('x-give-response-status', '500') + .set('x-give-response-status', 500) .expect(500) .expect('cache-control', 'no-cache') .end(assert(done)); From 248e00fef0f5fc6e0401ceaae6a43a6282b3c9e0 Mon Sep 17 00:00:00 2001 From: Stephen Davies Date: Tue, 14 May 2024 14:25:12 +1000 Subject: [PATCH 4/5] More CI --- .github/workflows/ci.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dffc0bb7..0f277503 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,10 +1,6 @@ name: CI -on: - push: - branches: [ "master" ] - pull_request: - branches: [ "master" ] +on: [push, pull_request] # When a PR is updated, cancel the jobs from the previous version. Merges # do not define head_ref, so use run_id to never cancel those jobs. From 5c3d52371202e2f8913b472a3bf1f4276b903938 Mon Sep 17 00:00:00 2001 From: Stephen Davies Date: Tue, 14 May 2024 14:31:13 +1000 Subject: [PATCH 5/5] Use "close" event on res rather than attaching an event handler to the socket --- lib/controllers/proxy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/controllers/proxy.js b/lib/controllers/proxy.js index 595aa5c4..11cb6097 100644 --- a/lib/controllers/proxy.js +++ b/lib/controllers/proxy.js @@ -198,7 +198,7 @@ module.exports = function(options) { // encoding : null means "body" passed to the callback will be raw bytes var proxiedRequest; - req.socket.once('close', function() { + res.once('close', function() { if (proxiedRequest && !res.writableEnded) { proxiedRequest.abort(); }