feat: Test And Adjust Attribute Sanitation

Adjusting the parameter order passed to the attribute predicate.
Now that `attributeName` comes first, it is easier for predicates
just to ignore the possibly irrelevant tag-name (instead of always
having to deal with an unused parameter).

packages/ckeditor5-bbcode/__tests__/bbob/htmlSanitizer.test.ts

@@ -0,0 +1,79 @@
+import { AllowedAttributePredicate, htmlSanitizer } from "../../src/bbob/htmlSanitizer";
+import bbob from "@bbob/core/es";
+import { render } from "@bbob/html/es";
+
+describe("htmlSanitizer", () => {
+  describe("Default Configuration", () => {
+    const plugin = htmlSanitizer();
+    const processor = bbob(plugin);
+    const process = (bbCode: string): string => processor.process(bbCode, { render }).html;
+
+    it.each`
+      input                       | expected                  | comment
+      ${``}                       | ${``}                     | ${`Empty string handling`}
+      ${`Lorem`}                  | ${`Lorem`}                | ${`unsuspicious string handling`}
+      ${`[b]T[/b]`}               | ${`<b>T</b>`}             | ${`unsuspicious element handling`}
+      ${`<b>T</b>`}               | ${`&lt;b&gt;T&lt;/b&gt;`} | ${`encode HTML elements by default`}
+      ${`[b onclick=xss()]T[/b]`} | ${`<b>T</b>`}             | ${`remove suspicious attributes`}
+      ${`[b oNcLick=xss()]T[/b]`} | ${`<b>T</b>`}             | ${`remove suspicious attributes`}
+    `(
+      "[$#] $comment: Should process '$input' to '$expected'",
+      ({ input, expected }: { input: string; expected: string }) => {
+        expect(process(input)).toBe(expected);
+      },
+    );
+  });
+
+  /**
+   * While not recommended in context as CKEditor 5-Plugin, this customization
+   * option matches a similar feature of Markdown. And: Internally it clearly
+   * shows where escaping happens and why.
+   */
+  describe("Allow HTML", () => {
+    const plugin = htmlSanitizer({ allowHtml: true });
+    const processor = bbob(plugin);
+    const process = (bbCode: string): string => processor.process(bbCode, { render }).html;
+
+    it.each`
+      input                       | expected      | comment
+      ${``}                       | ${``}         | ${`Empty string handling`}
+      ${`Lorem`}                  | ${`Lorem`}    | ${`unsuspicious string handling`}
+      ${`[b]T[/b]`}               | ${`<b>T</b>`} | ${`unsuspicious element handling`}
+      ${`<b>T</b>`}               | ${`<b>T</b>`} | ${`enabled by option: let HTML elements pass through`}
+      ${`[b onclick=xss()]T[/b]`} | ${`<b>T</b>`} | ${`still: remove suspicious attributes`}
+    `(
+      "[$#] $comment: Should process '$input' to '$expected'",
+      ({ input, expected }: { input: string; expected: string }) => {
+        expect(process(input)).toBe(expected);
+      },
+    );
+  });
+
+  describe("Custom Attribute Filter", () => {
+    /**
+     * Allows only `class` attribute for `b` and `i` tags. Allows `id`
+     * attribute for any tag.
+     */
+    const isAllowedAttribute: AllowedAttributePredicate = (attr: string, tag: string) => {
+      if (["b", "i"].includes(tag)) {
+        return attr === "class";
+      }
+      return attr === "id";
+    };
+    const plugin = htmlSanitizer({ isAllowedAttribute });
+    const processor = bbob(plugin);
+    const process = (bbCode: string): string => processor.process(bbCode, { render }).html;
+
+    it.each`
+      input                           | expected                    | comment
+      ${`[b class=CLASS]T[/b]`}       | ${`<b class="CLASS">T</b>`} | ${`b.class passes custom filter`}
+      ${`[u class=CLASS]T[/u]`}       | ${`<u>T</u>`}               | ${`u.class is blocked by custom filter`}
+      ${`[u class=CLASS id=ID]T[/u]`} | ${`<u id="ID">T</u>`}       | ${`u.class is blocked by custom filter, only u.id passes`}
+    `(
+      "[$#] $comment: Should process '$input' to '$expected'",
+      ({ input, expected }: { input: string; expected: string }) => {
+        expect(process(input)).toBe(expected);
+      },
+    );
+  });
+});

packages/ckeditor5-bbcode/src/bbob/htmlSanitizer.ts

@@ -2,13 +2,19 @@ import { CorePlugin, CoreTree } from "@bbob/core/es";
 import { escapeHTML, getUniqAttr, isTagNode } from "@bbob/plugin-helper/es";
 
 /**
- * Default attribute filter. Only forbids any `on*` handlers.
+ * Predicate for filtering attributes. This predicate is not applied to
+ * unique attributes.
  *
- * @param tagName - owning tag name
  * @param attributeName - name of attribute
+ * @param tagName - owning tag name
+ */
+export type AllowedAttributePredicate = (attributeName: string, tagName: string) => boolean;
+
+/**
+ * Default attribute filter. Only forbids any `on*` handlers.
  */
-export const defaultIsAllowedAttribute = (tagName: string, attributeName: string): boolean =>
-  !attributeName.startsWith("on");
+export const defaultIsAllowedAttribute: AllowedAttributePredicate = (attributeName: string): boolean =>
+  !attributeName.toLowerCase().startsWith("on");
 
 export interface HtmlSanitizerOptions {
   /**
@@ -24,7 +30,7 @@ export interface HtmlSanitizerOptions {
    * @param tagName - owning tag name
    * @param attributeName - name of attribute
    */
-  isAllowedAttribute?: typeof defaultIsAllowedAttribute;
+  isAllowedAttribute?: AllowedAttributePredicate;
 }
 
 const walk = <T extends CoreTree | CoreTree[number] | CoreTree[number][]>(
@@ -51,7 +57,7 @@ const walk = <T extends CoreTree | CoreTree[number] | CoreTree[number][]>(
       // by object.
       item.attrs = Object.fromEntries(
         Object.entries(item.attrs).filter(
-          ([attrName]) => uniqAttr === attrName || options.isAllowedAttribute(item.tag, attrName),
+          ([attrName]) => uniqAttr === attrName || options.isAllowedAttribute(attrName, item.tag),
         ),
       );
     } else {