angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.83k stars 27.51k forks source link

If you use the linky filter in ngSanitize any html will be rendered as escaped html #9586

Open karl-gustav opened 10 years ago

karl-gustav commented 10 years ago
Overview of the Issue

If you send the text <div>text</div>http://vg.no though the linky filter the resulting text will be:

&lt;div&gt;text&lt;/div&gt;<a href="http://vg.no">http://vg.no</a>
Motivation for or Use Case

What I would expect was

<div>text</div><a href="http://vg.no">http://vg.no</a>

because after all that's why you use the ngBindHtml directive, to put unescaped html into the page.

Angular Version(s)

I have reproduced the bug in angular version s 1.2.25 and 1.3.0-rc.5

Browsers and Operating System

I have only tested this bug in:

Plunker to prove the bug, and that it is sanitizeText() that causes it in angular-1.2.25 and angular-1.3.0-rc.5

Suggest a Fix

The bug seems to come from line 134 in the source code when calling the global function sanitizeText(). But since $santitize() is called on line 124 it seems redundant to call sanitizeText(). But I have not read the source code to confirm my suspicion, it's just that the funciton names seems to overlap.

Related Issues

I have not found any issues about the linky filter that looks like this bug.

lorenzhs commented 9 years ago

Uh, should linky really be called on HTML strings? I don't think it's equipped to handle DOM things at all.

karl-gustav commented 9 years ago

@lorenzhs : So the plugin/library ngSanitize, whose sole purpose is to enable html bindings, has a filter that don't support html? That makes no sense at all. And since it has been added to the backlog as a bug I'm guessing I'm not the only one that thinks that.

lorenzhs commented 9 years ago

@Karl-Gustav according to https://github.com/angular/angular.js/pull/3285#issuecomment-21958391, "notice that it doesn't take html as input, but text." -- I've had trouble with that in the past as well.

karl-gustav commented 9 years ago

@lorenzhs I see now that the bug is marked as "severity: broken expected use" so you might be right. Lets just hope they fix it to parse html in either case:-)

lorenzhs commented 9 years ago

@Karl-Gustav probably not going to happen, if what was written in the issue I linked still applies:

Now if you or others care about this functionality, then I suggest that you make it a project of its own and public is in bower so that others can reuse it.

Narretz commented 9 years ago

I labeled it as "broken expected use" because it seemed to me this way. But the docs also say it only takes text input. So I guess this is currently working as expected, but if it's possible to maintain a secure conversion with html input, then I don't see why it can't be a feature request at least. As far as I can tell, the linked issue is slightly different, as the OP thought linky should sanitize the html. Imo, linky should only convert text links to html links, and leave the rest of the input alone.

lorenzhs commented 9 years ago

Yeah that's how it is specified at the moment.

Regarding the feature request, I implemented a "meta-filter" that applies a filter to all text nodes in an HTML string: https://github.com/glowing-bear/glowing-bear/blob/5df0ce21a26681fb30184ab1bc4c325aa1bcad79/js/filters.js#L62 -- this is probably what @Karl-Gustav wants (use it as {{ foo | DOMfilter:'linky' }}, doesn't support parameters to the filter but that shouldn't be too hard to add.

EDIT: Updated the URL because of an escaping issue

amerikan commented 9 years ago

Using linky filter on the following:

I like http://www.google.com & many others

gets converted to:

I like <a href="http://www.google.com">http://www.google.com</a> &amp; many others

as you can see the & gets encoded to &amp; :disappointed:

Anyone has a solution or know if this will be fixed sometime? Or a better filter that will convert URL's to links without this bug?

lorenzhs commented 9 years ago

@amerikan that is intended behaviour, ngSanitize does escaping of HTML characters. Why is this a problem for you?

danieljsinclair commented 9 years ago

I also believe it should leave existing HTML alone and just convert the links. I don't see why it needs to perform two functions and seems in conflict with the single responsibility principle IMHO.

I'm trying to use the linky filter to convert links entered by the user in a contenteditable where HTML is also allowed. I can sanitize the HTML just fine but in order to use linky on the content I would need to RegExp out the links minus the HTML first which defeats the purpose of using linky in the first place.

With it working this way I'll have to abandon it and go roll something else instead.

hitesh358 commented 9 years ago

Using custom filters may help you in this case.

app.filter('parseUrlFilter', function() {
var urlPattern = /(www|http|ftp|https):\/\/[\w-]+(\.[\w-]+)+([\w.,@?^=%&amp;:\/~+#-]*[\w@?^=%&amp;\/~+#-])?/gi;
return function(text, target, otherProp) {
    target = "_blank";
    angular.forEach(text.match(urlPattern), function(url) {
        var urlPos = text.indexOf(url);
        if(text.substr(urlPos-6,4).indexOf('src') == -1){
            text = text.replace(url, "<a target=\"" + target + "\" href="+ url + " style='color:#4682B4'>" + url +"</a>");
        }
    });
    return text;
};
});
danieljsinclair commented 9 years ago

I similarly had to abandon linky, but rather than rolling a regex of my own I delegated to AutoLinker which works brilliantly. The one thing it does well that most other systems don't is it correctly interprets existing coded HTML without recoding it.

I implemented a one liner conversion in a contenteditable directive I have.

    html = Autolinker.link(html, "_blank");
shyamal890 commented 8 years ago

@danieljsinclair Thanks a lot for pointing to AutoLinker! It works great. For people wanting to render clickable links in textarea, checkout this PLUNKER

anshulsojatia commented 7 years ago

To solve this, I just created another simple filter to unescape the html characters app.filter('unescape', function () { return function (html) { return angular.element("<div/>").html(html).text(); } });

and passed the output of linky to this to reconvert those escaped characters {{myhtml | linky | unescape}} It worked for me with minimal change