MR: Address reviewer feedback: remove dotfiles option, fix keepAliveTimeout comment

This commit is contained in:
Trevor Renshaw 2026-05-17 09:41:20 -06:00
parent 09a84837cf
commit b2ba94eced
3 changed files with 10 additions and 17 deletions

View file

@ -301,9 +301,9 @@ class Server {
app.disable('x-powered-by') app.disable('x-powered-by')
this.server = http.createServer(app) this.server = http.createServer(app)
// Keep connections alive for 120s so multi-file downloads don't pay a new // Extend keep-alive timeout to 120s. Node's default (5s) closes idle
// TCP handshake per file. Default Node.js keepAliveTimeout is only 5s, // connections quickly, forcing a new TCP handshake for each file when a
// which causes connections to drop mid-download for files > 5s. // client downloads multiple files in sequence (e.g. individual chapters).
this.server.keepAliveTimeout = 120000 this.server.keepAliveTimeout = 120000
this.server.headersTimeout = 125000 this.server.headersTimeout = 125000

View file

@ -187,7 +187,7 @@ class LibraryItemController {
// Use sendFile so the `send` module sets Content-Length, Accept-Ranges, // Use sendFile so the `send` module sets Content-Length, Accept-Ranges,
// and handles range requests natively — no extra stat() call needed. // and handles range requests natively — no extra stat() call needed.
res.setHeader('Content-Disposition', `attachment; filename="${encodeURIComponent(Path.basename(req.libraryItem.relPath))}"`) res.setHeader('Content-Disposition', `attachment; filename="${encodeURIComponent(Path.basename(req.libraryItem.relPath))}"`)
await new Promise((resolve, reject) => res.sendFile(libraryItemPath, { dotfiles: 'allow' }, (error) => (error ? reject(error) : resolve()))) await new Promise((resolve, reject) => res.sendFile(libraryItemPath, (error) => (error ? reject(error) : resolve())))
} else { } else {
const filename = `${itemTitle}.zip` const filename = `${itemTitle}.zip`
await zipHelpers.zipDirectoryPipe(libraryItemPath, filename, res) await zipHelpers.zipDirectoryPipe(libraryItemPath, filename, res)
@ -1103,7 +1103,7 @@ class LibraryItemController {
// Use sendFile so the `send` module sets Content-Length, Accept-Ranges, // Use sendFile so the `send` module sets Content-Length, Accept-Ranges,
// and handles range requests natively — no extra stat() call needed. // and handles range requests natively — no extra stat() call needed.
res.setHeader('Content-Disposition', `attachment; filename="${encodeURIComponent(libraryFile.metadata.filename)}"`) res.setHeader('Content-Disposition', `attachment; filename="${encodeURIComponent(libraryFile.metadata.filename)}"`)
await new Promise((resolve, reject) => res.sendFile(libraryFile.metadata.path, { dotfiles: 'allow' }, (error) => (error ? reject(error) : resolve()))) await new Promise((resolve, reject) => res.sendFile(libraryFile.metadata.path, (error) => (error ? reject(error) : resolve())))
Logger.info(`[LibraryItemController] Downloaded file "${libraryFile.metadata.path}"`) Logger.info(`[LibraryItemController] Downloaded file "${libraryFile.metadata.path}"`)
} catch (error) { } catch (error) {
Logger.error(`[LibraryItemController] Failed to download file "${libraryFile.metadata.path}"`, error) Logger.error(`[LibraryItemController] Failed to download file "${libraryFile.metadata.path}"`, error)

View file

@ -332,26 +332,19 @@ describe('LibraryItemController', () => {
}) })
it('should call res.sendFile for single-file library items', async () => { it('should call res.sendFile for single-file library items', async () => {
res.sendFile.callsFake((filePath, opts, cb) => cb(null)) res.sendFile.callsFake((filePath, cb) => cb(null))
await LibraryItemController.download(req, res) await LibraryItemController.download(req, res)
expect(res.sendFile.calledOnce).to.be.true expect(res.sendFile.calledOnce).to.be.true
}) })
it('should pass { dotfiles: "allow" } option to res.sendFile', async () => {
res.sendFile.callsFake((filePath, opts, cb) => cb(null))
await LibraryItemController.download(req, res)
const opts = res.sendFile.firstCall.args[1]
expect(opts).to.deep.equal({ dotfiles: 'allow' })
})
it('should pass the correct file path to res.sendFile', async () => { it('should pass the correct file path to res.sendFile', async () => {
res.sendFile.callsFake((filePath, opts, cb) => cb(null)) res.sendFile.callsFake((filePath, cb) => cb(null))
await LibraryItemController.download(req, res) await LibraryItemController.download(req, res)
expect(res.sendFile.firstCall.args[0]).to.equal('/audiobooks/hitchhikers-guide.mp3') expect(res.sendFile.firstCall.args[0]).to.equal('/audiobooks/hitchhikers-guide.mp3')
}) })
it('should set Content-Disposition attachment header with the filename', async () => { it('should set Content-Disposition attachment header with the filename', async () => {
res.sendFile.callsFake((filePath, opts, cb) => cb(null)) res.sendFile.callsFake((filePath, cb) => cb(null))
await LibraryItemController.download(req, res) await LibraryItemController.download(req, res)
const dispositionCall = res.setHeader.args.find(([header]) => header === 'Content-Disposition') const dispositionCall = res.setHeader.args.find(([header]) => header === 'Content-Disposition')
expect(dispositionCall).to.exist expect(dispositionCall).to.exist
@ -361,14 +354,14 @@ describe('LibraryItemController', () => {
it('should URL-encode special characters in the Content-Disposition filename', async () => { it('should URL-encode special characters in the Content-Disposition filename', async () => {
req.libraryItem.relPath = 'Book With Spaces & Symbols!.mp3' req.libraryItem.relPath = 'Book With Spaces & Symbols!.mp3'
res.sendFile.callsFake((filePath, opts, cb) => cb(null)) res.sendFile.callsFake((filePath, cb) => cb(null))
await LibraryItemController.download(req, res) await LibraryItemController.download(req, res)
const dispositionCall = res.setHeader.args.find(([header]) => header === 'Content-Disposition') const dispositionCall = res.setHeader.args.find(([header]) => header === 'Content-Disposition')
expect(dispositionCall[1]).to.include(encodeURIComponent('Book With Spaces & Symbols!.mp3')) expect(dispositionCall[1]).to.include(encodeURIComponent('Book With Spaces & Symbols!.mp3'))
}) })
it('should return 500 when res.sendFile calls back with an error', async () => { it('should return 500 when res.sendFile calls back with an error', async () => {
res.sendFile.callsFake((filePath, opts, cb) => cb(new Error('File not found'))) res.sendFile.callsFake((filePath, cb) => cb(new Error('File not found')))
await LibraryItemController.download(req, res) await LibraryItemController.download(req, res)
expect(res.status.calledWith(500)).to.be.true expect(res.status.calledWith(500)).to.be.true
expect(res.send.called).to.be.true expect(res.send.called).to.be.true