Repository: apostrophecms/sanitize-html (js)

Issue:
Allow escaping of more than just tags
## The problem to solve

I would like the sanitised string to be displayed exactly as input. Discarding text is not helpful, as it will be missing.
`options.disallowedTagsMode = 'recursiveEscape'` already exists to escape tags instead of removing them. Attributes, etc. are still removed instead of escaped though.

User Story:
1. We start with (mostly) everything disallowed.
2. User attempts to input HTML, sees the resulting text and copy&pastes directly into a feature request to allow the tag/attribute/style/whatever they want and do not get.
3. (If the requested element can be safely allowed) we copy&paste the text directly into our mock project for TDD.
4. loop 2., 3.
5. Profit!

## Proposed solution

I, personally, would like a simple switch `options.disallowedEverythingMode: 'discard' | 'escape' | 'recursiveEscape'` or similar.

## Alternatives

Mirroring the lists and records of `options` akin to how the existing `options.disallowedTagsMode` mirrors `options.allowedTags` is probably the more sensible approach:
```
options.disallowedAttributesMode: 'discard' | 'escape' | 'recursiveEscape'
options.disallowedStylesMode: 'discard' | 'escape' | 'recursiveEscape'
<etc.>
```
Mayhap a synthetic "all the things" could be added as a function on top of those:
`function setDisallowedEverythingMode(mode: 'discard' | 'escape' | 'recursiveEscape') {/* set every flag to mode */}`

## Additional context

```
options: {
  allowedAttributes: {},
  allowedTags: [],
  disallowedTagsMode: 'recursiveEscape',
  nonTextTags: [],
  selfClosing: [],
}
```

Input: `"Cursed Project <img src=x onerror=alert('AllYourBaseAreBelongToUs!');>"`
Expected: `Cursed Project <img src=x onerror=alert('AllYourBaseAreBelongToUs!');>`
Displayed: `Cursed Project <img></img>`

Pull request:
Add `preserveEscapedAttributes` option to retain all attributes of escaped disallowed tags
Fixes https://github.com/apostrophecms/sanitize-html/issues/540

<!-- Please don't delete this template -->

<!-- PULL REQUEST TEMPLATE -->
<!-- (Update "[ ]" to "[x]" to check a box) -->

## Summary

*Summarize the changes briefly, including which issue/ticket this resolves. If it closes an existing Github issue, include "Closes #[issue number]"*

Add `preserveEscapedAttributes` option to allow attributes on escaped disallowed tags to be retained. Closes #540 

## What are the specific steps to test this change?

You can try the new behaviour by setting `preserveEscapedAttributes` to `true` when `disallowedTagsMode` is `'escape'` or `'recursiveEscape'`.

Added unit tests to PR. 

## What kind of change does this PR introduce?
*(Check at least one)*

- [ ] Bug fix
- [x] 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
- [ ] The changelog is updated
- [x] Related documentation has been updated
- [x] Related tests have been updated

If adding a new feature without an already open issue, it's best to open a **feature request issue** first and wait for approval before working on it.

**Other information:**


Patch:
diff --git a/README.md b/README.md
index 903c95e..6600332 100644
--- a/README.md
+++ b/README.md
@@ -745,7 +745,7 @@ This will transform `<disallowed>content <allowed>content</allowed> </disallowed
 
 #### Escape the disallowed tag and all its children even for allowed tags.
 
-if you set `disallowedTagsMode` to `recursiveEscape`, disallowed tag and its children will be escaped even for allowed tags
+if you set `disallowedTagsMode` to `recursiveEscape`, disallowed tags and their children will be escaped even for allowed tags:
 
 ```js
 disallowedTagsMode: `recursiveEscape`
@@ -753,6 +753,15 @@ disallowedTagsMode: `recursiveEscape`
 
 This will transform `<disallowed>hello<p>world</p></disallowed>` to `&lt;disallowed&gt;hello&lt;p&gt;world&lt;/p&gt;&lt;/disallowed&gt;`
 
