From 1d0def60477d27f2e8563c7122e0a2aee4ca2cfb Mon Sep 17 00:00:00 2001 From: Warren R Bank Date: Thu, 23 Dec 2021 15:54:16 -0800 Subject: [PATCH] release: 130002.3.1 (based on 13.0.2, version 3.1) purpose: * ensure that file descriptors are closed, and never leak - if a fd were to leak as the result of the file having been requested by a client and served by httpd, then the underlying file will remain open and cause the OS to prevent the file from being deleted until the process running `serve` terminates potential causes for fd leak: 1) bug in `serve` https://github.com/vercel/serve-handler/blob/6.1.3/src/index.js#L751 summary: - the fd is opened BEFORE checking ETag to conditionally return a 304 Not Modified Response - when this occurs, the fd is never piped to the response, and it is never closed 2) bug in `nodejs` https://github.com/nodejs/node/issues/1180 https://github.com/nodejs/node/issues/1834 summary: - if the client closes the connection before the piped fd stream is fully written to the response, it has been reported that the fd is never closed workaround: https://github.com/jshttp/on-finished#example --- lib/serve-handler/package.json | 2 ++ lib/serve-handler/src/index.js | 30 +++++++++++----------- package-lock.json | 47 ++++++++++++++++++++++++++++++++-- package.json | 2 +- test/httpd.json | 2 +- 5 files changed, 64 insertions(+), 19 deletions(-) diff --git a/lib/serve-handler/package.json b/lib/serve-handler/package.json index 524b8e5..efb06a4 100644 --- a/lib/serve-handler/package.json +++ b/lib/serve-handler/package.json @@ -16,9 +16,11 @@ "basic-auth": "2.0.1", "bytes": "3.0.0", "content-disposition": "0.5.2", + "destroy": "1.0.4", "fast-url-parser": "1.1.3", "mime-types": "2.1.18", "minimatch": "3.0.4", + "on-finished": "2.3.0", "path-is-inside": "1.0.2", "path-to-regexp": "2.2.1", "range-parser": "1.2.0" diff --git a/lib/serve-handler/src/index.js b/lib/serve-handler/src/index.js index c7da5d0..f536c2c 100644 --- a/lib/serve-handler/src/index.js +++ b/lib/serve-handler/src/index.js @@ -16,6 +16,8 @@ const isPathInside = require('path-is-inside') const parseRange = require('range-parser') const lnk = require('@recent-cli/resolve-lnk') const auth = require('basic-auth') +const onFinished = require('on-finished') +const destroy = require('destroy') // Other const directoryTemplate = require('./directory') @@ -886,6 +888,15 @@ module.exports = async (request, response, config = {}, methods = {}) => { return process_error(getError(404)) } + const headers = await getHeaders(handlers, config, paths.real.current, paths.real.absolutePath, paths.real.stats) + + if (!request.headers.range && headers.ETag && (headers.ETag === request.headers['if-none-match'])) { + response.statusCode = 304 + response.end() + + return + } + const streamOpts = {} // TODO ? if-range @@ -912,32 +923,21 @@ module.exports = async (request, response, config = {}, methods = {}) => { try { stream = await handlers.createReadStream(paths.real.absolutePath, streamOpts) + + onFinished(response, () => { + destroy(stream) + }) } catch (err) { return process_error(getError(err)) } - const headers = await getHeaders(handlers, config, paths.real.current, paths.real.absolutePath, paths.real.stats) - // eslint-disable-next-line no-undefined if (streamOpts.start !== undefined && streamOpts.end !== undefined) { headers['Content-Range'] = `bytes ${streamOpts.start}-${streamOpts.end}/${paths.real.stats.size}` headers['Content-Length'] = streamOpts.end - streamOpts.start + 1 } - // We need to check for `headers.ETag` being truthy first, otherwise it will - // match `undefined` being equal to `undefined`, which is true. - // - // Checking for `undefined` and `null` is also important, because `Range` can be `0`. - // - // eslint-disable-next-line no-eq-null - if (request.headers.range == null && headers.ETag && headers.ETag === request.headers['if-none-match']) { - response.statusCode = 304 - response.end() - - return - } - response.writeHead(response.statusCode || 200, headers) stream.pipe(response) } diff --git a/package-lock.json b/package-lock.json index 0d3f377..39d3581 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@warren-bank/serve", - "version": "130002.1.0", + "version": "130002.3.1", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@warren-bank/serve", - "version": "130002.1.0", + "version": "130002.3.1", "hasInstallScript": true, "license": "GPL-2.0", "dependencies": { @@ -45,9 +45,11 @@ "basic-auth": "2.0.1", "bytes": "3.0.0", "content-disposition": "0.5.2", + "destroy": "1.0.4", "fast-url-parser": "1.1.3", "mime-types": "2.1.18", "minimatch": "3.0.4", + "on-finished": "2.3.0", "path-is-inside": "1.0.2", "path-to-regexp": "2.2.1", "range-parser": "1.2.0" @@ -398,6 +400,16 @@ "node": ">=4.0.0" } }, + "node_modules/destroy": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/destroy/-/destroy-1.0.4.tgz", + "integrity": "sha1-l4hXRCxEdJ5CBmE+N5RiBYJqvYA=" + }, + "node_modules/ee-first": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/ee-first/-/ee-first-1.1.1.tgz", + "integrity": "sha1-WQxhFWsK4vTwJVcyoViyZrxWsh0=" + }, "node_modules/emoji-regex": { "version": "8.0.0", "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-8.0.0.tgz", @@ -601,6 +613,17 @@ "node": ">=4" } }, + "node_modules/on-finished": { + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/on-finished/-/on-finished-2.3.0.tgz", + "integrity": "sha1-IPEzZIGwg811M3mSoWlxqi2QaUc=", + "dependencies": { + "ee-first": "1.1.1" + }, + "engines": { + "node": ">= 0.8" + } + }, "node_modules/on-headers": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/on-headers/-/on-headers-1.0.2.tgz", @@ -1171,6 +1194,16 @@ "resolved": "https://registry.npmjs.org/deep-extend/-/deep-extend-0.6.0.tgz", "integrity": "sha512-LOHxIOaPYdHlJRtCQfDIVZtfw/ufM8+rVj649RIHzcm/vGwQRXFt6OPqIFWsm2XEMrNIEtWR64sY1LEKD2vAOA==" }, + "destroy": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/destroy/-/destroy-1.0.4.tgz", + "integrity": "sha1-l4hXRCxEdJ5CBmE+N5RiBYJqvYA=" + }, + "ee-first": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/ee-first/-/ee-first-1.1.1.tgz", + "integrity": "sha1-WQxhFWsK4vTwJVcyoViyZrxWsh0=" + }, "emoji-regex": { "version": "8.0.0", "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-8.0.0.tgz", @@ -1328,6 +1361,14 @@ "path-key": "^2.0.0" } }, + "on-finished": { + "version": "2.3.0", + "resolved": "https://registry.npmjs.org/on-finished/-/on-finished-2.3.0.tgz", + "integrity": "sha1-IPEzZIGwg811M3mSoWlxqi2QaUc=", + "requires": { + "ee-first": "1.1.1" + } + }, "on-headers": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/on-headers/-/on-headers-1.0.2.tgz", @@ -1442,9 +1483,11 @@ "basic-auth": "2.0.1", "bytes": "3.0.0", "content-disposition": "0.5.2", + "destroy": "1.0.4", "fast-url-parser": "1.1.3", "mime-types": "2.1.18", "minimatch": "3.0.4", + "on-finished": "2.3.0", "path-is-inside": "1.0.2", "path-to-regexp": "2.2.1", "range-parser": "1.2.0" diff --git a/package.json b/package.json index 2238dd2..4c04c64 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@warren-bank/serve", - "version": "130002.3.0", + "version": "130002.3.1", "description": "Static file serving and directory listing", "author": "Warren Bank (https://github.com/warren-bank)", "license": "GPL-2.0", diff --git a/test/httpd.json b/test/httpd.json index d722845..369ac6e 100644 --- a/test/httpd.json +++ b/test/httpd.json @@ -1,7 +1,7 @@ { "cleanUrls": false, "renderSingle": false, - "etag": false, + "etag": true, "symlinks": true, "headers": [{