fix(designer-ui): Prevent expressions using &...; syntax disappeari…

…ng from HTML editor (#3760) * Make into a promise * Encode token expression values * Move encoders to helper file * Remove unused encoder * Move encoding to 'convertEditorState' * Revert unneeded change * Fix appending of encoded nodes * Remove unused method * Add one more test case * Convert insecure regex to a helper * Fix handling of `"` * Fix neglected test case
Azure · Nov 28, 2023 · b7b7aaa · b7b7aaa
1 parent b75f328
commit b7b7aaa
Show file tree

Hide file tree

Showing 6 changed files with 133 additions and 16 deletions.
diff --git a/libs/designer-ui/src/lib/editor/base/nodes/tokenNode.tsx b/libs/designer-ui/src/lib/editor/base/nodes/tokenNode.tsx
@@ -68,7 +68,7 @@ export class TokenNode extends DecoratorNode<JSX.Element> {
 
   // This is to enable copy to clipboard, even though there are some cases where @{} isn't needed, for the time being it's easier to always include it when copying
   getTextContent(_includeInert?: boolean | undefined, _includeDirectionless?: false | undefined): string {
-    return `@{${this.__data.value}}` ?? '';
+    return `@{${this.__data.value}}`;
   }
 
   toString(): string {

diff --git a/libs/designer-ui/src/lib/editor/base/utils/__test__/parsesegments.spec.ts b/libs/designer-ui/src/lib/editor/base/utils/__test__/parsesegments.spec.ts
@@ -0,0 +1,17 @@
+import { encodeStringSegments } from '../parsesegments';
+
+describe('lib/editor/base/utils/parseSegments', () => {
+  describe('encodeStringSegments', () => {
+    it.each([
+      ['plain text', 'plain text'],
+      [`$[concat(...),concat('&lt;'),#AD008C]$`, `$[concat(...),concat('%26lt;'),#AD008C]$`],
+      [`text$[concat(...),concat('&lt;'),#AD008C]$text`, `text$[concat(...),concat('%26lt;'),#AD008C]$text`],
+      [
+        `$[replace(...),replace(replace(replace('abc','&lt;','<'),'&gt;','>'),'&quot;','"') ,#AD008C]$`,
+        `$[replace(...),replace(replace(replace('abc','%26lt;','<'),'%26gt;','>'),'%26quot;','%22') ,#AD008C]$`,
+      ],
+    ])('should properly encode segments in: %p', (input, expected) => {
+      expect(encodeStringSegments(input, true)).toBe(expected);
+    });
+  });
+});
diff --git a/libs/designer-ui/src/lib/editor/base/utils/parsesegments.ts b/libs/designer-ui/src/lib/editor/base/utils/parsesegments.ts
@@ -1,4 +1,5 @@
 import { processNodeType } from '../../../html/plugins/toolbar/helper/functions';
+import { decodeSegmentValue, encodeSegmentValue } from '../../../html/plugins/toolbar/helper/util';
 import { getExpressionTokenTitle } from '../../../tokenpicker/util';
 import type { ValueSegment } from '../../models/parameter';
 import { TokenType, ValueSegmentType } from '../../models/parameter';
@@ -33,7 +34,9 @@ export const parseHtmlSegments = (value: ValueSegment[], tokensEnabled?: boolean
   const nodeMap = new Map<string, ValueSegment>();
 
   const stringValue = convertSegmentsToString(value, nodeMap);
-  const dom = parser.parseFromString(stringValue, 'text/html');
+  const encodedStringValue = encodeStringSegments(stringValue, tokensEnabled);
+
+  const dom = parser.parseFromString(encodedStringValue, 'text/html');
   const nodes = $generateNodesFromDOM(editor, dom);
   nodes.forEach((currNode) => {
     if ($isParagraphNode(currNode) || $isListNode(currNode) || $isHeadingNode(currNode)) {
@@ -99,12 +102,20 @@ const appendChildrenNode = (
   // if is a text node, parse for tokens
   if ($isTextNode(childNode)) {
     const textContent = childNode.getTextContent();
+    const decodedTextContent = tokensEnabled ? decodeSegmentValue(textContent) : textContent;
     const childNodeStyles = childNode.getStyle();
     const childNodeFormat = childNode.getFormat();
     // we need to pass in the styles and format of the parent node to the children node
     // because Lexical text nodes do not have styles or format
     // and we'll need to use the ExtendedTextNode to apply the styles and format
-    appendStringSegment(paragraph, textContent, childNodeStyles, childNodeFormat, nodeMap, tokensEnabled);
+    appendStringSegment(
+      paragraph,
+      decodedTextContent,
+      childNodeStyles,
+      childNodeFormat,
+      nodeMap,
+      tokensEnabled
+    );
   } else {
     paragraph.append(childNode);
   }
@@ -243,3 +254,32 @@ export const convertSegmentsToString = (input: ValueSegment[], nodeMap?: Map<str
   });
   return text;
 };
+
+export const encodeStringSegments = (value: string, tokensEnabled: boolean | undefined): string => {
+  if (!tokensEnabled) {
+    return value;
+  }
+
+  let newValue = "";
+
+  for (let i = 0; i < value.length; i++) {
+    if (value.substring(i, i + 2) === '$[') {
+      const startIndex = i;
+      const endIndex = value.indexOf(']$', startIndex);
+      if (endIndex === -1) {
+        break;
+      }
+
+      const tokenSegment = value.substring(startIndex + 2, endIndex);
+      const encodedTokenSegment = `$[${encodeSegmentValue(tokenSegment)}]$`;
+      newValue += encodedTokenSegment;
+
+      i = endIndex + 1; // Skip the ']', and i++ will skip the '$'.
+      continue;
+    }
+
+    newValue += value[i];
+  }
+
+  return newValue;
+};
diff --git a/libs/designer-ui/src/lib/html/plugins/toolbar/helper/Change.tsx b/libs/designer-ui/src/lib/html/plugins/toolbar/helper/Change.tsx
@@ -1,7 +1,7 @@
 import type { ValueSegment } from '../../../../editor';
 import { convertStringToSegments } from '../../../../editor/base/utils/editorToSegement';
 import { getChildrenNodes } from '../../../../editor/base/utils/helper';
-import { cleanHtmlString } from './util';
+import { cleanHtmlString, decodeSegmentValue, encodeSegmentValue } from './util';
 import { $generateHtmlFromNodes } from '@lexical/html';
 import { OnChangePlugin } from '@lexical/react/LexicalOnChangePlugin';
 import type { EditorState, LexicalEditor } from 'lexical';
@@ -41,6 +41,11 @@ const convertEditorState = (editor: LexicalEditor, nodeMap: Map<string, ValueSeg
           if (attribute.name !== 'id' && attribute.name !== 'style' && attribute.name !== 'href') {
             element.removeAttribute(attribute.name);
           }
+          if (attribute.name === 'id') {
+            const idValue = element.getAttribute('id') ?? ''; // e.g., "$[concat(...),concat('&lt;', '"'),#AD008C]$"
+            const encodedIdValue = encodeSegmentValue(idValue); // e.g., "$[concat(...),concat('%26lt;', '%22'),#AD008C]$"
+            element.setAttribute('id', encodedIdValue);
+          }
         }
       }
 
@@ -51,8 +56,9 @@ const convertEditorState = (editor: LexicalEditor, nodeMap: Map<string, ValueSeg
       const spanIdPattern = /<span id="(.*?)"><\/span>/g;
       // Replace <span id="..."></span> with the captured "id" value if it is found in the viable ids map
       const removeTokenTags = cleanedHtmlString.replace(spanIdPattern, (match, idValue) => {
-        if (nodeMap.get(idValue)) {
-          return idValue;
+        const decodedIdValue = decodeSegmentValue(idValue);
+        if (nodeMap.get(decodedIdValue)) {
+          return decodedIdValue;
         } else {
           return match;
         }

diff --git a/libs/designer-ui/src/lib/html/plugins/toolbar/helper/__test__/util.spec.ts b/libs/designer-ui/src/lib/html/plugins/toolbar/helper/__test__/util.spec.ts
@@ -1,19 +1,50 @@
-import { cleanHtmlString } from "../util";
+import { cleanHtmlString, decodeSegmentValue, encodeSegmentValue } from '../util';
 
 describe('lib/html/plugins/toolbar/helper/util', () => {
   describe('cleanHtmlString', () => {
     it.each([
-      ["<p>text1<span>\n</span>text2</p>", "<p>text1<br>text2</p>"],
-      ["<p>text</p>", "<p>text</p>"],
-      ["<p>text1</p><p><br></p><p>text2</p>", "<p>text1</p><br><p>text2</p>"],
-      ["<p>text1<br></p><p><br></p><p>text2</p>", "<p>text1</p><br><br><p>text2</p>"],
-      ["<span>text</span>", "text"],
-      [
-        "<span id=\"$[abc,variables('abc'),#770bd6]$\">text</span>",
-        "<span id=\"$[abc,variables('abc'),#770bd6]$\">text</span>",
-      ],
+      ['<p>text1<span>\n</span>text2</p>', '<p>text1<br>text2</p>'],
+      ['<p>text</p>', '<p>text</p>'],
+      ['<p>text1</p><p><br></p><p>text2</p>', '<p>text1</p><br><p>text2</p>'],
+      ['<p>text1<br></p><p><br></p><p>text2</p>', '<p>text1</p><br><br><p>text2</p>'],
+      ['<span>text</span>', 'text'],
+      ['<span id="$[abc,variables(\'abc\'),#770bd6]$">text</span>', '<span id="$[abc,variables(\'abc\'),#770bd6]$">text</span>'],
+      ['<span id="$[concat(\'&lt;\'),#ad008c]$">text</span>', '<span id="$[concat(\'&lt;\'),#ad008c]$">text</span>'],
+      ['<span id="$[concat(\'%26lt;\'),#ad008c]$">text</span>', '<span id="$[concat(\'%26lt;\'),#ad008c]$">text</span>'],
     ])('should properly convert HTML: %p', (input, expected) => {
       expect(cleanHtmlString(input)).toBe(expected);
     });
   });
+
+  describe('decodeSegmentValue', () => {
+    it.each([
+      ['plain text', 'plain text'],
+      ['text with %26lt;%26gt;', 'text with &lt;&gt;'],
+      ['text with %26noIdeaWhatThisIs;', 'text with &noIdeaWhatThisIs;'],
+      ['text with %26#60;%26gt;', 'text with &#60;&gt;'],
+      ['text with %26amp;lt;%26amp;gt;', 'text with &amp;lt;&amp;gt;'],
+      ['text with %22%26quot;%22', 'text with "&quot;"'],
+      ["$[concat(...),concat('abc'),#AD008C]$", "$[concat(...),concat('abc'),#AD008C]$"],
+      ["$[concat(...),concat('%26lt;'),#AD008C]$", "$[concat(...),concat('&lt;'),#AD008C]$"],
+      ["$[concat(...),concat('%26amp;lt;'),#AD008C]$", "$[concat(...),concat('&amp;lt;'),#AD008C]$"],
+    ])('should properly decode segments in: %p', (input, expected) => {
+      expect(decodeSegmentValue(input)).toBe(expected);
+    });
+  });
+
+  describe('encodeSegmentValue', () => {
+    it.each([
+      ['plain text', 'plain text'],
+      ['text with &lt;&gt;', 'text with %26lt;%26gt;'],
+      ['text with &noIdeaWhatThisIs;', 'text with %26noIdeaWhatThisIs;'],
+      ['text with &#60;&gt;', 'text with %26#60;%26gt;'],
+      ['text with &amp;lt;&amp;gt;', 'text with %26amp;lt;%26amp;gt;'],
+      ['text with "&quot;"', 'text with %22%26quot;%22'],
+      ["$[concat(...),concat('abc'),#AD008C]$", "$[concat(...),concat('abc'),#AD008C]$"],
+      ["$[concat(...),concat('&lt;'),#AD008C]$", "$[concat(...),concat('%26lt;'),#AD008C]$"],
+      ["$[concat(...),concat('&amp;lt;'),#AD008C]$", "$[concat(...),concat('%26amp;lt;'),#AD008C]$"],
+    ])('should properly encode segments in: %p', (input, expected) => {
+      expect(encodeSegmentValue(input)).toBe(expected);
+    });
+  });
 });
diff --git a/libs/designer-ui/src/lib/html/plugins/toolbar/helper/util.ts b/libs/designer-ui/src/lib/html/plugins/toolbar/helper/util.ts
@@ -1,3 +1,13 @@
+const unsafeCharacters = ['&', '"'];
+const unsafeCharacterEncodingMap: Record<string, string> = unsafeCharacters.reduce(
+  (acc, key) => ({ ...acc, [key]: encodeURIComponent(key) }),
+  {}
+);
+const unsafeCharacterDecodingMap: Record<string, string> = unsafeCharacters.reduce(
+  (acc, key) => ({ ...acc, [encodeURIComponent(key)]: key }),
+  {}
+);
+
 export interface Position {
   x: number;
   y: number;
@@ -22,6 +32,19 @@ export const cleanHtmlString = (html: string): string => {
   return cleanedHtmlString;
 };
 
+export const decodeSegmentValue = (value: string): string => encodeOrDecodeSegmentValue(value, 'decode');
+
+export const encodeSegmentValue = (value: string): string => encodeOrDecodeSegmentValue(value, 'encode');
+
+const encodeOrDecodeSegmentValue = (value: string, direction: 'encode' | 'decode'): string => {
+  const map = direction === 'encode' ? unsafeCharacterEncodingMap : unsafeCharacterDecodingMap;
+  let newValue = value;
+  Object.keys(map).forEach((key) => {
+    newValue = newValue.replaceAll(key, map[key]);
+  });
+  return newValue;
+};
+
 export const dropDownActiveClass = (active: boolean) => {
   if (active) return 'active msla-dropdown-item-active';
   else return '';