fix: increased body size limit, added config option (#3011)

src/core/server/app/middleware/json.ts

@@ -1,3 +1,8 @@
-import express from "express";
+import bodyParser from "body-parser";
 
-export const jsonMiddleware = express.json({});
+/**
+ * jsonMiddleware is middleware that will parse the incoming JSON payloads.
+ *
+ * @param limit the amount of bytes to allow for POST requests
+ */
+export const jsonMiddleware = (limit: number) => bodyParser.json({ limit });

src/core/server/app/router/api/account.ts

@@ -1,4 +1,5 @@
 import bodyParser from "body-parser";
+import bytes from "bytes";
 
 import { AppOptions } from "coral-server/app";
 import {
@@ -18,6 +19,9 @@ import { RouterOptions } from "coral-server/app/router/types";
 
 import { createAPIRouter } from "./helpers";
 
+// REQUEST_MAX is the maximum request size for routes on this router.
+const REQUEST_MAX = bytes("100kb");
+
 export function createNewAccountRouter(
   app: AppOptions,
   { passport }: Pick<RouterOptions, "passport">
@@ -26,7 +30,7 @@ export function createNewAccountRouter(
 
   router.post(
     "/confirm",
-    jsonMiddleware,
+    jsonMiddleware(REQUEST_MAX),
     authenticate(passport),
     confirmRequestHandler(app)
   );
@@ -34,7 +38,7 @@ export function createNewAccountRouter(
   router.put("/confirm", confirmHandler(app));
 
   router.get("/invite", inviteCheckHandler(app));
-  router.put("/invite", jsonMiddleware, inviteHandler(app));
+  router.put("/invite", jsonMiddleware(REQUEST_MAX), inviteHandler(app));
 
   router.get("/notifications/unsubscribe", unsubscribeCheckHandler(app));
   router.delete("/notifications/unsubscribe", unsubscribeHandler(app));

src/core/server/app/router/api/auth.ts

@@ -1,3 +1,4 @@
+import bytes from "bytes";
 import express from "express";
 
 import { AppOptions } from "coral-server/app";
@@ -21,6 +22,9 @@ import { RouterOptions } from "coral-server/app/router/types";
 
 import { createAPIRouter } from "./helpers";
 
+// REQUEST_MAX is the maximum request size for routes on this router.
+const REQUEST_MAX = bytes("100kb");
+
 function wrapPath(
   app: AppOptions,
   { passport }: Pick<RouterOptions, "passport">,
@@ -43,21 +47,25 @@ export function createNewAuthRouter(
   // Mount the Local Authentication handlers.
   router.post(
     "/local",
-    jsonMiddleware,
+    jsonMiddleware(REQUEST_MAX),
     wrapAuthn(passport, app.signingConfig, "local")
   );
 
-  router.post("/local/signup", jsonMiddleware, signupHandler(app));
+  router.post("/local/signup", jsonMiddleware(REQUEST_MAX), signupHandler(app));
   router.get("/local/forgot", forgotCheckHandler(app));
-  router.put("/local/forgot", jsonMiddleware, forgotResetHandler(app));
-  router.post("/local/forgot", jsonMiddleware, forgotHandler(app));
+  router.put(
+    "/local/forgot",
+    jsonMiddleware(REQUEST_MAX),
+    forgotResetHandler(app)
+  );
+  router.post("/local/forgot", jsonMiddleware(REQUEST_MAX), forgotHandler(app));
 
   // Mount the link handler.
   router.post(
     "/link",
     authenticate(passport),
     loggedInMiddleware,
-    jsonMiddleware,
+    jsonMiddleware(REQUEST_MAX),
     linkHandler(app)
   );
 

src/core/server/app/router/api/index.ts

@@ -55,7 +55,7 @@ export function createAPIRouter(app: AppOptions, options: RouterOptions) {
   router.use(
     "/graphql",
     authenticate(options.passport),
-    jsonMiddleware,
+    jsonMiddleware(app.config.get("max_request_size")),
     persistedQueryMiddleware(app),
     graphQLHandler(app)
   );

src/core/server/app/router/api/install.ts

@@ -1,3 +1,4 @@
+import bytes from "bytes";
 import { Router } from "express";
 
 import { AppOptions } from "coral-server/app";
@@ -7,6 +8,9 @@ import { tenantMiddleware } from "coral-server/app/middleware/tenant";
 
 import { createAPIRouter } from "./helpers";
 
+// REQUEST_MAX is the maximum request size for routes on this router.
+const REQUEST_MAX = bytes("100kb");
+
 export function createNewInstallRouter(app: AppOptions): Router {
   // Create a router.
   const router = createAPIRouter();
@@ -18,7 +22,7 @@ export function createNewInstallRouter(app: AppOptions): Router {
   );
   router.post(
     "/",
-    jsonMiddleware,
+    jsonMiddleware(REQUEST_MAX),
     tenantMiddleware({ cache: app.tenantCache, passNoTenant: true }),
     installHandler(app)
   );

src/core/server/config.ts

@@ -1,4 +1,5 @@
 import Joi from "@hapi/joi";
+import bytes from "bytes";
 import convict from "convict";
 import { parseConnectionString } from "mongodb-core";
 import ms from "ms";
@@ -55,6 +56,16 @@ convict.addFormat({
   coerce: (val: string): number => ms(val),
 });
 
+// Add a custom format that is a number of bytes parsed with `bytes`. This
+// allows more compact representations of values (10mb instead of 10e6).
+convict.addFormat({
+  name: "bytes",
+  validate: (val: number) => {
+    Joi.assert(val, Joi.number().positive().integer().required());
+  },
+  coerce: (val: string): number => bytes(val),
+});
+
 const algorithms = [
   "HS256",
   "HS384",
@@ -188,7 +199,7 @@ const config = convict({
     doc:
       "The keepalive timeout (in ms) that should be used to send keep alive messages through the websocket to keep the socket alive",
     format: "ms",
-    default: "30 seconds",
+    default: ms("30 seconds"),
     env: "WEBSOCKET_KEEP_ALIVE_TIMEOUT",
   },
   disable_tenant_caching: {
@@ -209,7 +220,7 @@ const config = convict({
     doc:
       "Disables subscriptions for the comment stream for all stories across all tenants where a comment has not been left within the timeout",
     format: "ms",
-    default: "2 weeks",
+    default: ms("2 weeks"),
     env: "DISABLE_LIVE_UPDATES_TIMEOUT",
   },
   disable_client_routes: {
@@ -228,22 +239,27 @@ const config = convict({
   },
   scrape_max_response_size: {
     doc: "The maximum size (in bytes) to allow for scraping responses.",
-    format: Number,
-    default: 10e6,
+    format: "bytes",
+    default: bytes("10mb"),
     env: "SCRAPE_MAX_RESPONSE_SIZE",
-    arg: "scrapeMaxResponseSize",
+  },
+  max_request_size: {
+    doc: "The maximum size (in bytes) to allow for post bodies to accept.",
+    format: "bytes",
+    default: bytes("500kb"),
+    env: "MAX_REQUEST_SIZE",
   },
   scrape_timeout: {
     doc: "The request timeout (in ms) for scraping operations.",
     format: "ms",
-    default: "10 seconds",
+    default: ms("10 seconds"),
     env: "SCRAPE_TIMEOUT",
   },
   perspective_timeout: {
     doc:
       "The request timeout (in ms) for perspective comment checking operations.",
     format: "ms",
-    default: "800 milliseconds",
+    default: ms("800 milliseconds"),
     env: "PERSPECTIVE_TIMEOUT",
   },
   force_ssl: {
@@ -263,7 +279,7 @@ const config = convict({
     doc:
       "The word list timeout (in ms) that should be used to limit the amount of time the process is frozen processing a word list comparison",
     format: "ms",
-    default: "100",
+    default: ms("100ms"),
     env: "WORD_LIST_TIMEOUT",
   },
   analytics_frontend_key: {

src/core/server/events/listeners/perspective.ts

@@ -80,11 +80,8 @@ export class PerspectiveCoralEventListener
       // Reconstruct the Tenant URL.
       const tenantURL = reconstructTenantURL(ctx.config, ctx.tenant);
 
-      // This typecast is needed because the custom `ms` format does not return the
-      // desired `number` type even though that's the only type it can output.
-      const timeout = (ctx.config.get(
-        "perspective_timeout"
-      ) as unknown) as number;
+      // Get the timeout value.
+      const timeout = ctx.config.get("perspective_timeout");
 
       // Get the response from perspective.
       const result = await sendToPerspective(

src/core/server/graph/loaders/Stories.ts

@@ -210,12 +210,9 @@ export default (ctx: GraphContext) => ({
     }).then(primeStoriesFromConnection(ctx)),
   debugScrapeMetadata: new DataLoader(
     createManyBatchLoadFn((url: string) =>
-      // This typecast is needed because the custom `ms` format does not return
-      // the desired `number` type even though that's the only type it can
-      // output.
       scraper.scrape({
         url,
-        timeout: (ctx.config.get("scrape_timeout") as unknown) as number,
+        timeout: ctx.config.get("scrape_timeout"),
         size: ctx.config.get("scrape_max_response_size"),
         customUserAgent: ctx.tenant.stories.scraping.customUserAgent,
         proxyURL: ctx.tenant.stories.scraping.proxyURL,

src/core/server/graph/resolvers/LiveConfiguration.ts

@@ -18,11 +18,9 @@ export const LiveConfiguration: GQLLiveConfigurationTypeResolver<LiveConfigurati
       return false;
     }
 
-    // This typecast is needed because the custom `ms` format does not return the
-    // desired `number` type even though that's the only type it can output.
-    const disableLiveUpdatesTimeout = (ctx.config.get(
+    const disableLiveUpdatesTimeout = ctx.config.get(
       "disable_live_updates_timeout"
-    ) as unknown) as number;
+    );
     if (disableLiveUpdatesTimeout > 0) {
       // If one of these is available, use it to determine the time since the
       // last comment.

src/core/server/graph/subscriptions/server.ts

@@ -259,11 +259,7 @@ export function createSubscriptionServer(
   schema: GraphQLSchema,
   options: Options
 ) {
-  // This typecast is needed because the custom `ms` format does not return the
-  // desired `number` type even though that's the only type it can output.
-  const keepAlive = (options.config.get(
-    "websocket_keep_alive_timeout"
-  ) as unknown) as number;
+  const keepAlive = options.config.get("websocket_keep_alive_timeout");
 
   return SubscriptionServer.create(
     {

src/core/server/services/comments/pipeline/phases/toxic.ts

@@ -73,9 +73,8 @@ export const toxic: IntermediateModerationPhase = async ({
     );
   }
 
-  // This typecast is needed because the custom `ms` format does not return the
-  // desired `number` type even though that's the only type it can output.
-  const timeout = (config.get("perspective_timeout") as unknown) as number;
+  // Get the timeout value.
+  const timeout = config.get("perspective_timeout");
 
   try {
     // FEATURE_FLAG:DISABLE_WARN_USER_OF_TOXIC_COMMENT

src/core/server/services/comments/pipeline/phases/wordList.ts

@@ -27,7 +27,7 @@ export const wordList: IntermediateModerationPhase = ({
   }
 
   // Get the timeout to use.
-  const timeout = (config.get("word_list_timeout") as unknown) as number;
+  const timeout = config.get("word_list_timeout");
 
   // Test the comment for banned words.
   const banned = list.test(tenant, "banned", timeout, bodyText);

src/core/server/services/stories/scraper/scraper.ts

@@ -189,9 +189,7 @@ export async function scrape(
     storyURL = retrievedStory.url;
   }
 
-  // This typecast is needed because the custom `ms` format does not return the
-  // desired `number` type even though that's the only type it can output.
-  const timeout = (config.get("scrape_timeout") as unknown) as number;
+  const timeout = config.get("scrape_timeout");
   const size = config.get("scrape_max_response_size");
 
   // Get the metadata from the scraped html.