Skip to content

Commit 7574742

Browse files
authored
improvement: sanitization via DOMPurify (#4513)
* swap `sanitize-html` for `dompurify` * set up node enzyme tests with jsdom dompurify, as the name suggests, needs a DOM or it won't work! * reconcile tests and sanitizer settings * remove obsolete sanitizeOptions * add `jsdom` dependency
1 parent 8055129 commit 7574742

File tree

4 files changed

+37
-25
lines changed

4 files changed

+37
-25
lines changed

package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
"test": "npm run lint-errors && npm run just-test-in-node",
3232
"test-in-node": "npm run lint-errors && npm run just-test-in-node",
3333
"just-test": "karma start --config karma.conf.js",
34-
"just-test-in-node": "mocha --recursive --compilers js:babel-core/register test/core test/components test/bugs test/swagger-ui-dist-package test/xss",
34+
"just-test-in-node": "mocha --require test/setup.js --recursive --compilers js:babel-core/register test/core test/components test/bugs test/swagger-ui-dist-package test/xss",
3535
"just-check-coverage": "nyc npm run just-test-in-node",
3636
"test-e2e": "sleep 3 && nightwatch test/e2e/scenarios/ --config test/e2e/nightwatch.json",
3737
"e2e-initial-render": "nightwatch test/e2e/scenarios/ --config test/e2e/nightwatch.json --group initial-render",
@@ -48,6 +48,7 @@
4848
"core-js": "^2.5.1",
4949
"css.escape": "1.5.1",
5050
"deep-extend": "0.4.1",
51+
"dompurify": "^1.0.4",
5152
"expect": "1.20.2",
5253
"getbase": "^2.8.2",
5354
"ieee754": "^1.1.8",
@@ -80,7 +81,6 @@
8081
"redux-logger": "*",
8182
"remarkable": "^1.7.1",
8283
"reselect": "2.5.3",
83-
"sanitize-html": "^1.14.1",
8484
"scroll-to-element": "^2.0.0",
8585
"serialize-error": "2.0.0",
8686
"shallowequal": "0.2.2",
@@ -119,6 +119,7 @@
119119
"file-loader": "0.11.2",
120120
"git-describe": "^4.0.1",
121121
"imports-loader": "0.7.1",
122+
"jsdom": "^11.10.0",
122123
"json-loader": "0.5.4",
123124
"json-server": "^0.11.0",
124125
"karma": "^1.7.0",

src/core/components/providers/markdown.jsx

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React from "react"
22
import PropTypes from "prop-types"
33
import Remarkable from "remarkable"
4-
import sanitize from "sanitize-html"
4+
import DomPurify from "dompurify"
55
import cx from "classnames"
66

77
// eslint-disable-next-line no-useless-escape
@@ -40,20 +40,8 @@ Markdown.propTypes = {
4040

4141
export default Markdown
4242

43-
const sanitizeOptions = {
44-
allowedTags: sanitize.defaults.allowedTags.concat([ "h1", "h2", "img", "span" ]),
45-
allowedAttributes: {
46-
...sanitize.defaults.allowedAttributes,
47-
"img": sanitize.defaults.allowedAttributes.img.concat(["title"]),
48-
"td": [ "colspan" ],
49-
"*": [ "class" ]
50-
},
51-
allowedSchemesByTag: { img: [ "http", "https", "data" ] },
52-
textFilter: function(text) {
53-
return text.replace(/"/g, "\"")
54-
}
55-
}
56-
5743
export function sanitizer(str) {
58-
return sanitize(str, sanitizeOptions)
44+
return DomPurify.sanitize(str, {
45+
ADD_ATTR: ["target"]
46+
})
5947
}

test/components/markdown.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,19 @@ describe("Markdown component", function() {
1616
it("allows td elements with colspan attrib", function() {
1717
const str = `<table><tr><td>ABC</td></tr></table>`
1818
const el = render(<Markdown source={str} />)
19-
expect(el.html()).toEqual(`<div class="markdown"><table><tr><td>ABC</td></tr></table></div>`)
19+
expect(el.html()).toEqual(`<div class="markdown"><table><tbody><tr><td>ABC</td></tr></tbody></table></div>`)
2020
})
2121

2222
it("allows image elements", function() {
2323
const str = `![Image alt text](http://image.source "Image title")`
2424
const el = render(<Markdown source={str} />)
25-
expect(el.html()).toEqual(`<div class="markdown"><p><img src="http://image.source" title="Image title"></p>\n</div>`)
25+
expect(el.html()).toEqual(`<div class="markdown"><p><img title="Image title" alt="Image alt text" src="http://image.source"></p>\n</div>`)
2626
})
27-
27+
2828
it("allows image elements with https scheme", function() {
2929
const str = `![Image alt text](https://image.source "Image title")`
3030
const el = render(<Markdown source={str} />)
31-
expect(el.html()).toEqual(`<div class="markdown"><p><img src="https://image.source" title="Image title"></p>\n</div>`)
31+
expect(el.html()).toEqual(`<div class="markdown"><p><img title="Image title" alt="Image alt text" src="https://image.source"></p>\n</div>`)
3232
})
3333

3434
it("allows image elements with data scheme", function() {
@@ -52,21 +52,21 @@ describe("Markdown component", function() {
5252
it("allows links", function() {
5353
const str = `[Link](https://example.com/)`
5454
const el = render(<Markdown source={str} />)
55-
expect(el.html()).toEqual(`<div class="markdown"><p><a href="https://example.com/" target="_blank">Link</a></p>\n</div>`)
55+
expect(el.html()).toEqual(`<div class="markdown"><p><a target="_blank" href="https://example.com/">Link</a></p>\n</div>`)
5656
})
5757
})
5858

5959
describe("OAS 3", function() {
6060
it("allows image elements", function() {
6161
const str = `![Image alt text](http://image.source "Image title")`
6262
const el = render(<OAS3Markdown source={str} />)
63-
expect(el.html()).toEqual(`<div class="renderedMarkdown"><div><p><img src="http://image.source" title="Image title"></p></div></div>`)
63+
expect(el.html()).toEqual(`<div class="renderedMarkdown"><div><p><img title="Image title" alt="Image alt text" src="http://image.source"></p></div></div>`)
6464
})
6565

6666
it("allows image elements with https scheme", function() {
6767
const str = `![Image alt text](https://image.source "Image title")`
6868
const el = render(<OAS3Markdown source={str} />)
69-
expect(el.html()).toEqual(`<div class="renderedMarkdown"><div><p><img src="https://image.source" title="Image title"></p></div></div>`)
69+
expect(el.html()).toEqual(`<div class="renderedMarkdown"><div><p><img title="Image title" alt="Image alt text" src="https://image.source"></p></div></div>`)
7070
})
7171

7272
it("allows image elements with data scheme", function() {

test/setup.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
const { JSDOM } = require("jsdom")
2+
const win = require("core/window")
3+
4+
const jsdom = new JSDOM("<!doctype html><html><body></body></html>")
5+
const { window } = jsdom
6+
7+
function copyProps(src, target) {
8+
const props = Object.getOwnPropertyNames(src)
9+
.filter(prop => typeof target[prop] === "undefined")
10+
.reduce((result, prop) => ({
11+
...result,
12+
[prop]: Object.getOwnPropertyDescriptor(src, prop),
13+
}), {})
14+
Object.defineProperties(target, props)
15+
}
16+
17+
global.window = window
18+
global.document = window.document
19+
global.navigator = {
20+
userAgent: "node.js",
21+
}
22+
copyProps(win, window) // use UI's built-in window wrapper
23+
copyProps(window, global)

0 commit comments

Comments
 (0)