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

URLHelper::is_external not working for URLs without protocol #1924

Closed
hacknug opened this issue Feb 18, 2019 · 3 comments
Closed

URLHelper::is_external not working for URLs without protocol #1924

hacknug opened this issue Feb 18, 2019 · 3 comments

Comments

@hacknug
Copy link
Contributor

hacknug commented Feb 18, 2019

Expected behavior

URLHelper::is_external returns false for external URL's with no protocol.

  • URLHelper::is_external('//github.com/') => true
  • URLHelper::is_external('https://github.com/') => true

Actual behavior

  • URLHelper::is_external('//github.com/') => false
  • URLHelper::is_external('https://github.com/') => true

Steps to reproduce behavior

var_dump( Timber\URLHelper::is_external( '//github.com/' ) );       // false
var_dump( Timber\URLHelper::is_external( 'http://github.com/' ) );  // true
var_dump( Timber\URLHelper::is_external( 'https://github.com/' ) ); // true

What version of WordPress, PHP and Timber are you using?

WordPress 5.0.3, PHP 7.2, Timber 1.8.4.

How did you install Timber? (for example, from GitHub, Composer/Packagist, WP.org?)

Upgraded to newest version via plugin updater in WordPress dashboard.

@palmiak
Copy link
Collaborator

palmiak commented Feb 19, 2019

First of all thanks for very detailed issue. You made our work much easier.

I took a look at the code and your are right. I think we need to change checking for http to checking for array('//', 'http') at the beginning of the url.

If you would like to make a pull request it would very appreciated.

@hacknug
Copy link
Contributor Author

hacknug commented Feb 19, 2019

Sure thing! Will open a PR with the changes today, just wanted to make sure it made sense to change this.

@palmiak
Copy link
Collaborator

palmiak commented Feb 19, 2019

Great, thanks.

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

No branches or pull requests

3 participants