feat(uploads): headersSent guard on stream errors + text/html MIME for GridFS

modules/uploads/controllers/uploads.controller.js

@@ -6,6 +6,7 @@ import _ from 'lodash';
 
 import config from '../../../config/index.js';
 import errors from '../../../lib/helpers/errors.js';
+import logger from '../../../lib/services/logger.js';
 import responses from '../../../lib/helpers/responses.js';
 import UploadsService from '../services/uploads.service.js';
 
@@ -38,9 +39,21 @@ const normalizeMime = (raw) => String(raw).toLowerCase().split(';')[0].trim();
 const get = async (req, res) => {
   try {
     const stream = await UploadsService.getStream({ _id: req.upload._id });
-    if (!stream) responses.error(res, 404, 'Not Found', 'No Upload with that identifier can been found')();
+    if (!stream) return responses.error(res, 404, 'Not Found', 'No Upload with that identifier can been found')();
     stream.on('error', (err) => {
-      responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
+      // Guard against ERR_HTTP_HEADERS_SENT when GridFS fails mid-transfer.
+      // Response headers are written when the first chunk is piped to res
+      // (`stream.pipe(res)` below). If GridFS errors after that point, a
+      // second status+body write would throw ERR_HTTP_HEADERS_SENT and crash
+      // the worker process. Pre-header errors still reach the client as 422;
+      // post-header errors destroy the socket so Express surfaces them via
+      // its default error handler instead of double-sending.
+      if (!res.headersSent) {
+        responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
+      } else {
+        logger.error('uploads.get - stream error after headers sent', err);
+        res.destroy(err);
+      }
     });
     const raw = req.upload.contentType || req.upload.metadata?.contentType || 'application/octet-stream';
     const norm = normalizeMime(raw);
@@ -63,27 +76,45 @@ const get = async (req, res) => {
 const getSharp = async (req, res) => {
   try {
     const stream = await UploadsService.getStream({ _id: req.upload._id });
-    if (!stream) responses.error(res, 404, 'Not Found', 'No Upload with that identifier can been found')();
-    stream.on('error', (err) => {
-      responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
-    });
+    if (!stream) return responses.error(res, 404, 'Not Found', 'No Upload with that identifier can been found')();
+    // headersSent guard for the GridFS source stream.
+    // pipe() does not forward 'error' events between streams; each stream in the
+    // chain needs its own listener. Source and Transform errors are handled below.
+    // Pre-header errors reach the client as 422; post-header errors destroy the
+    // socket so Express surfaces them via its default handler instead of
+    // attempting a second write that would throw ERR_HTTP_HEADERS_SENT.
+    const onStreamError = (label) => (err) => {
+      if (!res.headersSent) {
+        responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
+      } else {
+        logger.error(`uploads.getSharp - ${label} error after headers sent`, err);
+        res.destroy(err);
+      }
+    };
+    stream.on('error', onStreamError('source stream'));
     const raw = req.upload.contentType || req.upload.metadata?.contentType || 'application/octet-stream';
     const norm = normalizeMime(raw);
     const contentType = SAFE_IMAGE_MIME.has(norm) ? norm : 'image/jpeg';
     res.set('Content-Type', contentType);
-    switch (req.sharpOption) {
-      case 'blur':
-        stream.pipe(sharp().resize(req.sharpSize).blur(config.uploads.sharp.blur)).pipe(res);
-        break;
-      case 'bw':
-        stream.pipe(sharp().resize(req.sharpSize).grayscale()).pipe(res);
-        break;
-      case 'blur&bw':
-        stream.pipe(sharp().resize(req.sharpSize).grayscale().blur(config.uploads.sharp.blur)).pipe(res);
-        break;
-      default:
-        stream.pipe(sharp().resize(req.sharpSize)).pipe(res);
-    }
+    const buildTransform = () => {
+      let transform;
+      switch (req.sharpOption) {
+        case 'blur':
+          transform = sharp().resize(req.sharpSize).blur(config.uploads.sharp.blur);
+          break;
+        case 'bw':
+          transform = sharp().resize(req.sharpSize).grayscale();
+          break;
+        case 'blur&bw':
+          transform = sharp().resize(req.sharpSize).grayscale().blur(config.uploads.sharp.blur);
+          break;
+        default:
+          transform = sharp().resize(req.sharpSize);
+      }
+      transform.on('error', onStreamError('sharp transform'));
+      return transform;
+    };
+    stream.pipe(buildTransform()).pipe(res);
   } catch (err) {
     responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
   }

modules/uploads/services/uploads.service.js

@@ -15,6 +15,9 @@ const MIME_TO_EXT = {
   'image/png': 'png',
   'image/gif': 'gif',
   'application/pdf': 'pdf',
+  // Allows downstream features that store rendered HTML snapshots in GridFS
+  // alongside binary uploads (e.g. error pages, scrap previews).
+  'text/html': 'html',
 };
 
 /**

modules/uploads/tests/uploads.controller.contenttype.unit.tests.js

@@ -35,6 +35,11 @@ describe('Uploads controller content-type allowlist unit tests:', () => {
     };
   };
 
+  let mockLogger;
+  // Captures the 'error' listener registered on the sharp Transform so tests
+  // can fire it manually to simulate a sharp-pipeline mid-stream failure.
+  let sharpListeners;
+
   beforeEach(async () => {
     jest.resetModules();
 
@@ -44,6 +49,9 @@ describe('Uploads controller content-type allowlist unit tests:', () => {
       remove: jest.fn(),
     };
 
+    mockLogger = { error: jest.fn(), warn: jest.fn(), info: jest.fn() };
+    sharpListeners = {};
+
     jest.unstable_mockModule('../services/uploads.service.js', () => ({
       default: mockUploadsService,
     }));
@@ -56,12 +64,16 @@ describe('Uploads controller content-type allowlist unit tests:', () => {
     // sharp is a real dependency; mock it to avoid needing native bindings in unit context.
     // pipe must return `dest` (matching the real Stream.pipe contract) so that chained
     // calls stream.pipe(transform).pipe(res) work without throwing.
+    // on() captures listeners so tests can fire transform error events.
     jest.unstable_mockModule('sharp', () => ({
       default: jest.fn(() => ({
         resize: jest.fn().mockReturnThis(),
         blur: jest.fn().mockReturnThis(),
         grayscale: jest.fn().mockReturnThis(),
         pipe: jest.fn((dest) => dest),
+        on: jest.fn((event, handler) => {
+          sharpListeners[event] = handler;
+        }),
       })),
     }));
 
@@ -76,6 +88,10 @@ describe('Uploads controller content-type allowlist unit tests:', () => {
       },
     }));
 
+    jest.unstable_mockModule('../../../lib/services/logger.js', () => ({
+      default: mockLogger,
+    }));
+
     const mod = await import('../controllers/uploads.controller.js');
     UploadsController = mod.default;
   });
@@ -360,4 +376,161 @@ describe('Uploads controller content-type allowlist unit tests:', () => {
       expect(res.set).toHaveBeenCalledWith('Content-Type', 'image/jpeg');
     });
   });
+
+  // ── get — headersSent guard ───────────────────────────────────────────────
+
+  describe('get — headersSent guard on stream error', () => {
+    /**
+     * Install a fake stream that captures the `error` listener so tests can
+     * fire it manually — simulating either a pre-flush or mid-flush GridFS
+     * failure depending on `res.headersSent`.
+     */
+    const installStream = () => {
+      const listeners = {};
+      const stream = {
+        pipe: jest.fn().mockReturnThis(),
+        on: jest.fn((event, handler) => {
+          listeners[event] = handler;
+          return stream;
+        }),
+      };
+      mockUploadsService.getStream.mockResolvedValueOnce(stream);
+      return { stream, listeners };
+    };
+
+    const buildRes = () => {
+      const res = {};
+      res.status = jest.fn().mockReturnValue(res);
+      res.json = jest.fn().mockReturnValue(res);
+      res.set = jest.fn();
+      res.destroy = jest.fn();
+      res.headersSent = false;
+      return res;
+    };
+
+    const req = {
+      upload: {
+        _id: 'u1',
+        contentType: 'image/jpeg',
+        length: 1234,
+        metadata: { contentType: 'image/jpeg' },
+      },
+    };
+
+    test('responds via responses.error when stream errors before headers are sent', async () => {
+      const { listeners } = installStream();
+      const { default: responses } = await import('../../../lib/helpers/responses.js');
+      const res = buildRes();
+
+      await UploadsController.get(req, res);
+
+      res.headersSent = false;
+      listeners.error(new Error('gridfs chunk missing'));
+
+      expect(responses.error).toHaveBeenCalled();
+      expect(res.destroy).not.toHaveBeenCalled();
+    });
+
+    test('calls res.destroy(err) and logger.error when stream errors after headers are sent', async () => {
+      const { listeners } = installStream();
+      const { default: responses } = await import('../../../lib/helpers/responses.js');
+      const res = buildRes();
+
+      await UploadsController.get(req, res);
+
+      res.headersSent = true;
+      const err = new Error('gridfs mid-stream abort');
+      listeners.error(err);
+
+      // Must NOT attempt a new response body on an already-flushed response.
+      expect(responses.error).not.toHaveBeenCalled();
+      expect(res.destroy).toHaveBeenCalledWith(err);
+      // Error must reach the logger so mid-stream GridFS failures are visible.
+      expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('uploads.get'), err);
+    });
+  });
+
+  // ── getSharp — headersSent guard ──────────────────────────────────────────
+
+  describe('getSharp — headersSent guard on stream error', () => {
+    const installStream = () => {
+      const listeners = {};
+      const stream = {
+        pipe: jest.fn().mockReturnThis(),
+        on: jest.fn((event, handler) => {
+          listeners[event] = handler;
+          return stream;
+        }),
+      };
+      mockUploadsService.getStream.mockResolvedValueOnce(stream);
+      return { stream, listeners };
+    };
+
+    const buildRes = () => {
+      const res = {};
+      res.status = jest.fn().mockReturnValue(res);
+      res.json = jest.fn().mockReturnValue(res);
+      res.set = jest.fn();
+      res.destroy = jest.fn();
+      res.headersSent = false;
+      return res;
+    };
+
+    const req = {
+      upload: {
+        _id: 'u1',
+        contentType: 'image/jpeg',
+        metadata: { contentType: 'image/jpeg' },
+      },
+      sharpSize: null,
+      sharpOption: null,
+    };
+
+    test('responds via responses.error when stream errors before headers are sent', async () => {
+      const { listeners } = installStream();
+      const { default: responses } = await import('../../../lib/helpers/responses.js');
+      const res = buildRes();
+
+      await UploadsController.getSharp(req, res);
+
+      res.headersSent = false;
+      listeners.error(new Error('early gridfs failure'));
+
+      expect(responses.error).toHaveBeenCalled();
+      expect(res.destroy).not.toHaveBeenCalled();
+    });
+
+    test('calls res.destroy(err) and logger.error when source stream errors after headers are sent', async () => {
+      const { listeners } = installStream();
+      const { default: responses } = await import('../../../lib/helpers/responses.js');
+      const res = buildRes();
+
+      await UploadsController.getSharp(req, res);
+
+      res.headersSent = true;
+      const err = new Error('gridfs source mid-stream abort');
+      listeners.error(err);
+
+      expect(responses.error).not.toHaveBeenCalled();
+      expect(res.destroy).toHaveBeenCalledWith(err);
+      expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('uploads.getSharp'), err);
+    });
+
+    test('calls res.destroy(err) and logger.error when sharp transform errors after headers are sent', async () => {
+      installStream();
+      const { default: responses } = await import('../../../lib/helpers/responses.js');
+      const res = buildRes();
+
+      await UploadsController.getSharp(req, res);
+
+      // Simulate a sharp transform error mid-stream (after headers are flushed).
+      res.headersSent = true;
+      const err = new Error('sharp transform mid-stream abort');
+      sharpListeners.error(err);
+
+      expect(responses.error).not.toHaveBeenCalled();
+      expect(res.destroy).toHaveBeenCalledWith(err);
+      expect(mockLogger.error).toHaveBeenCalledWith(expect.stringContaining('uploads.getSharp'), err);
+    });
+  });
 });

