Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible issue with malformed URLs #7

Closed
paulhibbitts opened this issue Nov 16, 2015 · 4 comments
Closed

Possible issue with malformed URLs #7

paulhibbitts opened this issue Nov 16, 2015 · 4 comments
Labels

Comments

@paulhibbitts
Copy link

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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 😄

UPDATE: It turns out that changing L200 to

return $external;

yields the correct behaviour ;-)

@paulhibbitts
Copy link
Author

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

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

No branches or pull requests

2 participants