Merge b2ba94eced into 1bad2d9072

.gitignore

@@ -24,3 +24,4 @@ sw.*
 .idea/*
 tailwind.compiled.css
 tailwind.config.js
+.taskling

server/Server.js

@@ -301,6 +301,11 @@ class Server {
     app.disable('x-powered-by')
 
     this.server = http.createServer(app)
+    // Extend keep-alive timeout to 120s. Node's default (5s) closes idle
+    // connections quickly, forcing a new TCP handshake for each file when a
+    // client downloads multiple files in sequence (e.g. individual chapters).
+    this.server.keepAliveTimeout = 120000
+    this.server.headersTimeout = 125000
 
     router.use(
       fileUpload({

server/controllers/LibraryItemController.js

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

server/utils/fileUtils.js

@@ -318,8 +318,9 @@ module.exports.downloadFile = (url, filepath, contentTypeFilter = null) => {
         const totalSize = parseInt(response.headers['content-length'], 10)
         let downloadedSize = 0
 
-        // Write to filepath
+        // Use a 512 KiB write buffer. Node's default 16 KiB highWaterMark causes
-        const writer = fs.createWriteStream(filepath)
+        // many small write syscalls for large audio files and limits throughput.
+        const writer = fs.createWriteStream(filepath, { highWaterMark: 524288 })
         response.data.pipe(writer)
 
         let lastProgress = 0

server/utils/zipHelpers.js

@@ -9,7 +9,11 @@ module.exports.zipDirectoryPipe = (path, filename, res) => {
     res.attachment(filename)
 
     const archive = archiver('zip', {
-      zlib: { level: 0 } // Sets the compression level.
+      zlib: { level: 0 }, // Sets the compression level (0 = store, no CPU overhead).
+      // Increase the internal pipeline highWaterMark from 16 KiB to 512 KiB.
+      // This allows archiver to buffer more data before applying backpressure,
+      // keeping the read-from-disk and write-to-network pipelines busy.
+      highWaterMark: 524288
     })
 
     // listen for all archive data to be written
@@ -67,7 +71,9 @@ module.exports.zipDirectoriesPipe = (pathObjects, filename, res) => {
     res.attachment(filename)
 
     const archive = archiver('zip', {
-      zlib: { level: 0 } // Sets the compression level.
+      zlib: { level: 0 }, // Sets the compression level (0 = store, no CPU overhead).
+      // Increase the internal pipeline highWaterMark from 16 KiB to 512 KiB.
+      highWaterMark: 524288
     })
 
     // listen for all archive data to be written

test/server/controllers/LibraryItemController.test.js

@@ -8,6 +8,7 @@ const LibraryItemController = require('../../../server/controllers/LibraryItemCo
 const ApiCacheManager = require('../../../server/managers/ApiCacheManager')
 const Auth = require('../../../server/Auth')
 const Logger = require('../../../server/Logger')
+const zipHelpers = require('../../../server/utils/zipHelpers')
 
 describe('LibraryItemController', () => {
   /** @type {ApiRouter} */
@@ -299,4 +300,88 @@ describe('LibraryItemController', () => {
       expect(fakeRes.sendStatus.calledWith(403)).to.be.true
     })
   })