+#### Escape the disallowed tag, including all its attributes.
+
+By default, attributes are not preserved when tags are escaped. You can set `preserveEscapedAttributes` to `true` to 
+keep the attributes, which will also be escaped and therefore have no effect on the browser.
+
+```js
+preserveEscapedAttributes: true
+```
+
 ### Ignore style attribute contents
 
 Instead of discarding faulty style attributes, you can allow them by disabling the parsing of style attributes:
diff --git a/index.js b/index.js
index ebb741e..348ec00 100644
--- a/index.js
+++ b/index.js
@@ -300,7 +300,14 @@ function sanitizeHtml(html, options, _recursing) {
         }
       }
 
-      if (!allowedAttributesMap || has(allowedAttributesMap, name) || allowedAttributesMap['*']) {
+      const isBeingEscaped = skip && (options.disallowedTagsMode === 'escape' || options.disallowedTagsMode === 'recursiveEscape');
+      const shouldPreserveEscapedAttributes = isBeingEscaped && options.preserveEscapedAttributes;
+
+      if (shouldPreserveEscapedAttributes) {
+        each(attribs, function(value, a) {
+          result += ' ' + a + '="' + escapeHtml((value || ''), true) + '"';
+        });
+      } else if (!allowedAttributesMap || has(allowedAttributesMap, name) || allowedAttributesMap['*']) {
         each(attribs, function(value, a) {
           if (!VALID_HTML_ATTRIBUTE_NAME.test(a)) {
             // This prevents part of an attribute name in the output from being
@@ -923,7 +930,8 @@ sanitizeHtml.defaults = {
   allowedSchemesAppliedToAttributes: [ 'href', 'src', 'cite' ],
   allowProtocolRelative: true,
   enforceHtmlBoundary: false,
-  parseStyleAttributes: true
+  parseStyleAttributes: true,
+  preserveEscapedAttributes: false
 };
 
 sanitizeHtml.simpleTransform = function(newTagName, newAttribs, merge) {


Test patch:
diff --git a/test/test.js b/test/test.js
index efd02ce..c43587e 100644
--- a/test/test.js
+++ b/test/test.js
@@ -1811,4 +1811,41 @@ describe('sanitizeHtml', function() {
     );
     assert.equal(sanitizedHtml, expectedOutput);
   });
+  it('should not preserve attributes on escaped disallowed tags when `preserveEscapedAttributes` is false', () => {
+    const inputHtml = '<div class="foo">Some Text</div>';
+    const expectedOutput = '&lt;div&gt;Some Text&lt;/div&gt;';
+    const sanitizedHtml = sanitizeHtml(inputHtml, {
+      allowedTags: [],
+      disallowedTagsMode: 'escape',
+      preserveEscapedAttributes: false
+    });
+
+    assert.equal(sanitizedHtml, expectedOutput);
+  });
+  it('should preserve attributes on escaped disallowed tags when `preserveEscapedAttributes` is true', () => {
+    const inputHtml = '<div class="foo">Some Text</div>';
+    const expectedOutput = '&lt;div class="foo"&gt;Some Text&lt;/div&gt;';
+    const sanitizedHtml = sanitizeHtml(inputHtml, {
+      allowedTags: [],
+      disallowedTagsMode: 'escape',
+      preserveEscapedAttributes: true
+    });
+
+    assert.equal(sanitizedHtml, expectedOutput);
+  });
+  it('should ignore the `preserveEscapedAttributes` option when discarding diallowed tags (rather than escaping)', () => {
+    const inputHtml = '<div class="foo">Some Text</div>';
+    const sanitizedHtmlPreservedAttrsTrue = sanitizeHtml(inputHtml, {
+      allowedTags: [],
+      disallowedTagsMode: 'discard',
+      preserveEscapedAttributes: true
+    });
+    const sanitizedHtmlPreservedAttrsFalse = sanitizeHtml(inputHtml, {
+      allowedTags: [],
+      disallowedTagsMode: 'discard',
+      preserveEscapedAttributes: false
+    });
+
+    assert.equal(sanitizedHtmlPreservedAttrsTrue, sanitizedHtmlPreservedAttrsFalse);
+  });
 });