search

arnowelzel / lightbox-photoswipe

Integration of PhotoSwipe to WordPress
https://arnowelzel.de/wp/en/projects/wordpress/lightbox-with-photoswipe
GNU General Public License v2.0
27 stars 7 forks source link

Wrong path usage with `get_home_url` #98

Closed herrvigg closed 1 year ago

herrvigg commented 1 year ago

See code here: https://github.com/arnowelzel/lightbox-photoswipe/blob/3fde62402ef03760d385003191e26346fc0feaf2/src/LightboxPhotoSwipe/LightboxPhotoSwipe.php#L274

The second parameter passed to get_home_url should not be null, you should use '' empty string instead if you want to leave it as default. This is not compliant with the WordPress signature: https://developer.wordpress.org/reference/functions/get_home_url/.

Also, why do you call for the 'http' scheme? The default is https. Seems you should use the function with default parameters, that is get_home_url(). Please check all the calls done in a similar fashion.

Related issue detected in our qTranslate-XT plugin: https://github.com/qtranslate/qtranslate-xt/issues/1333.

arnowelzel commented 1 year ago

Thanks for pointing this out. Indeed the second parameter for get_home_url() must not be null but has to be a string. This bug got introduced with one of the latest updates to fix an issue with sites which use different URLs for the site and the frontend.

I will provide a fix for this as soon as possible.

About your question:

get_home_url() will not default to https - if https is used depends on wether the website itself is configured to use https or not (to be precise: if is_ssl() returns true, than https is used, otherwise http). Furthermore it can happen, that images got inserted to posts before a website was changed from http to https, so the image URL stil uses http:// while the website itself already changed to https://. Therefore I create two variants of the URL to be able to compare them both with the image URL.

arnowelzel commented 1 year ago

Closed with https://github.com/arnowelzel/lightbox-photoswipe/commit/9209ff1c3e2379b99fb63aeeebf2ae9e13e0dea4

herrvigg commented 1 year ago

Awesome, thanks! 👍

herrvigg commented 1 year ago

This kind of bugs is detected by stronger PHP types used in the signatures. It would be great if WordPress add such checks, but it would be hard for them to deal with all the technical debt (many plugins would break). The best we can do is start to adding those in plugins. The transition for better a PHP state in WordPress will be long.