Merge pull request #395 from stasm/unskip-bomb

Bail out of Patterns with too long placeables

Author: stasm

fluent/src/bundle.js

@@ -1,5 +1,6 @@
 import {resolveComplexPattern} from "./resolver.js";
 import FluentResource from "./resource.js";
+import { FluentNone } from "./types.js";
 
 /**
  * Message bundles are single-language stores of translations.  They are
@@ -220,8 +221,13 @@ export default class FluentBundle {
     // Resolve a complex pattern.
     if (Array.isArray(pattern)) {
       let scope = this._createScope(args, errors);
-      let value = resolveComplexPattern(scope, pattern);
-      return value.toString(scope);
+      try {
+        let value = resolveComplexPattern(scope, pattern);
+        return value.toString(scope);
+      } catch (err) {
+        scope.errors.push(err);
+        return new FluentNone().toString(scope);
+      }
     }
 
     throw new TypeError("Invalid Pattern type");

fluent/src/resolver.js

@@ -277,17 +277,19 @@ export function resolveComplexPattern(scope, ptn) {
     }
 
     if (part.length > MAX_PLACEABLE_LENGTH) {
-      scope.errors.push(
-        new RangeError(
-          "Too many characters in placeable " +
-          `(${part.length}, max allowed is ${MAX_PLACEABLE_LENGTH})`
-        )
+      scope.dirty.delete(ptn);
+      // This is a fatal error which causes the resolver to instantly bail out
+      // on this pattern. The length check protects against excessive memory
+      // usage, and throwing protects against eating up the CPU when long
+      // placeables are deeply nested.
+      throw new RangeError(
+        "Too many characters in placeable " +
+        `(${part.length}, max allowed is ${MAX_PLACEABLE_LENGTH})`
       );
-      result.push(part.slice(MAX_PLACEABLE_LENGTH));
-    } else {
-      result.push(part);
     }
 
+    result.push(part);
+
     if (useIsolating) {
       result.push(PDI);
     }

fluent/test/bomb_test.js

@@ -30,12 +30,10 @@ suite('Reference bombs', function() {
         `);
     });
 
-    // XXX Protect the FTL Resolver against the billion laughs attack
-    // https://bugzil.la/1307126
-    test.skip('does not expand all placeables', function() {
+    test('does not expand all placeables', function() {
       const msg = bundle.getMessage('lolz');
       const val = bundle.formatPattern(msg.value, args, errs);
-      assert.strictEqual(val, '???');
+      assert.strictEqual(val, '{???}');
       assert.strictEqual(errs.length, 1);
     });
   });