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

Enhancing ngSanitize whitelists #5900

Closed
bradyisom opened this issue Jan 20, 2014 · 26 comments
Closed

Enhancing ngSanitize whitelists #5900

bradyisom opened this issue Jan 20, 2014 · 26 comments

Comments

@bradyisom
Copy link
Contributor

There needs to be a way to extend the ngSanitize whitelists of elements and attributes. In our project, we need to add additional valid HTML5 elements to the list of valid elements (like video and audio). We also need to add some of our custom elements to the list.

I am willing to do the work and submit a PR. However, I want to make sure I do it the right way.

I was thinking of adding something similar to $compileProvider's aHrefSanitizationWhitelist and imgSrcSanitizationWhitelist that manages the voidElements, optionalEndTagBlockElements, optionalEndTagInlineElements, blockElements, inlineElements, specialElements, uriAttrs, and validAttrs lists within the $sanitize implementation.

I would probably make them as methods that take an array of additional whitelisted elements or attributes. These would then be used to extend the current whitelists. This could be implemented on $compileProvider or on a new provider inside the $sanitize module.

Another option would be to make the new methods take the entire whitelist. This would allow the caller to remove some elements or attributes that are already in the existing whitelists. However, the current whitelists would then have to be documented. The caller would also have to pass in the entire existing whitelist with their additions if all they wanted to do was add and element or attribute.

@bradyisom
Copy link
Contributor Author

As a side comment, when our team realized that we needed this feature, our first reaction was to build our own version of ngSanitize and just use it in our project. However, after recent experiences, we decided to pursue the path of contributing to the project.

One experience was my recent streamlined experience with submitting a small pull request and having that PR show up within a week in the master branch. Knowing that changes we contribute to the project will be available in a timely manner is essential for our team.

The other experience was listening to @IgorMinar 's heartfelt talk at ng-conf. Knowing the commitment of the Angular team to work with the community inspired me to follow the contribution path. I was glad I was able to be at the conference.

Thank you for all of your hard work and commitment to this wonderful project.

@OliverJAsh
Copy link

I have exactly the same needs. Not much help right now, but I might look further into this.

bradyisom added a commit to bradyisom/angular.js that referenced this issue Feb 13, 2014
Add $sanitizeExt factory to allow extenstion of the $sanitize element and
attribute whitelists. This allows for unsupported, standard elements and
attributes, as well as custom elements and attributes.

Closes angular#5900
bradyisom added a commit to bradyisom/angular.js that referenced this issue Feb 14, 2014
Add $sanitizeExt factory to allow extenstion of the $sanitize element and
attribute whitelists. This allows for unsupported, standard elements and
attributes, as well as custom elements and attributes.

Closes angular#5900
@OliverJAsh
Copy link

My two cents:

It seems logical that $sanitize should have its own provider whereby you can customise the whitelists. In my mind it makes more sense for the current whitelists of $compileProvider, i.e. aHrefSanitizationWhitelist, to be part of $sanitizeProvider instead. However ngSanitize is not part of the core. Could that be why it wasn’t done like that?

I definitely prefer your last suggestion, whereby we’d allow the consumer to override the whitelist completely to add or remove items.

@bradyisom
Copy link
Contributor Author

Providing a method to override the entire whitelist seemed too cumbersome. The user would have to pass the entire list. Also, there are multiple underlying lists that the override functions in the PR update. We'd have to supply an override function for each of these internal lists if we went down that path.

If we want the ability to remove tags/attributes, I would suggest creating "remove" functions in addition to the "add" functions that are in the PR.

bradyisom pushed a commit to bradyisom/angular.js that referenced this issue Apr 25, 2014
Add $sanitizeExt factory to allow extenstion of the $sanitize element
and
attribute whitelists. This allows for unsupported, standard elements and
attributes, as well as custom elements and attributes.

