YOURLS does wrong comparisons for the configuration

[nymous]

Describe the bug
YOURLS does not take into account the configuration values for YOURLS_UNIQUE_URLS and YOURLS_PRIVATE when passed as environment variables.

To Reproduce
When running the official Docker image for YOURLS (https://hub.docker.com/_/yourls), configuration is passed as environment variables.
Setting the env variable YOURLS_UNIQUE_URLS to false results in the PHP constant YOURLS_UNIQUE_URLS to be set to the string "false", which results in an incorrect comparison in the yourls_allow_duplicate_longurls function (YOURLS_UNIQUE_URLS == false returns false, as any not-empty not-"0" string is truthy).

The same problem happens for the YOURLS_PRIVATE constant and the yourls_is_private function (and maybe for others, I haven't checked).

I have worked around the issue with the following patch (works against the tag 1.7.3):

diff --git a/includes/functions.php b/includes/functions.php
index 265f97e..78f5ec7 100644
--- a/includes/functions.php
+++ b/includes/functions.php
@@ -1284,7 +1284,7 @@ function yourls_maybe_unserialize( $original ) {
 function yourls_is_private() {
 	$private = false;

-	if ( defined('YOURLS_PRIVATE') && YOURLS_PRIVATE == true ) {
+	if ( defined('YOURLS_PRIVATE') && filter_var(YOURLS_PRIVATE, FILTER_VALIDATE_BOOLEAN) == true ) {

 		// Allow overruling for particular pages:

@@ -1331,7 +1331,7 @@ function yourls_allow_duplicate_longurls() {
 		if ( isset($_REQUEST['source']) && $_REQUEST['source'] == 'plugin' )
 			return false;
 	}
-	return ( defined( 'YOURLS_UNIQUE_URLS' ) && YOURLS_UNIQUE_URLS == false );
+	return ( defined( 'YOURLS_UNIQUE_URLS' ) && filter_var(YOURLS_UNIQUE_URLS, FILTER_VALIDATE_BOOLEAN) == false );
 }

 /**

Environment (please complete the following information):

• YOURLS version: 1.7.3 (from the official docker image)
  • Plugins enabled: YOURLS-Import-Export
• PHP version: 7.2.14

[LeoColomb]

Thanks for this report, @nymous!
What is your config.php?
Instead of editing core file, you should prefer using the config file.
The one distributed with the Docker image is written to respect environnement variables.

https://github.com/YOURLS/docker-yourls/blob/d076c5dbc3d1641e855765c3410490e710b19b6f/config-docker.php#L43-L51

[nymous]

I didn't change the config.php from the Docker image, that isn't the issue here.

Environment variables are respected, but they are retrieved as strings, which in YOURLS code are compared to a boolean. In PHP, the string "false" is truthy.

My patch forces YOURLS to convert the constant value to the corresponding boolean, whether this value is already a boolean or a string.

That being said, maybe this isn't YOURLS responsibility to handle this better, maybe the maintainers from the Docker image should patch it, as it works if the config file is edited manually to enter a boolean value...

[nymous]

Wait, did I really create this issue in the Docker repository? Woops, it was supposed to go to the YOURLS source repository, sorry! 😅

[LeoColomb]

Wait, did I really create this issue in the Docker repository?

No, I moved it, the issue does not target YOURLS itself but its usage inside the docker image.

Environment variables are respected, but they are retrieved as strings, which in YOURLS code are compared to a boolean. In PHP, the string "false" is truthy

OK, got it now.
What about fixing this on the PHP constants in the config.php directly?

[nymous]

Oh, right. Thought I lost my mind for a second ^^

Well, adding this filter in the config file seems like a better idea. Not sure if there is any other variable that would be affected by the same issue.

[LeoColomb]

Agreed. I think this is the case for all boolean variables.
Would you aim opening a pull request with the fix? 👍