-
Notifications
You must be signed in to change notification settings - Fork 104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Utilize DOMPurify hooks to preserve <use> elements in playground mode #156
Utilize DOMPurify hooks to preserve <use> elements in playground mode #156
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR :)
playground/index.js
Outdated
@@ -1,6 +1,12 @@ | |||
const { jsPDF } = window.jspdf | |||
const DOMPurify = window.DOMPurify | |||
|
|||
DOMPurify.addHook('afterSanitizeAttributes', node => { | |||
if(node.hasAttribute('xlink:href') && !node.getAttribute('xlink:href').match(/^#/)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also check the href
attribute without the xlink
namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I didn't know that it is deprecated in SVG 2. I have amended the commit and also added playground
to prettier check.
ff40942
to
7a86f13
Compare
package.json
Outdated
@@ -25,7 +25,7 @@ | |||
"test-typescript": "karma start ./test/deployment/typescript/karma.conf.js", | |||
"test:ci": "cross-env SHOW_DIFF=true npm run test", | |||
"createreferences": "node test/common/reference-server.js", | |||
"prettier": "prettier --write {src,tests,typings}/**/*.{ts,js}", | |||
"prettier": "prettier --write {playground,src,tests,typings}/**/*.{ts,js}", | |||
"lint": "eslint {src,tests,typings}/**/*.{ts,js}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for nitpicking, but we should also add it to the lint
task then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, I updated that 👌
7a86f13
to
9880623
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. Thanks for the PR :)
Closes #155.
<use>
tags were filtered out after sanitization, but with DOMPurify hooks this problem can be solved (regular expression still filter out links to external resources in the interest of security).Can be checked in playground with following SVG code:
Before PR
After PR