One Range parser, and a path that leaves the volume is a refusal

backend/src/routes/files/preview.js

@@ -2,19 +2,39 @@ const path = require('path');
 const fs = require('fs/promises');
 const fss = require('fs');
 const { normalizeRelativePath } = require('../../utils/pathUtils');
+const { parseByteRange } = require('../../utils/httpRange');
 const { resolvePathWithAccess } = require('../../services/accessManager');
 const { extensions, mimeTypes } = require('../../config/index');
 const { getRawPreviewJpegPath } = require('../../services/rawPreviewService');
 const asyncHandler = require('../../utils/asyncHandler');
 const {
   ValidationError,
   ForbiddenError,
+  NotFoundError,
   UnsupportedMediaTypeError,
 } = require('../../errors/AppError');
 const logger = require('../../utils/logger');
+const { markLongPoll } = require('../../middleware/heldRequests');
 
 const router = require('express').Router();
 
+// Formats the browser executes when opened as a top-level document. Served
+// inline they would run their own scripts on the application origin, so they
+// get a sandbox CSP — which still lets an <img> render them normally.
+const ACTIVE_CONTENT_EXTENSIONS = new Set(['svg']);
+
+const buildPreviewSecurityHeaders = (extension) => {
+  const headers = {
+    // Never let the browser second-guess the declared type.
+    'X-Content-Type-Options': 'nosniff',
+    'X-Robots-Tag': 'noindex',
+  };
+  if (ACTIVE_CONTENT_EXTENSIONS.has(extension)) {
+    headers['Content-Security-Policy'] = 'sandbox';
+  }
+  return headers;
+};
+
 router.get(
   '/preview',
   asyncHandler(async (req, res) => {
@@ -32,7 +52,17 @@ router.get(
     }
 
     const { absolutePath } = resolved;
-    const stats = await fs.stat(absolutePath);
+    let stats;
+    try {
+      stats = await fs.stat(absolutePath);
+    } catch (error) {
+      // A file deleted or renamed since the listing was drawn: a stale view,
+      // not a fault in the server.
+      if (error.code === 'ENOENT') {
+        throw new NotFoundError('File not found.');
+      }
+      throw error;
+    }
 
     if (stats.isDirectory()) {
       throw new ValidationError('Cannot preview a directory.');
@@ -54,6 +84,7 @@ router.get(
       res.writeHead(200, {
         'Content-Type': 'image/jpeg',
         'Content-Length': jpegStats.size,
+        ...buildPreviewSecurityHeaders('jpeg'),
       });
 
       const stream = fss.createReadStream(jpegPath);
@@ -74,6 +105,7 @@ router.get(
     }
 
     const mimeType = mimeTypes[extension] || 'application/octet-stream';
+    const securityHeaders = buildPreviewSecurityHeaders(extension);
     const isSeekableMedia =
       extensions.videos.includes(extension) || (extensions.audios || []).includes(extension);
 
@@ -93,41 +125,40 @@ router.get(
     };
 
     if (isSeekableMedia) {
-      const rangeHeader = req.headers.range;
-      if (rangeHeader) {
-        const bytesPrefix = 'bytes=';
-        if (!rangeHeader.startsWith(bytesPrefix)) {
-          res.status(416).send('Malformed Range header');
-          return;
-        }
-
-        const [startString, endString] = rangeHeader.slice(bytesPrefix.length).split('-');
-        let start = Number(startString);
-        let end = endString ? Number(endString) : stats.size - 1;
-
-        if (Number.isNaN(start)) start = 0;
-        if (Number.isNaN(end) || end >= stats.size) end = stats.size - 1;
-
-        if (start > end) {
-          res.status(416).send('Range Not Satisfiable');
-          return;
-        }
-
-        const chunkSize = end - start + 1;
+      // Streaming a film holds the connection open for as long as the browser
+      // wants it — minutes, and longer over a slow link. That is the request
+      // doing its job, not a symptom, and the held-request instrument reports
+      // only ten before falling silent for the life of the process: a handful
+      // of videos would spend the whole budget and switch off the one tool
+      // there is for finding a genuinely stuck server.
+      markLongPoll(req);
+
+      const range = parseByteRange(req.headers.range, stats.size);
+      if (range?.malformed) {
+        res.status(416).send('Malformed Range header');
+        return;
+      }
+      if (range?.unsatisfiable) {
+        res.status(416).send('Range Not Satisfiable');
+        return;
+      }
+      if (range) {
         res.writeHead(206, {
-          'Content-Range': `bytes ${start}-${end}/${stats.size}`,
+          'Content-Range': `bytes ${range.start}-${range.end}/${stats.size}`,
           'Accept-Ranges': 'bytes',
-          'Content-Length': chunkSize,
+          'Content-Length': range.chunkSize,
           'Content-Type': mimeType,
+          ...securityHeaders,
         });
-        streamFile({ start, end });
+        streamFile({ start: range.start, end: range.end });
         return;
       }
 
       res.writeHead(200, {
         'Content-Type': mimeType,
         'Content-Length': stats.size,
         'Accept-Ranges': 'bytes',
+        ...securityHeaders,
       });
       streamFile();
       return;
@@ -136,6 +167,7 @@ router.get(
     res.writeHead(200, {
       'Content-Type': mimeType,
       'Content-Length': stats.size,
+      ...securityHeaders,
     });
     streamFile();
   })

backend/src/routes/shares.js

@@ -1,4 +1,5 @@
 const express = require('express');
+const { parseByteRange } = require('../utils/httpRange');
 const fs = require('fs/promises');
 const fss = require('fs');
 const path = require('path');
@@ -253,34 +254,23 @@ const streamResolvedFile = async ({ absolutePath, stats, mode, req, res }) => {
     'X-Robots-Tag': 'noindex',
   };
 
-  const rangeHeader = req.headers.range;
-  if (rangeHeader) {
-    const bytesPrefix = 'bytes=';
-    if (!rangeHeader.startsWith(bytesPrefix)) {
-      res.status(416).send('Malformed Range header');
-      return;
-    }
-
-    const [startString, endString] = rangeHeader.slice(bytesPrefix.length).split('-');
-    let start = Number(startString);
-    let end = endString ? Number(endString) : stats.size - 1;
-
-    if (Number.isNaN(start)) start = 0;
-    if (Number.isNaN(end) || end >= stats.size) end = stats.size - 1;
-
-    if (start > end) {
-      res.status(416).send('Range Not Satisfiable');
-      return;
-    }
-
-    const chunkSize = end - start + 1;
+  const range = parseByteRange(req.headers.range, stats.size);
+  if (range?.malformed) {
+    res.status(416).send('Malformed Range header');
+    return;
+  }
+  if (range?.unsatisfiable) {
+    res.status(416).send('Range Not Satisfiable');
+    return;
+  }
+  if (range) {
     res.writeHead(206, {
       ...baseHeaders,
-      'Content-Range': `bytes ${start}-${end}/${stats.size}`,
+      'Content-Range': `bytes ${range.start}-${range.end}/${stats.size}`,
       'Accept-Ranges': 'bytes',
-      'Content-Length': chunkSize,
+      'Content-Length': range.chunkSize,
     });
-    streamFile({ start, end });
+    streamFile({ start: range.start, end: range.end });
     return;
   }
 

backend/src/utils/httpRange.js

@@ -0,0 +1,52 @@
+/**
+ * Byte-range parsing for the routes that stream a file.
+ *
+ * The share download route and the preview route each carried their own copy
+ * of this, which is how one of them ended up serving SVG without the headers
+ * the other set. One implementation means one place to fix.
+ */
+
+/**
+ * Interpret a Range header against a known file size.
+ *
+ * Returns `null` when the header is absent (send the whole file), or
+ * `{ malformed: true }` / `{ unsatisfiable: true }` for a 416 — the caller
+ * decides how to answer, since it owns the response.
+ */
+const parseByteRange = (rangeHeader, size) => {
+  if (!rangeHeader) return null;
+
+  const bytesPrefix = 'bytes=';
+  if (!String(rangeHeader).startsWith(bytesPrefix)) {
+    return { malformed: true };
+  }
+
+  const [startString, endString] = String(rangeHeader).slice(bytesPrefix.length).split('-');
+
+  // "bytes=-500" asks for the last 500 bytes, not the first 501. Reading the
+  // empty start as 0 turned every suffix request into a request for the head
+  // of the file — silently wrong, since the response still looks valid.
+  if (startString === '' && endString !== '' && endString !== undefined) {
+    const suffixLength = Number(endString);
+    if (Number.isNaN(suffixLength)) return { malformed: true };
+    // An empty file has no last N bytes, and a zero-length suffix asks for
+    // nothing: both would otherwise produce end = -1 and a bogus Content-Range.
+    if (suffixLength === 0 || size === 0) return { unsatisfiable: true };
+    const start = Math.max(0, size - suffixLength);
+    return { start, end: size - 1, chunkSize: size - start };
+  }
+
+  let start = Number(startString);
+  let end = endString ? Number(endString) : size - 1;
+
+  if (Number.isNaN(start)) start = 0;
+  if (Number.isNaN(end) || end >= size) end = size - 1;
+
+  if (start > end) {
+    return { unsatisfiable: true };
+  }
+
+  return { start, end, chunkSize: end - start + 1 };
+};
+
+module.exports = { parseByteRange };

backend/src/utils/pathUtils.js

@@ -83,7 +83,10 @@ const normalizeRelativePath = (relativePath = '') => {
   }
 
   if (normalized === '..' || normalized.startsWith('..' + path.sep)) {
-    throw new Error('Invalid path. Traversal outside the volume root is not allowed.');
+    // The request's fault, not the server's: a plain Error reached the browser as a
+    // 500, so a path that leaves the volume read as a server fault rather than a
+    // refusal — and a 500 is what a caller retries.
+    throw new ValidationError('Invalid path. Traversal outside the volume root is not allowed.');
   }
 
   return normalized;