Merge pull request #179 from TerriaJS/share-url-too-large

Set 413 response status when share data is too large

Author: na9da

CHANGES.md

@@ -1,5 +1,8 @@
 ### Next version
 
+* Exposed and the 413 status code for the `/share` endpoint to the client for a more meaningful error handling when the story causes shareData to exceed the limit.
+* Exposed the `shareMaxRequestSize` string value and `shareMaxRequestSizeBytes` numeric value in the `/serverconfig` endpoint. 
+
 ### 4.0.2 - 2025-06-03
 
 * Requires Node 20.

lib/controllers/serverconfig.js

@@ -1,13 +1,14 @@
 /* jshint node: true, esnext: true */
 "use strict";
 var express = require('express');
+var bytes = require('bytes');
 
 // Expose a whitelisted set of configuration attributes to the world. This definitely doesn't include authorisation tokens, local file paths, etc.
 // It mirrors the structure of the real config file.
 module.exports = function(options) {
     var router = express.Router();
     var settings = Object.assign({}, options.settings), safeSettings = {};
-    var safeAttributes = ['allowProxyFor', 'newShareUrlPrefix', 'proxyAllDomains'];
+    var safeAttributes = ['allowProxyFor', 'newShareUrlPrefix', 'proxyAllDomains', 'shareMaxRequestSize'];
     safeAttributes.forEach(key => safeSettings[key] = settings[key]);
     safeSettings.version = require('../../package.json').version;
     if (typeof settings.shareUrlPrefixes === 'object') {
@@ -19,6 +20,9 @@ module.exports = function(options) {
     if (settings.feedback && settings.feedback.additionalParameters) {
         safeSettings.additionalFeedbackParameters = settings.feedback.additionalParameters;
     }
+    if (settings.shareMaxRequestSize) {
+        safeSettings.shareMaxRequestSizeBytes = bytes.parse(settings.shareMaxRequestSize);
+    }
 
     router.get('/', function(req, res, next) {
         res.status(200).send(safeSettings);

lib/controllers/share.js

@@ -148,6 +148,15 @@ module.exports = function(hostName, port, options) {
         limit: options.shareMaxRequestSize || '200kb'
     }));
 
+    // Return the 413 error thrown by body-parser to the client
+    router.use(function(error, req, res, next) {
+        if (error && (error.status === 413 || error.type === 'entity.too.large')) {
+            res.status(413).send('Payload Too Large');
+        } else {
+            next(error);
+        }
+    });
+
     // Requested creation of a new short URL.
     router.post('/', function(req, res, next) {
         if (options.newShareUrlPrefix === undefined || !options.shareUrlPrefixes[options.newShareUrlPrefix]) {

package.json

@@ -33,6 +33,7 @@
     "base-x": "^5.0.1",
     "basic-auth": "^2.0.1",
     "body-parser": "^1.20.3",
+    "bytes": "^3.1.2",
     "compression": "^1.6.0",
     "cors": "^2.7.1",
     "express": "^4.21.2",
@@ -48,6 +49,7 @@
   },
   "devDependencies": {
     "jasmine": "^4.6.0",
+    "nock": "^14.0.4",
     "supertest": "^7.0.0"
   }
 }

spec/share.spec.js

@@ -0,0 +1,92 @@
+"use strict";
+
+const makeServer = require('../lib/makeserver');
+const request = require('supertest');
+const nock = require('nock');
+const path = require('path');
+
+jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000;
+
+describe('share endpoint (integration, real controller)', function() {
+    const appOptions = {
+        wwwroot: "./spec/mockwwwroot",
+        hostName: "localhost",
+        port: "3001",
+        settings: {
+            shareUrlPrefixes: {
+                "g": {
+                    "service": "gist",
+                    "userAgent": "TerriaJS-Server",
+                    "gistFilename": "terriajs-server-catalog.json",
+                    "gistDescription": "TerriaJS Shared catalog"
+                }
+            },
+            newShareUrlPrefix: "g",
+            shareMaxRequestSize: "200kb"
+        }
+    };
+
+    function buildApp() {
+        const options = require('../lib/options').init(true);
+        const mergedOptions = Object.assign(options, appOptions);
+        return makeServer(mergedOptions);
+    }
+
+    beforeEach(() => {
+        nock.cleanAll();
+    });
+
+    afterAll(() => {
+        nock.restore();
+    });
+
+    describe('POST /share', function() {
+        it('should return 201 and correct response for valid payload', function(done) {
+            // Mock GitHub Gist API
+            const fakeGistId = "123456";
+            nock('https://api.github.com')
+                .post('/gists')
+                .reply(201, {
+                    id: fakeGistId
+                });
+
+            const payload = { test: "me" };
+
+            request(buildApp())
+                .post('/share')
+                .send(payload)
+                .expect(201)
+                .expect('Content-Type', /json/)
+                .expect(res => {
+                    const actualUrl = res.body.url;
+                    const expectedPath = `/share/g-${fakeGistId}`;
+                    expect(res.body.id).toBe(`g-${fakeGistId}`);
+                    expect(res.body.path).toBe(expectedPath);
+                    expect(actualUrl.endsWith(expectedPath)).toBeTrue();
+                })
+                .end(function(err) {
+                    if (err) return done.fail(err);
+                    done();
+                });
+        });
+
+        it('should return 413 when payload exceeds shareMaxRequestSize', function(done) {
+            const largePayload = 'a'.repeat(250000); // 250KB
+            request(buildApp())
+                .post('/share')
+                .set('Content-Type', 'application/json')
+                .send(`{"data":"${largePayload}"}`)
+                .expect(413)
+                .expect('Content-Type', /text|plain/)
+                .expect(res => {
+                    expect(
+                        res.text.includes('Payload Too Large')
+                    ).toBeTrue();
+                })
+                .end(function(err) {
+                    if (err) return done.fail(err);
+                    done();
+                });
+        });
+    });
+});