Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

ngSanitize remove valids SVG elements #9578

Closed
eXon opened this issue Oct 12, 2014 · 4 comments
Closed

ngSanitize remove valids SVG elements #9578

eXon opened this issue Oct 12, 2014 · 4 comments

Comments

@eXon
Copy link

eXon commented Oct 12, 2014

ngSanitize does not support some very basics SVG elements.

An example of something not working with ng-bind-html:

ng-bind-html="data"

$scope.data = '<circle cx="50" cy="50" r="40" stroke="black" stroke-width="3" fill="red" />';

Their is many legits SVG elements that are not supported.

Their is an extensive list here: https://developer.mozilla.org/en-US/docs/Web/SVG/Element

I think ngSanitize should keep all of them. I can't think of any security reasons why we shouldn't.

The only though I have is wether we want to keep ngSanitize only for html5 or should we support more. SVG is being used a lot with angular now.

@IgorMinar
Copy link
Contributor

yeah this sounds reasonable. anyone wants to send a clean PR that whitelists legit svg elements.

@eXon
Copy link
Author

eXon commented Oct 13, 2014

Perfect! I will take care of it and follow the PR checklist :)

christianliebel pushed a commit to christianliebel/angular.js that referenced this issue Oct 22, 2014
SVG elements and attributes are now accepted and sanitized by ngSanitize.

Closes angular#9578
christianliebel pushed a commit to christianliebel/angular.js that referenced this issue Oct 22, 2014
SVG elements and attributes are now accepted and sanitized by ngSanitize.

Closes angular#9578
@caitp caitp closed this as completed in a54b25d Oct 23, 2014
@eXon
Copy link
Author

eXon commented Oct 23, 2014

Well you've been faster than me. Thank you for the fix :-)

@christianliebel
Copy link
Contributor

@eXon You're welcome. That was a nice task to accomplish during ngEurope. ;-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants