From 91e5eb62998a7ea8d4e25c96b55c2dd709a9b21f Mon Sep 17 00:00:00 2001 From: Eric Cornelissen Date: Sun, 16 Nov 2025 16:50:51 +0100 Subject: [PATCH] [Refactor] Handle pathological strings better Improve input string that are pathological by virtue of containing lots of whitespace but not at the end or start of the string. Such strings can lead to quadratic runtime of the `rightWhitespace` regular expression, unnecessarily consuming computational resources. This patch addresses the problem by ensuring that the string does end with a whitespace character before removing whitespace character from the end of the string. This ensures that pathological inputs cannot cause quadratic runtime because such inputs won't reach the `rightWhitespace` expression. An alternative solution would be to have a negative lookbehind in the `rightWhitespace` expression. But, this regexp feature was not yet available in versions of JavaScript for which this polyfill is meant, hence it is not viable. The change is tested with a pathalogical input in the test suite. This is supported by the assert-time package which faciliates enforcing a maximum runtime on the function call. --- implementation.js | 7 +++++++ package.json | 5 ++++- test/tests.js | 16 ++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/implementation.js b/implementation.js index bf457b5..3152b02 100644 --- a/implementation.js +++ b/implementation.js @@ -3,6 +3,7 @@ var RequireObjectCoercible = require('es-object-atoms/RequireObjectCoercible'); var ToString = require('es-abstract/2024/ToString'); var callBound = require('call-bound'); +var safeRegexTester = require('safe-regex-test'); var $replace = callBound('String.prototype.replace'); var mvsIsWS = (/^\s$/).test('\u180E'); @@ -13,9 +14,15 @@ var leftWhitespace = mvsIsWS var rightWhitespace = mvsIsWS ? /[\x09\x0A\x0B\x0C\x0D\x20\xA0\u1680\u180E\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200A\u202F\u205F\u3000\u2028\u2029\uFEFF]+$/ : /[\x09\x0A\x0B\x0C\x0D\x20\xA0\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200A\u202F\u205F\u3000\u2028\u2029\uFEFF]+$/; +var hasRightMostWhitespace = safeRegexTester(mvsIsWS + ? /[\x09\x0A\x0B\x0C\x0D\x20\xA0\u1680\u180E\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200A\u202F\u205F\u3000\u2028\u2029\uFEFF]$/ + : /[\x09\x0A\x0B\x0C\x0D\x20\xA0\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200A\u202F\u205F\u3000\u2028\u2029\uFEFF]$/); /* eslint-enable no-control-regex */ module.exports = function trim() { var S = ToString(RequireObjectCoercible(this)); + if (!hasRightMostWhitespace(S)) { + return $replace(S, leftWhitespace, ''); + } return $replace($replace(S, leftWhitespace, ''), rightWhitespace, ''); }; diff --git a/package.json b/package.json index e76066e..ff4ee38 100644 --- a/package.json +++ b/package.json @@ -52,9 +52,11 @@ "define-properties": "^1.2.1", "es-abstract": "^1.23.5", "es-object-atoms": "^1.0.0", - "has-property-descriptors": "^1.0.2" + "has-property-descriptors": "^1.0.2", + "safe-regex-test": "^1.1.0" }, "devDependencies": { + "@ericcornelissen/assert-time": "^1.0.0", "@es-shims/api": "^2.5.1", "@ljharb/eslint-config": "^21.1.1", "auto-changelog": "^2.5.0", @@ -67,6 +69,7 @@ "npmignore": "^0.3.1", "nyc": "^10.3.2", "safe-publish-latest": "^2.0.0", + "string.prototype.repeat": "^1.0.0", "tape": "^5.9.0" }, "testling": { diff --git a/test/tests.js b/test/tests.js index fcc0651..473a3c7 100644 --- a/test/tests.js +++ b/test/tests.js @@ -1,11 +1,15 @@ 'use strict'; var forEach = require('for-each'); +var repeat = require('string.prototype.repeat'); +var assertTime = require('@ericcornelissen/assert-time'); module.exports = function (trim, t) { t.test('normal cases', function (st) { st.equal(trim(' \t\na \t\n'), 'a', 'strips whitespace off left and right sides'); st.equal(trim('a'), 'a', 'noop when no whitespace'); + st.equal(trim(' a'), 'a', 'strips whitespace off left side only'); + st.equal(trim('a '), 'a', 'strips whitespace off right side only'); var allWhitespaceChars = '\x09\x0A\x0B\x0C\x0D\x20\xA0\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200A\u202F\u205F\u3000\u2028\u2029\uFEFF'; st.equal(trim(allWhitespaceChars + 'a' + allWhitespaceChars), 'a', 'all expected whitespace chars are trimmed'); @@ -13,6 +17,18 @@ module.exports = function (trim, t) { st.end(); }); + t.test('pathological case', function (st) { + var input = 'A' + repeat(' ', 100000) + 'A'; + + st.doesNotThrow(function () { + assertTime(function () { + trim(input); + }, 500); + }); + + st.end(); + }); + // see https://codeblog.jonskeet.uk/2014/12/01/when-is-an-identifier-not-an-identifier-attack-of-the-mongolian-vowel-separator/ var mongolianVowelSeparator = '\u180E'; var mvsIsWS = (/^\s$/).test('\u180E');