Bug breaking links: linkUrlsInTrustedHtml does not handle entities correctly

[Llbe]

$urlLinker = new Youthweb\UrlLinker\UrlLinker();

var_dump ($urlLinker->linkUrlsInTrustedHtml ("http://foo.bar/baz?a=b&a=b"));

The result here is
string(56) "<a href="http://foo.bar/baz?a=b">foo.bar/baz</a>&amp;a=b"

The expected result is
string(56) "<a href="http://foo.bar/baz?a=b&amp;a=b">foo.bar/baz</a>"

This is a bug since '&' must be encoded in HTML (to avoid conflict with the encoding) and linkUrlsInTrustedHtml() expects valid HTML.

[Art4]

Thanks for this issue but I can't identify this as a bug.

http://foo.bar/baz?a=b&a=b isn't a valid url, because it is already encoded (i.e with htmlspecialchars()).
So this url must be either inside a html tag, not encoded as http://foo.bar/baz?a=b&a=b or urlencoded as http://foo.bar/baz?a=b&amp%3ba=b.

I've written tests for all these cases, see 8ad91f6

[Llbe]

Thanks for your reply!

It's not a valid URL but it is perfectly valid HTML.

The HTML is http://foo.bar/baz?a=b&a=b

The URL is http://foo.bar/baz?a=b&a=b

And the result should be <a href="http://foo.bar/baz?a=b&amp;a=b">foo.bar/baz</a>. You are avoding the double encoding already, in escapeHtml().

This is the only way if accepting valid HTML in linkUrlsInTrustedHtml().

[Art4]

This would be valid HTML if it was already inside a HTML tag like <a href="http://foo.bar/baz?a=b&amp;a=b">.... But it isn't, so it is neither valid HTML nor a valid url.

In this case you have to use $urlLinker->linkUrlsAndEscapeHtml('http://foo.bar/baz?a=b&a=b') what works fine. See tests in 2492474.

[Llbe]

So you are saying that you can only use & inside an attribute in HTML?

Eg the following would not be valid HTML: <p>http://foo.bar/baz?a=b&amp;a=b</p>?

[Art4]

Eg the following would not be valid HTML: <p>http://foo.bar/baz?a=b&amp;a=b</p>?

Sure, this is valid HTML, but without a valid url. (with the exception of the first part http://foo.bar/baz?a=b)

The question is less if this HTML is valid but if it is trusted. So if this <p>http://foo.bar/baz?a=b&amp;a=b</p> came from as a request from a user it is not trusted and you should use $urlLinker->linkUrlsAndEscapeHtml().

But if this came from a trusted source than you can use $urlLinker->linkUrlsInTrustedHtml() but you can see that it contains not a valid url.

This is a valid url: http://foo.bar/baz?a=b&a=b
This is not a valid url: http://foo.bar/baz?a=b&a=b

[Llbe]

The encoding are layers. The & belongs to HTML as you know. It has nothing to do with the URL. This is why it still is a valid URL given that the HTML is interpreted.

I am not saying that http://foo.bar/baz?a=b&a=b is a valid URL. I am saying that it is valid HTML that contains a valid URL.

[Llbe]

The linkUrlsInTrustedHtml() should understand HTML correctly and trusting.

[Art4]

I am not saying that http://foo.bar/baz?a=b&a=b is a valid URL. I am saying that it is valid HTML that contains a valid URL.

If you insert an invalid URL the library will try to recognize as much as possible and put it in a <a>-tag. You can't expect that the library will know the meaning of an invalid url.

The linkUrlsInTrustedHtml() should understand HTML correctly and trusting.

So how should the library decide how to handle an invalid url? The meanings of http://foo.bar/baz?a=b&a=b could be:

valid url	in <a href="
http://foo.bar/baz?a=b&a=b	http://foo.bar/baz?a=b&amp;a=b
http://foo.bar/baz?a=b&amp%3ba=b	http://foo.bar/baz?a=b&amp;amp%3ba=b
http://foo.bar/baz?a=b%26amp%3ba=b	http://foo.bar/baz?a=b%26amp%3ba=b

The library will recognize an url until the first query part http://foo.bar/baz?a=b and ignore the rest. We can discuss if the library should recognize the url until http://foo.bar/baz?a=b&amp because this is also valid. But the rest ;a=b is invalid and is ignored.

I'd be glad if you would create a PR with a solution how to decide what the library should do.

[Art4]

Another example: If you call the url http://foo.bar/baz?a=b&c=d PHP will interpret the querystring as:

array(2) {
  ["a"]=>
  string(1) "b"
  ["amp;c"]=>
  string(1) "d"
}

This is because the url will be urlencoded in the browser and became http://foo.bar/baz?a=b&amp%3ba=b.

But how should the library if it should use urlencode() or turn & back to &? You provided "trusted HTML" to $urlLinker->linkUrlsInTrustedHtml() so it will not change it but search for valid urls.
But if you put this invalid url into $urlLinker->linkUrlsAndEscapeHtml() it will escape the HTML and will recognize the & as &.

So you have two options:

1. Make sure you insert valid urls
2. Use $urlLinker->linkUrlsAndEscapeHtml()

[Llbe]

I am not saying that http://foo.bar/baz?a=b&a=b is a valid URL. I am saying that it is valid HTML that contains a valid URL.
If you insert an invalid URL the library will try to recognize as much as possible and put it in a -tag. You can't expect that the library will know the meaning of an invalid url.

I'm sorry but you are missing the point. I am not inserting an invalid URL. I am inserting valid HTML. When the HTML is interpreted, it becomes a valid URL.

The fundamentally important thing is to encode/decode per context. You must not handle HTML entities as a part of a URL. Currently you are converting HTML → text without a HTML decode.

Basically linkUrlsInTrustedHtml() should HTML decode before calling linkUrlsAndEscapeHtml() - because the latter method expects text and not HTML.

For this to work the regex in linkUrlsInTrustedHtml() must include HTML entities when finding the URLs.

[Llbe]

You are mixing different contexts.

[Llbe]

Use html_entity_decode().

[Art4]

I'm sorry but I don't get your problem. Can you provide me a specific example of your problem?

If you have the input from above:

$urlLinker = new Youthweb\UrlLinker\UrlLinker();

var_dump($urlLinker->linkUrlsInTrustedHtml("http://foo.bar/baz?a=b&a=b"));

you will get

input	output from linkUrlsInTrustedHtml()
http://foo.bar/baz?a=b&amp;a=b	<a href="http://foo.bar/baz?a=b">foo.bar/baz</a>&amp;a=b
<a href="http://foo.bar/baz?a=b&amp;a=b">foo.bar</a>	<a href="http://foo.bar/baz?a=b&amp;a=b">foo.bar</a>
<p>http://foo.bar/baz?a=b&amp;a=b</p>	<p><a href="http://foo.bar/baz?a=b">foo.bar/baz</a>&amp;a=b</p>

I don't see a problem here.

Use html_entity_decode().

Why should it use html_entity_decode? You inserted trustful HTML. But you can use html_entity_decode() on your own before insert it to linkUrlsInTrustedHtml().