modules/uploads/tests/uploads.createFromBuffer.unit.tests.js

@@ -175,6 +175,24 @@ describe('Uploads createFromBuffer unit tests:', () => {
     expect(filename).toMatch(/^[a-f0-9]{64}\.html$/);
   });
 
+  test('should resolve text/html to .html extension via built-in MIME_TO_EXT (no config.uploads.mimeTypes needed)', async () => {
+    // text/html is in the built-in MIME_TO_EXT map so downstream features that
+    // store rendered HTML snapshots in GridFS get a correct .html filename
+    // without requiring per-deployment mimeTypes config.
+    mockConfig.uploads.snapshot = {
+      kind: 'snapshot',
+      formats: ['text/html'],
+      limits: { fileSize: 1 * 1024 * 1024 },
+    };
+    mockGridfs.createFromBuffer.mockResolvedValue({ ...fakeFile, contentType: 'text/html' });
+
+    const buffer = Buffer.alloc(512);
+    await UploadsService.createFromBuffer(buffer, 'text/html', 'snapshot');
+
+    const [, filename] = mockGridfs.createFromBuffer.mock.calls[0];
+    expect(filename).toMatch(/^[a-f0-9]{64}\.html$/);
+  });
+
   test('should throw error when kind has no formats configured', async () => {
     // Adding 'broken' kind at runtime — service reads config dynamically via module reference
     mockConfig.uploads.broken = { kind: 'broken', limits: { fileSize: 1024 } };