Fix/413 payload too large

server/server.js

@@ -27,7 +27,7 @@ const CONFIG = {
   backupKeep: Math.max(0, parseInt(env.BACKUP_KEEP || '14', 10)),
   backupEveryHours: Math.max(1, parseInt(env.BACKUP_EVERY_HOURS || '24', 10)),
   historyKeep: Math.max(50, parseInt(env.HISTORY_KEEP || '5000', 10)),
-  maxImportBytes: parseInt(env.MAX_IMPORT_MB || '20', 10) * 1024 * 1024,
+  maxImportBytes: Math.max(1, parseInt(env.MAX_IMPORT_MB, 10) || 30) * 1024 * 1024,
   maxDocBytes: 512 * 1024,
   // AGPL-3.0 section 13: users of a modified version must be offered its source. Point this at your fork.
   sourceUrl: /^https?:\/\//.test(env.SOURCE_URL || '') ? env.SOURCE_URL : 'https://github.com/SolidGeeks/strata',
@@ -296,8 +296,20 @@ function send(res, status, body, headers = {}) {
 const fail = (res, status, error, extra) => send(res, status, { error, ...(extra || {}) });
 function readBody(req, limit) {
   return new Promise((resolve, reject) => {
+    const tooLarge = () => { 
+      req.resume(); 
+      reject(httpError(413, 'Payload too large')); 
+    };
+    if (Number(req.headers['content-length']) > limit) return tooLarge();
     let size = 0; const chunks = [];
-    req.on('data', c => { size += c.length; if (size > limit) { reject(httpError(413, 'Payload too large')); req.destroy(); return; } chunks.push(c); });
+    const onData = c => {
+      size += c.length;
+      if (size <= limit) return chunks.push(c);
+      req.off('data', onData);
+      chunks.length = 0;
+      tooLarge();
+    };
+    req.on('data', onData);
     req.on('end', () => { try { resolve(chunks.length ? JSON.parse(Buffer.concat(chunks).toString('utf8')) : null); } catch { reject(httpError(400, 'Invalid JSON')); } });
     req.on('error', reject);
   });
@@ -571,7 +583,10 @@ const server = http.createServer(async (req, res) => {
     else if (req.method === 'GET' || req.method === 'HEAD') await serveStatic(req, res, url.pathname);
     else fail(res, 405, 'Method not allowed');
   } catch (err) {
-    if (!res.headersSent) fail(res, err.status || 500, err.status ? err.message : 'Internal error', err.extra);
+    if (!res.headersSent) {
+      if (err.status === 413) res.setHeader('Connection', 'close');
+      fail(res, err.status || 500, err.status ? err.message : 'Internal error', err.extra);
+    }
     if (!err.status) log('ERROR', req.method, req.url, err.stack || err.message);
   }
 });

tests/api.test.mjs

@@ -176,8 +176,9 @@ test('import: replaces the map, validates ids, and enforces MAX_IMPORT_MB', asyn
   const bob = as('bob');
   assert.equal((await bob.post('/api/import', { si: { 'bad id!': {} }, assets: {} })).status, 400);
   const big = { si: { x: { name: 'x'.repeat(1.2 * 1024 * 1024) } }, assets: {} };
-  const tooBig = await bob.post('/api/import', big).catch(e => ({ status: 'connection closed', error: e }));
-  assert.ok(tooBig.status === 413 || tooBig.status === 'connection closed', `got ${tooBig.status}`);
+  const tooBig = await bob.post('/api/import', big);
+  assert.equal(tooBig.status, 413);
+  assert.deepEqual(tooBig.data, { error: 'Payload too large' });
   const before = await rev();
   const r = await bob.post('/api/import', { si: { lille: { name: 'Lille' } }, assets: {} });
   assert.equal(r.status, 200, JSON.stringify(r.data));
@@ -189,6 +190,32 @@ test('import: replaces the map, validates ids, and enforces MAX_IMPORT_MB', asyn
   assert.match(exp.headers.get('content-disposition'), /strata-\d{4}-\d{2}-\d{2}\.json/);
 });
 
+test('request bodies: oversized ones get a 413 whether announced or streamed, and the server keeps serving', async () => {
+  const bob = as('bob');
+  const rawPost = (path, { body = '', headers = {}, end = true }) => new Promise((res, rej) => {
+    const req = request({
+      host: '127.0.0.1', port, path, method: 'POST',
+      headers: { Authorization: bob.auth, 'Content-Type': 'application/json', 'X-Requested-With': 'strata', ...headers },
+    }, r => { let b = ''; r.on('data', c => (b += c)); r.on('end', () => { req.destroy(); res({ status: r.statusCode, connection: r.headers.connection, data: JSON.parse(b) }); }); });
+    req.on('error', rej);
+    req.write(body);
+    if (end) req.end();
+  });
+
+  const announced = await rawPost('/api/import', { headers: { 'Content-Length': 2 * 1024 * 1024 }, body: '{', end: false });
+  assert.equal(announced.status, 413, 'rejected from the Content-Length header without waiting for the body');
+  assert.equal(announced.connection, 'close');
+  assert.deepEqual(announced.data, { error: 'Payload too large' });
+
+  const streamed = await rawPost('/api/import', { body: JSON.stringify({ si: { x: { name: 'x'.repeat(1.2 * 1024 * 1024) } } }) });
+  assert.equal(streamed.status, 413, 'chunked bodies without a Content-Length are counted too');
+  assert.deepEqual(streamed.data, { error: 'Payload too large' });
+
+  assert.equal((await bob.put('/api/me/password', { password: 'x'.repeat(8 * 1024) })).status, 413, 'small per-route limits apply');
+  assert.deepEqual((await rawPost('/api/import', { body: '{"si":' })).data, { error: 'Invalid JSON' });
+  assert.equal((await bob.get('/api/state')).status, 200);
+});
+
 test('static files: path traversal cannot reach the server code', async () => {
   const raw = p => new Promise((res, rej) => {
     request({ host: '127.0.0.1', port, path: p, headers: { Authorization: as('vic').auth } }, r => { let b = ''; r.on('data', c => (b += c)); r.on('end', () => res({ status: r.statusCode, body: b })); }).on('error', rej).end();