-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Fix initialization of autocreate
and use_ssl
#2309
Conversation
8f2969e
to
3c82bad
Compare
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);
This shows, that the old logic does not work as documented (e.g. default to |
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 I will update the commit message and push again soon |
This comment was marked as off-topic.
This comment was marked as off-topic.
Related: #1948 |
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`. (See nextcloud#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 `false` when converted to lowercase This should now match the documented behavior. Signed-off-by: Valentin Brandl <mail@vbrandl.net>
3c82bad
to
053da1e
Compare
I just pushed again with proper Also sorry for feeding the troll... I fell for them, but reported the account to Github. |
Thanks! |
According to the documentation, both
OBJECTSTORE_S3_SSL
andOBJECTSTORE_S3_AUTOCREATE
should default totrue
. Currently, when these environment variables are not set, they default tofalse
. (Closes #2308).This fix works, because
strtolower(false)
returns the empty string. Sowhen
OBJECTSTORE_S3_SSL
is not set andgetenv('OBJECTSTORE_S3_SSL')
returns
false
, the checkstrtolower($use_ssl) !== 'false'
willevaluate to
true
.With this fix, both values will be
true
if they arefalse
when converted to lowercaseThis should now match the documented behavior.