From 06a8165d8a5e9366e680b7d1e73798dcba192366 Mon Sep 17 00:00:00 2001 From: Pierre Michel Date: Tue, 19 Sep 2023 08:32:18 -0600 Subject: [PATCH 1/4] Changing the way we remove unmatched closing parentheses to stop the url at the first one we encounter Signed-off-by: Pierre Michel --- lib/ExpensiMark.js | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/lib/ExpensiMark.js b/lib/ExpensiMark.js index 70c63dd2..8ebdf18d 100644 --- a/lib/ExpensiMark.js +++ b/lib/ExpensiMark.js @@ -446,26 +446,22 @@ export default class ExpensiMark { let startIndex = 0; while (match !== null) { - // we want to avoid matching ending ) unless it is a closing parenthesis for the URL - if (textToCheck[(match.index + match[2].length) - 1] === ')' && !match[2].includes('(')) { - match[0] = match[0].substr(0, match[0].length - 1); - match[2] = match[2].substr(0, match[2].length - 1); - } - - // remove extra ) parentheses - let brace = 0; - if (match[2][match[2].length - 1] === ')') { - for (let i = 0; i < match[2].length; i++) { - if (match[2][i] === '(') { - brace++; - } - if (match[2][i] === ')') { - brace--; - } + // We end the link at the last closing parenthesis that matches an opening parenthesis because unmatched closing parentheses are unlikely to be in the url + // and can be part of markdown for example + let unmatchedOpenParentheses = 0; + let url = match[2]; + for (let i = 0; i < url.length; i++) { + if (url[i] === '(') { + unmatchedOpenParentheses++; } - if (brace) { - match[0] = match[0].substr(0, match[0].length + brace); - match[2] = match[2].substr(0, match[2].length + brace); + else if (url[i] === ')') { + if(unmatchedOpenParentheses <= 0){ // unmatched closing parenthesis + const numberOfCharsToRemove = url.length - i; + match[0] = match[0].substr(0, match[0].length - numberOfCharsToRemove); + url = url.substr(0, url.length - numberOfCharsToRemove); + break; + } + unmatchedOpenParentheses--; } } replacedText = replacedText.concat(textToCheck.substr(startIndex, (match.index - startIndex))); @@ -479,7 +475,7 @@ export default class ExpensiMark { // If we find a URL with a leading @ sign, we need look for other domains in the rest of the string if ((match.index !== 0) && (textToCheck[match.index - 1] === '@')) { const domainRegex = new RegExp('^(([a-z-0-9]+\\.)+[a-z]{2,})(\\S*)', 'i'); - const domainMatch = domainRegex.exec(match[2]); + const domainMatch = domainRegex.exec(url); // If we find another domain in the remainder of the string, we apply the auto link rule again and set a flag to avoid re-doing below. if ((domainMatch !== null) && (domainMatch[3] !== '')) { @@ -506,7 +502,7 @@ export default class ExpensiMark { filterRules: ['bold', 'strikethrough', 'italic'], shouldEscapeText: false, }); - replacedText = replacedText.concat(replacement(match[0], linkText, match[2])); + replacedText = replacedText.concat(replacement(match[0], linkText, url)); } startIndex = match.index + (match[0].length); From 2a38db3bc304ccfd8435a1873ef98842fd054076 Mon Sep 17 00:00:00 2001 From: Pierre Michel Date: Tue, 19 Sep 2023 09:01:27 -0600 Subject: [PATCH 2/4] Adding more tests for unmatched closing parentheses Signed-off-by: Pierre Michel --- __tests__/ExpensiMark-HTML-test.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/__tests__/ExpensiMark-HTML-test.js b/__tests__/ExpensiMark-HTML-test.js index a6b3f4ff..ab69797b 100644 --- a/__tests__/ExpensiMark-HTML-test.js +++ b/__tests__/ExpensiMark-HTML-test.js @@ -631,9 +631,13 @@ test('Test a url ending with a question mark autolinks correctly', () => { expect(parser.replace(testString)).toBe(resultString); }); -test('Test a url ending with a closing parentheses autolinks correctly', () => { - const testString = 'https://github.com/Expensify/ReactNativeChat/pull/645)'; - const resultString = 'https://github.com/Expensify/ReactNativeChat/pull/645)'; +test('Test urls with unmatched closing parentheses autolinks correctly', () => { + const testString = 'https://github.com/Expensify/ReactNativeChat/pull/645) ' + + 'https://github.com/Expensify/ReactNativeChat/pull/test(645)) ' + + 'google.com/(toto))titi)'; + const resultString = 'https://github.com/Expensify/ReactNativeChat/pull/645) ' + + 'https://github.com/Expensify/ReactNativeChat/pull/test(645)) ' + + 'google.com/(toto))titi)'; expect(parser.replace(testString)).toBe(resultString); }); From 619092ea09729f92589da5dc003a8eca474eeaa1 Mon Sep 17 00:00:00 2001 From: Pierre Michel Date: Mon, 25 Sep 2023 09:13:27 -0600 Subject: [PATCH 3/4] Improve tests format and 1 comment Signed-off-by: Pierre Michel --- __tests__/ExpensiMark-HTML-test.js | 25 ++++++++++++++++++------- lib/ExpensiMark.js | 3 ++- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/__tests__/ExpensiMark-HTML-test.js b/__tests__/ExpensiMark-HTML-test.js index ab69797b..4156baca 100644 --- a/__tests__/ExpensiMark-HTML-test.js +++ b/__tests__/ExpensiMark-HTML-test.js @@ -632,13 +632,24 @@ test('Test a url ending with a question mark autolinks correctly', () => { }); test('Test urls with unmatched closing parentheses autolinks correctly', () => { - const testString = 'https://github.com/Expensify/ReactNativeChat/pull/645) ' - + 'https://github.com/Expensify/ReactNativeChat/pull/test(645)) ' - + 'google.com/(toto))titi)'; - const resultString = 'https://github.com/Expensify/ReactNativeChat/pull/645) ' - + 'https://github.com/Expensify/ReactNativeChat/pull/test(645)) ' - + 'google.com/(toto))titi)'; - expect(parser.replace(testString)).toBe(resultString); + const testCases = [ + { + testString: 'https://github.com/Expensify/ReactNativeChat/pull/645)', + resultString: 'https://github.com/Expensify/ReactNativeChat/pull/645)', + }, + { + testString: 'https://github.com/Expensify/ReactNativeChat/pull/test(645))', + resultString: 'https://github.com/Expensify/ReactNativeChat/pull/test(645))', + }, + { + testString: 'google.com/(toto))titi)', + resultString: 'google.com/(toto))titi)', + }, + + ]; + testCases.forEach(testCase => { + expect(parser.replace(testCase.testString)).toBe(testCase.resultString); + }); }); test('Test urls autolinks correctly', () => { diff --git a/lib/ExpensiMark.js b/lib/ExpensiMark.js index 8ebdf18d..9de8c246 100644 --- a/lib/ExpensiMark.js +++ b/lib/ExpensiMark.js @@ -455,7 +455,8 @@ export default class ExpensiMark { unmatchedOpenParentheses++; } else if (url[i] === ')') { - if(unmatchedOpenParentheses <= 0){ // unmatched closing parenthesis + // Unmatched closing parenthesis + if(unmatchedOpenParentheses <= 0){ const numberOfCharsToRemove = url.length - i; match[0] = match[0].substr(0, match[0].length - numberOfCharsToRemove); url = url.substr(0, url.length - numberOfCharsToRemove); From dec7dfd75a06b5fc1e5b033bc624de81ca82be33 Mon Sep 17 00:00:00 2001 From: Pierre Michel Date: Tue, 26 Sep 2023 05:48:27 -0600 Subject: [PATCH 4/4] Fix lint errors Signed-off-by: Pierre Michel --- lib/ExpensiMark.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/ExpensiMark.js b/lib/ExpensiMark.js index 9de8c246..b74270b4 100644 --- a/lib/ExpensiMark.js +++ b/lib/ExpensiMark.js @@ -453,10 +453,9 @@ export default class ExpensiMark { for (let i = 0; i < url.length; i++) { if (url[i] === '(') { unmatchedOpenParentheses++; - } - else if (url[i] === ')') { + } else if (url[i] === ')') { // Unmatched closing parenthesis - if(unmatchedOpenParentheses <= 0){ + if (unmatchedOpenParentheses <= 0) { const numberOfCharsToRemove = url.length - i; match[0] = match[0].substr(0, match[0].length - numberOfCharsToRemove); url = url.substr(0, url.length - numberOfCharsToRemove);