Closes angular#5900
bradyisom added a commit to bradyisom/angular.js that referenced this issue Apr 28, 2014
Add $sanitizeExt factory to allow extenstion of the $sanitize element and
attribute whitelists. This allows for unsupported, standard elements and
attributes, as well as custom elements and attributes.

Closes angular#5900
bradyisom added a commit to bradyisom/angular.js that referenced this issue Apr 28, 2014
Add $sanitizeExt factory to allow extenstion of the $sanitize element and
attribute whitelists. This allows for unsupported, standard elements and
attributes, as well as custom elements and attributes.

Closes angular#5900
bradyisom added a commit to bradyisom/angular.js that referenced this issue Apr 28, 2014
Add $sanitizeExt factory to allow extenstion of the $sanitize element and
attribute whitelists. This allows for unsupported, standard elements and
attributes, as well as custom elements and attributes.

Closes angular#5900
bradyisom added a commit to bradyisom/angular.js that referenced this issue Apr 28, 2014
Add $sanitizeExt factory to allow extenstion of the $sanitize element and
attribute whitelists. This allows for unsupported, standard elements and
attributes, as well as custom elements and attributes.

Closes angular#5900
bradyisom added a commit to bradyisom/angular.js that referenced this issue Apr 28, 2014
Add $sanitizeExt factory to allow extenstion of the $sanitize element and
attribute whitelists. This allows for unsupported, standard elements and
attributes, as well as custom elements and attributes.

Closes angular#5900
@bvaughn
Copy link

bvaughn commented Jul 5, 2014

+1

Spent the day going down the rabbit hole of trying to decorate $sanitize to add this ability. Much nicer if it were supported by ngSanitize in the first place. Doesn't seem like it would be that drastic of a change.

@crisbeto
Copy link
Member

+1

Awesome! Tried it out today and it works great, also I agree with @OliverJAsh that it would be better if it had a provider.

@caitp
Copy link
Contributor

caitp commented Sep 9, 2014

@bvaughn why not submit a pull request? thanks

@bvaughn
Copy link

bvaughn commented Sep 9, 2014

Great suggest, @caitp. I'll try to get to it sometime this week. :)

@saiichihashimoto
Copy link

+1

Is this still being worked on?

@SyntaxRules
Copy link
Contributor

+1

4 similar comments
@johnnncodes
Copy link

+1

@vontobel
Copy link

+1

@jordajm
Copy link

jordajm commented Aug 3, 2015

+1

@hellsan631
Copy link

+1

@Anaphase
Copy link

+1 I'd love to be able to set my own whitelist. I want to bind HTML with <span> tags and nothing else.

@petebacondarwin petebacondarwin modified the milestones: 1.5.x - migration-facilitation, 1.3.x - superluminal-nudge Sep 9, 2015
@petebacondarwin
Copy link
Contributor

Moving to 1.5 - as noted in #6252 (comment) the solution would be implemented via provider methods.

@manuel-di-iorio
Copy link

+1

@cciatti
Copy link

cciatti commented May 17, 2016

+1
I added it on my own copy but I hate to do that... please add this to the core soon. Also, add a method to extend the attribute list.

@gkalpak
Copy link
Member

gkalpak commented May 17, 2016

The [PRs plz!] label is there !
Just saying... 😛

@jmanico
Copy link

jmanico commented Nov 30, 2016

This is a very critical feature for enhancing security of angular and I would be glad to offer a financial bounty or place to crash in Hawaii where I live for anyone who gets it done...

@jmanico
Copy link

jmanico commented Nov 30, 2016

@bvaughn is there a way to pull this off today, albeit in a non graceful way? If so would you be so kind as to provide an example here? Thank you!

@bvaughn
Copy link

bvaughn commented Nov 30, 2016

Sorry @jmanico. No clue. This is pretty far back in my memory and I haven't worked with an Angular 1.x tech stack in over a year. ☹️

@chris-denning
Copy link

+1

3 similar comments
@jlopezr
Copy link

jlopezr commented Feb 3, 2017

+1

@wangfengming
Copy link

+1

@nikuelias
Copy link

+1

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