Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Typeahead use bind-html-unsafe #2884

Closed
albertein opened this issue Oct 24, 2014 · 10 comments
Closed

Typeahead use bind-html-unsafe #2884

albertein opened this issue Oct 24, 2014 · 10 comments

Comments

@albertein
Copy link

The typeahead directive uses bind-html-unsafe for the template of matches, https://github.com/angular-ui/bootstrap/blob/master/template/typeahead/typeahead-match.html.

The property on the directive doesn't indicate on any way that it can be dangerous, it should either:

a) Remove html-bind-unsafe entirely
b) Having typeahead use html-bind-unsafe should be a parameter set explicitly.

I would be happy to do any of those.

@chrisirhc
Copy link
Contributor

I think we should remove its use entirely. To do this we'll need to:

  1. Switch to use ng-bind-html in the typeahead-match.html template
  2. Modify typeaheadHighlight:
    1. If the matchItem.toString() contains a starting tag (< followed by >), typeaheadHighlight should use [$sce.getTrustedHtml] on the matchItem
    2. If $sanitize service exists, call $sce.getTrustedHtml on the output. Otherwise, if it doesn't exist, call $sce.trustAsHtml on the output.
  3. Add Breaking Changes message to note to users to make sure that matchItem labels that contain HTML must be made trusted using the $sce.trustAsHtml method.

This sequence should be safe because if $sanitize doesn't exist and the matchItem string contains a tag, an exception will be thrown. Otherwise, if the matchItem string doesn't contain a tag, there's no need to sanitize it.

@fernando-sendMail
Copy link
Contributor

Hello:
I found this issue too, and since I need to create a workaround for this I want to work out this this weekend. Is anybody working on this currently or is it done yet?

If not, let me know so I can do it, and do a pull request.

On a side note I've never made a contribution, do you have any additional guidelines besides the ones posted on github, say for this project in particular?

Thanks for your time!

@karianna
Copy link
Contributor

Nothing special, except for make sure it's covered by tests :-) go for it!

@nschloe
Copy link

nschloe commented Aug 3, 2015

This is also popping up for our project; a fix for this would be appreciated. :)

@erik-slack
Copy link

Same here.

@Frondor
Copy link

Frondor commented Aug 5, 2015

bindHtmlUnsafe is now deprecated. Use ngBindHtml instead (Google Chrome console warning)

@andrzejpiasecki
Copy link

Same here.

@kikar
Copy link

kikar commented Aug 13, 2015

+1

@bambooyzj
Copy link

same here

@wesleycho wesleycho modified the milestones: 0.13.4 (Performance), Backlog Aug 18, 2015
@wesleycho
Copy link
Contributor

This is now removed in master, and should be resolved in the next update.

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

No branches or pull requests