+
+  describe('download', () => {
+    let req
+    let res
+
+    beforeEach(() => {
+      req = {
+        user: { username: 'testUser', canDownload: true },
+        libraryItem: {
+          isFile: true,
+          path: '/audiobooks/hitchhikers-guide.mp3',
+          relPath: 'hitchhikers-guide.mp3',
+          media: { title: "The Hitchhiker's Guide To The Galaxy" }
+        }
+      }
+      res = {
+        setHeader: sinon.spy(),
+        sendFile: sinon.stub(),
+        sendStatus: sinon.spy(),
+        status: sinon.stub().returnsThis(),
+        send: sinon.spy()
+      }
+    })
+
+    it('should return 403 when user does not have canDownload permission', async () => {
+      req.user.canDownload = false
+      await LibraryItemController.download(req, res)
+      expect(res.sendStatus.calledWith(403)).to.be.true
+      expect(res.sendFile.called).to.be.false
+    })
+
+    it('should call res.sendFile for single-file library items', async () => {
+      res.sendFile.callsFake((filePath, cb) => cb(null))
+      await LibraryItemController.download(req, res)
+      expect(res.sendFile.calledOnce).to.be.true
+    })
+
+    it('should pass the correct file path to res.sendFile', async () => {
+      res.sendFile.callsFake((filePath, cb) => cb(null))
+      await LibraryItemController.download(req, res)
+      expect(res.sendFile.firstCall.args[0]).to.equal('/audiobooks/hitchhikers-guide.mp3')
+    })
+
+    it('should set Content-Disposition attachment header with the filename', async () => {
+      res.sendFile.callsFake((filePath, cb) => cb(null))
+      await LibraryItemController.download(req, res)
+      const dispositionCall = res.setHeader.args.find(([header]) => header === 'Content-Disposition')
+      expect(dispositionCall).to.exist
+      expect(dispositionCall[1]).to.include('attachment')
+      expect(dispositionCall[1]).to.include('hitchhikers-guide.mp3')
+    })
+
+    it('should URL-encode special characters in the Content-Disposition filename', async () => {
+      req.libraryItem.relPath = 'Book With Spaces & Symbols!.mp3'
+      res.sendFile.callsFake((filePath, cb) => cb(null))
+      await LibraryItemController.download(req, res)
+      const dispositionCall = res.setHeader.args.find(([header]) => header === 'Content-Disposition')
+      expect(dispositionCall[1]).to.include(encodeURIComponent('Book With Spaces & Symbols!.mp3'))
+    })
+
+    it('should return 500 when res.sendFile calls back with an error', async () => {
+      res.sendFile.callsFake((filePath, cb) => cb(new Error('File not found')))
+      await LibraryItemController.download(req, res)
+      expect(res.status.calledWith(500)).to.be.true
+      expect(res.send.called).to.be.true
+    })
+
+    it('should call zipDirectoryPipe for multi-file directory items', async () => {
+      req.libraryItem.isFile = false
+      const zipStub = sinon.stub(zipHelpers, 'zipDirectoryPipe').resolves()
+      await LibraryItemController.download(req, res)
+      expect(zipStub.calledOnce).to.be.true
+      expect(zipStub.firstCall.args[0]).to.equal(req.libraryItem.path)
+      expect(zipStub.firstCall.args[1]).to.equal("The Hitchhiker's Guide To The Galaxy.zip")
+      expect(zipStub.firstCall.args[2]).to.equal(res)
+    })
+
+    it('should pass the response object to zipDirectoryPipe', async () => {
+      req.libraryItem.isFile = false
+      const zipStub = sinon.stub(zipHelpers, 'zipDirectoryPipe').resolves()
+      await LibraryItemController.download(req, res)
+      expect(zipStub.firstCall.args[2]).to.equal(res)
+    })
+  })
 })

test/server/utils/fileUtils.test.js

@@ -3,6 +3,9 @@ const expect = chai.expect
 const sinon = require('sinon')
 const fileUtils = require('../../../server/utils/fileUtils')
 const fs = require('fs')
+const http = require('http')
+const path = require('path')
+const os = require('os')
 const Logger = require('../../../server/Logger')
 
 describe('fileUtils', () => {
@@ -134,4 +137,85 @@ describe('fileUtils', () => {
       })
     })
   })
+
+  describe('downloadFile', () => {
+    let server
+    let serverPort
+    let tmpDir
+    let serveContent
+    let serveContentType
+    let serveStatusCode
+
+    before(async () => {
+      // Disable the SSRF filter so we can hit 127.0.0.1 in tests
+      global.DisableSsrfRequestFilter = () => true
+
+      server = http.createServer((req, res) => {
+        res.writeHead(serveStatusCode, {
+          'Content-Type': serveContentType,
+          'Content-Length': serveContent.length.toString()
+        })
+        res.end(serveContent)
+      })
+      await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve))
+      serverPort = server.address().port
+    })
+
+    after(async () => {
+      delete global.DisableSsrfRequestFilter
+      await new Promise((resolve) => server.close(resolve))
+    })
+
+    beforeEach(() => {
+      serveContent = Buffer.from('fake audio content for testing purposes')
+      serveContentType = 'audio/mpeg'
+      serveStatusCode = 200
+      tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'abs-fileutils-test-'))
+      sinon.stub(Logger, 'debug')
+      sinon.stub(Logger, 'error')
+    })
+
+    afterEach(() => {
+      sinon.restore()
+      fs.rmSync(tmpDir, { recursive: true, force: true })
+    })
+
+    it('should download file and write it to the specified path', async () => {
+      const destPath = path.join(tmpDir, 'output.mp3')
+      await fileUtils.downloadFile(`http://127.0.0.1:${serverPort}/test.mp3`, destPath)
+      expect(fs.existsSync(destPath)).to.be.true
+      expect(fs.readFileSync(destPath).toString()).to.equal(serveContent.toString())
+    })
+
+    it('should write the exact number of bytes served', async () => {
+      serveContent = Buffer.alloc(8192, 0xff)
+      const destPath = path.join(tmpDir, 'exact.mp3')
+      await fileUtils.downloadFile(`http://127.0.0.1:${serverPort}/exact.mp3`, destPath)
+      expect(fs.statSync(destPath).size).to.equal(8192)
+    })
+
+    it('should reject when content type filter rejects the response content type', async () => {
+      const destPath = path.join(tmpDir, 'rejected.mp3')
+      let didReject = false
+      try {
+        await fileUtils.downloadFile(`http://127.0.0.1:${serverPort}/test.mp3`, destPath, (ct) => ct.includes('video/'))
+      } catch (e) {
+        didReject = true
+        expect(e.message).to.include('Invalid content type')
+      }
+      expect(didReject).to.be.true
+    })
+
+    it('should pass when content type filter accepts the response content type', async () => {
+      const destPath = path.join(tmpDir, 'accepted.mp3')
+      await fileUtils.downloadFile(`http://127.0.0.1:${serverPort}/test.mp3`, destPath, (ct) => ct.includes('audio/'))
+      expect(fs.existsSync(destPath)).to.be.true
+    })
+
+    it('should resolve with no content type filter provided', async () => {
+      const destPath = path.join(tmpDir, 'nofilter.mp3')
+      await fileUtils.downloadFile(`http://127.0.0.1:${serverPort}/test.mp3`, destPath)
+      expect(fs.existsSync(destPath)).to.be.true
+    })
+  })
 })

