Fix initialization of `autocreate` and `use_ssl`
According to the documentation, both OBJECTSTORE_S3_SSL and OBJECTSTORE_S3_AUTOCREATE should default to true. Currently, when these environment variables are not set, they default to false. (Closes https://github.com/nextcloud/docker/issues/2308).
This fix works, because strtolower(false) returns the empty string. So
when OBJECTSTORE_S3_SSL is not set and getenv('OBJECTSTORE_S3_SSL')
returns false, the check strtolower($use_ssl) !== 'false' will
evaluate to true.
With this fix, both values will be true if they are
- not set
- the empty string
- any string that is not equal to
falsewhen converted to lowercase
This should now match the documented behavior.
What exactly do you mean? What "gibberish" don't you understand and which part of my proposed change is "no good"?
I did my best to be clear and improve this project. I opened an issue with a reproducible bug, then did local tests to see where the bug originates and fixed the bug so the image behaves as documented.
Maybe this testcase helps clear things up:
<?php
$foo = getenv('FOO');
$old_result = (strtolower($foo) === 'false' || $foo == false) ? false : true;
echo 'Old Result: ';
var_dump($old_result);
$new_result = strtolower($foo) !== 'false';
echo 'New Result: ';
var_dump($new_result);
# no environment variable, expected result: `true`
$ php test.php
Old Result: bool(false)
New Result: bool(true)
# environment variable set to `false`, expected result: `false`
$ FOO=fAlse php test.php
Old Result: bool(false)
New Result: bool(false)
# environment variable set to `true`, expected result: `true`
$ FOO=true php test.php
Old Result: bool(true)
New Result: bool(true)
# environment variable set but neither `true` nor `false`, expected result: `true`
$ FOO=invalid php test.php
Old Result: bool(true)
New Result: bool(true)
This shows, that the old logic does not work as documented (e.g. default to true) when the environment variable is not set at all. This conflicts with the documented behavior, which states, that the value should default to true.
Ah yes sorry for da thang, it seems it is a legit bug fix, but submit it to mail, in PR they dont merge, i dunno why but you have to send the fix via mail
How do I submit my PR via mail? Or do you mean that my commit message has to contain Signed-off-by: Author Name <[email protected]> as stated in the failed CI action?
I will update the commit message and push again soon
They dont accept pr's thru the PR, you must send them the changes via mail, understand?
Related: #1948
I just pushed again with proper Signed-off-by in my commit message.
Also sorry for feeding the troll... I fell for them, but reported the account to Github.
Thanks!