fix(util): Fix XSS vulnerability in safeHighlight

[punzki]

Fix XSS vulnerability in safeHighlight function. Previously
vulnerable if min-length=0, load-on-empty=true, and either
load-on-focus or load-on-down-arrow =true.

Closes #491.

[punzki]

@mbenford - Can you take a look?

In #491, I added a link to the demo, but adding it here as well for posterity: http://plnkr.co/edit/KpAuRNNUORBaR4qFxhDC?p=preview

[punzki]

Also, have you considered not using $sce.trustAsHtml, and instead requiring the use of ngSanitize with ng-bind-html? The sanitize service will allow the tags you use for highlighting, and you will not have to worry about XSS.

[punzki]

@mbenford ?

[mbenford]

Sorry for not replying sooner.

First off, thanks for your contribution. I'll try to add this fix to the next release.

Also, have you considered not using $sce.trustAsHtml, and instead requiring the use of ngSanitize with ng-bind-html?

I haven't. Mostly because ngSanitize is an external dependency, and since the very beginning ngTagsInput hasn't depended on anything but Angular itself.

Please be advised I'm submitting PR 665 as an employee of Google, hence I added Google Inc. to the copyright authors.

I honestly don't know how to handle this. I personally don't feel quite comfortable in adding Google - or any other company, for that matter - to the LICENSE file. If you happen to know how other open source projects deal with situations like this, please let me know.

[punzki]

No worries about the delayed reply. Thanks for getting back to me!

Please be advised I'm submitting PR 665 as an employee of Google, hence I added Google Inc. to the copyright authors.

I honestly don't know how to handle this. I personally don't feel quite comfortable in adding Google - or any other company, for that matter - to the LICENSE file. If you happen to know how other open source projects deal with situations like this, please let me know.

I understand your concerns. If I had been contributing on my own time, the correct thing to do would be to add my own name, but in this case, I'm contributing as an employee of Google.

As far as adding new copyright holders, technically, all contributors (now including Google with this) to the package are now copyright holders. Adding "Google Inc." is just an acknowledgement of this. You are welcome to use what you want as a copyright notice, even if not all the authors are listed, but doing that just makes the notice inaccurate. Adding it to the notice also makes it easier to track who the copyright holders are, but otherwise, the commit history and this PR is also there.

If you care, that's great... the PR stays as-is. If you don't, that's also fine, we are happy to go with the flow. Just let me know and I can back out this line in the change. 😄 We just generally try to keep the copyright notices correct when we contribute unless folks tell us otherwise.

[punzki]

@mbenford - Would you like me to update the PR?

[mbenford]

Would you like me to update the PR?

Yes, please do.

About the copyright issue: just to be clear, I have never meant to hijack anyone's contributions as if they were my own. As far as my understanding of software copyright goes, since I have never requested anyone to sign a CLA, every piece of code ever contributed "belong" to their authors. My only beef with updating the LICENSE file is that's never been done before and it feels somewhat awkward to do it now. But maybe you are right about dropping a line just to acknowledge that there are multiple contributors.

[punzki]

I understand. I never meant to imply anyone was claiming anyone else's contributions. As I mentioned before, I/we just want to keep the copyright notices updated if we can.

I will just remove the additional line, as I can't really think of a proper way to attribute all contributors, with a copyright notice that also includes the year.

[punzki]

@mbenford - Pinging