test/server/utils/zipHelpers.test.js

@@ -0,0 +1,87 @@
+const { expect } = require('chai')
+const sinon = require('sinon')
+const { PassThrough } = require('stream')
+const path = require('path')
+const fs = require('fs')
+const os = require('os')
+const zipHelpers = require('../../../server/utils/zipHelpers')
+const Logger = require('../../../server/Logger')
+
+describe('zipHelpers', () => {
+  let tmpDir
+
+  beforeEach(() => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'abs-zip-test-'))
+    sinon.stub(Logger, 'info')
+    sinon.stub(Logger, 'debug')
+  })
+
+  afterEach(() => {
+    sinon.restore()
+    fs.rmSync(tmpDir, { recursive: true, force: true })
+  })
+
+  describe('zipDirectoryPipe', () => {
+    function makeMockRes() {
+      const stream = new PassThrough()
+      stream.attachment = sinon.spy()
+      // zipDirectoryPipe resolves on 'close'. PassThrough emits 'finish' but not 'close'
+      // automatically — destroy() triggers 'close', so bridge the two events here.
+      stream.on('finish', () => stream.destroy())
+      return stream
+    }
+
+    it('should call res.attachment with the provided filename', async () => {
+      fs.writeFileSync(path.join(tmpDir, 'book.mp3'), 'audio content')
+      const res = makeMockRes()
+      await zipHelpers.zipDirectoryPipe(tmpDir, 'my-book.zip', res)
+      expect(res.attachment.calledWith('my-book.zip')).to.be.true
+    })
+
+    it('should resolve and produce data for a non-empty directory', async () => {
+      fs.writeFileSync(path.join(tmpDir, 'chapter1.mp3'), 'audio data chapter 1')
+      fs.writeFileSync(path.join(tmpDir, 'chapter2.mp3'), 'audio data chapter 2')
+
+      const res = makeMockRes()
+      const chunks = []
+      res.on('data', (chunk) => chunks.push(chunk))
+
+      await zipHelpers.zipDirectoryPipe(tmpDir, 'book.zip', res)
+      const totalBytes = chunks.reduce((sum, c) => sum + c.length, 0)
+      expect(totalBytes).to.be.greaterThan(0)
+    })
+
+    it('should resolve for an empty directory', async () => {
+      const res = makeMockRes()
+      // Should not throw
+      await zipHelpers.zipDirectoryPipe(tmpDir, 'empty.zip', res)
+    })
+
+    it('should produce a valid zip containing the expected files', async () => {
+      fs.writeFileSync(path.join(tmpDir, 'test.txt'), 'Hello audiobookshelf')
+      const res = makeMockRes()
+      const chunks = []
+      res.on('data', (chunk) => chunks.push(chunk))
+
+      await zipHelpers.zipDirectoryPipe(tmpDir, 'archive.zip', res)
+
+      // ZIP files start with the PK signature (0x50 0x4B)
+      const combined = Buffer.concat(chunks)
+      expect(combined[0]).to.equal(0x50) // 'P'
+      expect(combined[1]).to.equal(0x4b) // 'K'
+    })
+
+    it('should resolve (not throw) when the source directory does not exist', async () => {
+      // archiver treats a missing directory as a warning (ENOENT), not a fatal error,
+      // so zipDirectoryPipe resolves with an empty archive rather than rejecting.
+      const res = makeMockRes()
+      let threw = false
+      try {
+        await zipHelpers.zipDirectoryPipe('/nonexistent/path/that/does/not/exist', 'fail.zip', res)
+      } catch (e) {
+        threw = true
+      }
+      expect(threw).to.be.false
+    })
+  })
+})