Sommerregen / grav-plugin-external-links

This plugin adds small icons to external and mailto links, informing users the link will take them to a new site or open their email client.
Other
16 stars 14 forks source link

Possible issue with malformed URLs #7

Closed paulhibbitts closed 8 years ago

paulhibbitts commented 8 years ago

After I had mistakenly formatted a URL without the 'http://' prefix (for example, Grav) in a content page my site produced the following error: crikey there was an error

With some help I was able to locate the issue to the external_links plugin, and removing/disabling it fixed the error. I realize that the URL was malformed, but this might happen more than once and the resulting error in Grav is quite a show-stopper.

I am happy to try more tests, just let me know.

Thanks, Paul

Sommerregen commented 8 years ago

Hi @paulhibbitts , yes I'm curious about this bug. Can you give me an example in quoted form (your's above seems a little broken, but do you mean getgrav.org)?

paulhibbitts commented 8 years ago

Hi Benny,

Yes, you are correct. Here is the commit that I think triggered the issue (i.e. look for the cmpt-363 malformed link): https://github.com/hibbitts-design/hibbitts-design-org/commit/3fc3d0fef174f1d0dea821969a67c3f87543ba9d

Please let me know if you would like me to test anything, etc.

Sommerregen commented 8 years ago

Ok, thanks Paul. Interestingly I don't have this issue neither with getgrav.org nor with your commit. But anyhow from your tracetstack and the message I clearly see that's a bug in the plugin pointing me to right direction. More specifically https://github.com/Sommerregen/grav-plugin-external-links/blob/master/classes/ExternalLinks.php#L182 . I guess if you add

$colonpos = false;

before that line it will resolve your issue. But in order to make the external link recognition more stable there are more changes involved, than adding just one line. I will fix it (together with the title issue you informed me about some weeks ago) and hopefully have a new release today :smile:

UPDATE: It turns out that changing L200 to

return $external;

yields the correct behaviour ;-)

paulhibbitts commented 8 years ago

Thanks for looking into the issue Benny, sounds like you've got a fix :-)