Skip to content

Commit

Permalink
release: 130002.3.1 (based on 13.0.2, version 3.1)
Browse files Browse the repository at this point in the history
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`
      nodejs/node#1180
      nodejs/node#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
  • Loading branch information
warren-bank committed Dec 23, 2021
1 parent b268b19 commit 1d0def6
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 19 deletions.
2 changes: 2 additions & 0 deletions lib/serve-handler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
30 changes: 15 additions & 15 deletions lib/serve-handler/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
47 changes: 45 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
2 changes: 1 addition & 1 deletion test/httpd.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"cleanUrls": false,
"renderSingle": false,
"etag": false,
"etag": true,
"symlinks": true,

"headers": [{
Expand Down

0 comments on commit 1d0def6

Please sign in to comment.