Repository: apostrophecms/sanitize-html
Pull request #623 (2023-05-26 09:37:01)

Bug Fix: allow false in allowedClasses
<!-- Please don't delete this template -->

<!-- PULL REQUEST TEMPLATE -->
<!-- (Update "[ ]" to "[x]" to check a box) -->

## Summary
Prior to 2.4.0, `allowedClasses` can be false, this has been an undocumented feature.
Since 2.4.0, this behavior is broken. This PR reverted to the previous behavior where `false` is allowed in the `allowedClasses` attribute.


See https://github.com/apostrophecms/sanitize-html/discussions/621 for more detail

## What are the specific steps to test this change?
The following code is broken in the main branch of this base repo, but should work in the branch in this pr.
 
```javascript
const sanitizeHtml = require('sanitize-html');

const ALLOWED_TAGS = [
    'li',
    'ul',
    'ol',
    'p',
    'h1',
    'h2',
    'h3',
    'h4',
    'h5',
    'h6',
    'img',
    'a',
    'div',
    'b',
    'table',
    'tbody',
    'tr',
    'th',
    'td'
];
const ALLOWED_SCHEMES = ['https', 'mailto'];

const sanitized = sanitizeHtml(`<ul class="should not sanitize"><li>Hello </li></ul>`, {
    allowedAttributes: { '*': ['*'] },
    allowedTags: ALLOWED_TAGS,
    allowedClasses: {
        ul: false
    },
    allowProtocolRelative: false,
    allowedSchemes: ALLOWED_SCHEMES
});
//expected <ul class="should not sanitize"><li>Hello </li></ul>
console.log(sanitized)

```

## What kind of change does this PR introduce?
*(Check at least one)*

- [x] Bug fix
- [ ] New feature
- [ ] Refactor
- [ ] Documentation
- [ ] Build-related changes
- [ ] Other

## Make sure the PR fulfills these requirements:

- [x] It includes a) the existing issue ID being resolved, b) a convincing reason for adding this feature, or c) a clear description of the bug it resolves
- [x] The changelog is updated
- [x] Related documentation has been updated
- [x] Related tests have been updated



**Other information:**



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 394fdbe..59e926c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## UNRELEASED
 
+- Fix to allow `false` in `allowedClasses` attributes
 - Upgrade mocha version
 - Apply small linter fixes in tests
 - Add `.idea` temp files to `.gitignore`
diff --git a/README.md b/README.md
index c51512d..d18237c 100644
--- a/README.md
+++ b/README.md
@@ -255,6 +255,8 @@ allowedClasses: {
 }
 ```
 
+If `allowedClasses` for a certain tag is `false`, all the classes for this tag will be allowed.
+
 > Note: It is advised that your regular expressions always begin with `^` so that you are requiring a known prefix. A regular expression with neither `^` nor `$` just requires that something appear in the middle.
 
 ### Allowed CSS Styles
diff --git a/index.js b/index.js
index ad9c6ad..63e38bd 100644
--- a/index.js
+++ b/index.js
@@ -170,20 +170,24 @@ function sanitizeHtml(html, options, _recursing) {
       allowedAttributesMap[tag].push('class');
     }
 
-    allowedClassesMap[tag] = [];
-    allowedClassesRegexMap[tag] = [];
-    const globRegex = [];
-    classes.forEach(function(obj) {
-      if (typeof obj === 'string' && obj.indexOf('*') >= 0) {
-        globRegex.push(escapeStringRegexp(obj).replace(/\\\*/g, '.*'));
-      } else if (obj instanceof RegExp) {
-        allowedClassesRegexMap[tag].push(obj);
-      } else {
-        allowedClassesMap[tag].push(obj);
+    allowedClassesMap[tag] = classes;
+
+    if (Array.isArray(classes)) {
+      const globRegex = [];
+      allowedClassesMap[tag] = [];
+      allowedClassesRegexMap[tag] = [];
+      classes.forEach(function(obj) {
+        if (typeof obj === 'string' && obj.indexOf('*') >= 0) {
+          globRegex.push(escapeStringRegexp(obj).replace(/\\\*/g, '.*'));
+        } else if (obj instanceof RegExp) {
+          allowedClassesRegexMap[tag].push(obj);
+        } else {
+          allowedClassesMap[tag].push(obj);
+        }
+      });
+      if (globRegex.length) {
+        allowedClassesGlobMap[tag] = new RegExp('^(' + globRegex.join('|') + ')$');
       }
-    });
-    if (globRegex.length) {
-      allowedClassesGlobMap[tag] = new RegExp('^(' + globRegex.join('|') + ')$');
     }
   });
 


Test patch:
diff --git a/test/test.js b/test/test.js
index 753114d..8bbbd47 100644
--- a/test/test.js
+++ b/test/test.js
@@ -482,6 +482,20 @@ describe('sanitizeHtml', function() {
       '<p class="nifty simple dippy">whee</p>'
     );
   });
+  it('should allow all classes for a single tag if `allowedClasses` for the tag is false', function() {
+    assert.equal(
+      sanitizeHtml(
+        '<p class="nifty simple dippy">whee</p>',
+        {
+          allowedTags: [ 'p' ],
+          allowedClasses: {
+            p: false
+          }
+        }
+      ),
+      '<p class="nifty simple dippy">whee</p>'
+    );
+  });
   it('should allow only classes that matches `allowedClasses` regex', function() {
     assert.equal(
       sanitizeHtml(