Merge pull request #575 from ShogunFire/fixSpecialCharsEndingURL

Removing special chars that are in the end of a url and shouldn't be
Expensify · Oct 17, 2023 · 6517be4 · 6517be4
2 parents bdbdf44 + a604e9d
commit 6517be4
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 0 deletions.
diff --git a/__tests__/ExpensiMark-HTML-test.js b/__tests__/ExpensiMark-HTML-test.js
@@ -675,6 +675,36 @@ test('Test urls with unmatched closing parentheses autolinks correctly', () => {
     });
 });
 
+test('Test urls ending with special characters followed by unmatched closing parentheses autolinks correctly', () => {
+    const testString = 'https://github.com/Expensify/ReactNativeChat/pull/645.) '
+        + 'https://github.com/Expensify/ReactNativeChat/pull/645$) '
+        + 'https://github.com/Expensify/ReactNativeChat/pull/645*) '
+        + 'https://github.com/Expensify/ReactNativeChat/pull/645+) '
+        + 'https://github.com/Expensify/ReactNativeChat/pull/645!) '
+        + 'https://github.com/Expensify/ReactNativeChat/pull/645,) '
+        + 'https://github.com/Expensify/ReactNativeChat/pull/645=) '
+        + 'https://staging.new.expensify.com/get-assistance/WorkspaceGeneralSettings. '
+        + 'https://staging.new.expensify.com/get-assistance/WorkspaceGeneralSettings.. '
+        + 'https://staging.new.expensify.com/get-assistance/WorkspaceGeneralSettings..) '
+        + '[https://staging.new.expensify.com/get-assistance/WorkspaceGeneralSettings] '
+        + 'https://google.com/path?param=) '
+        + 'https://google.com/path#fragment!) ';
+    const resultString = '<a href="https://github.com/Expensify/ReactNativeChat/pull/645" target="_blank" rel="noreferrer noopener">https://github.com/Expensify/ReactNativeChat/pull/645</a>.) '
+        + '<a href="https://github.com/Expensify/ReactNativeChat/pull/645" target="_blank" rel="noreferrer noopener">https://github.com/Expensify/ReactNativeChat/pull/645</a>$) '
+        + '<a href="https://github.com/Expensify/ReactNativeChat/pull/645" target="_blank" rel="noreferrer noopener">https://github.com/Expensify/ReactNativeChat/pull/645</a>*) '
+        + '<a href="https://github.com/Expensify/ReactNativeChat/pull/645" target="_blank" rel="noreferrer noopener">https://github.com/Expensify/ReactNativeChat/pull/645</a>+) '
+        + '<a href="https://github.com/Expensify/ReactNativeChat/pull/645" target="_blank" rel="noreferrer noopener">https://github.com/Expensify/ReactNativeChat/pull/645</a>!) '
+        + '<a href="https://github.com/Expensify/ReactNativeChat/pull/645" target="_blank" rel="noreferrer noopener">https://github.com/Expensify/ReactNativeChat/pull/645</a>,) '
+        + '<a href="https://github.com/Expensify/ReactNativeChat/pull/645" target="_blank" rel="noreferrer noopener">https://github.com/Expensify/ReactNativeChat/pull/645</a>=) '
+        + '<a href="https://staging.new.expensify.com/get-assistance/WorkspaceGeneralSettings" target=\"_blank\" rel=\"noreferrer noopener\">https://staging.new.expensify.com/get-assistance/WorkspaceGeneralSettings</a>. '
+        + '<a href="https://staging.new.expensify.com/get-assistance/WorkspaceGeneralSettings" target=\"_blank\" rel=\"noreferrer noopener\">https://staging.new.expensify.com/get-assistance/WorkspaceGeneralSettings</a>.. '
+        + '<a href="https://staging.new.expensify.com/get-assistance/WorkspaceGeneralSettings" target=\"_blank\" rel=\"noreferrer noopener\">https://staging.new.expensify.com/get-assistance/WorkspaceGeneralSettings</a>..) '
+        + '[<a href=\"https://staging.new.expensify.com/get-assistance/WorkspaceGeneralSettings\" target=\"_blank\" rel=\"noreferrer noopener\">https://staging.new.expensify.com/get-assistance/WorkspaceGeneralSettings</a>] '
+        + '<a href=\"https://google.com/path?param=" target=\"_blank\" rel=\"noreferrer noopener\">https://google.com/path?param=</a>) '
+        + '<a href=\"https://google.com/path#fragment!" target=\"_blank\" rel=\"noreferrer noopener\">https://google.com/path#fragment!</a>) ';
+    expect(parser.replace(testString)).toBe(resultString);
+});
+
 test('Test urls autolinks correctly', () => {
     const testCases = [
         {

diff --git a/lib/CONST.jsx b/lib/CONST.jsx
@@ -270,6 +270,13 @@ export const CONST = {
         }
     },
 
+    /**
+     * Special characters that need to be removed when they are ending an url
+     *
+     * @type String
+     */
+    SPECIAL_CHARS_TO_REMOVE: '$*.+!(,=',
+
     /**
      * Store all the regular expression we are using for matching stuff
      */

diff --git a/lib/ExpensiMark.js b/lib/ExpensiMark.js
@@ -469,6 +469,27 @@ export default class ExpensiMark {
                     unmatchedOpenParentheses--;
                 }
             }
+
+            // Because we are removing ) parenthesis, some special characters that shouldn't be in the href are in the href
+            // For example google.com/toto.) is accepted by the regular expression above and we remove the ) parenthesis, so the link becomes google.com/toto. which is not a valid link
+            // In that case we should also remove the "."
+            // Those characters should only be remove from the url if this url doesn't have a parameter or a fragment
+            if (!url.includes('?') && !url.includes('#')) {
+                let numberOfCharsToRemove = 0;
+
+                for (let i = url.length - 1; i >= 0; i--) {
+                    if (CONST.SPECIAL_CHARS_TO_REMOVE.includes(url[i])) {
+                        numberOfCharsToRemove++;
+                    } else {
+                        break;
+                    }
+                }
+                if (numberOfCharsToRemove) {
+                    match[0] = match[0].substring(0, match[0].length - numberOfCharsToRemove);
+                    url = url.substring(0, url.length - numberOfCharsToRemove);
+                }
+            }
+
             replacedText = replacedText.concat(textToCheck.substr(startIndex, (match.index - startIndex)));
 
             // We want to avoid matching domains in email addresses so we don't render them as URLs,