-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix initialization of autocreate and use_ssl
#2309
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
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 |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
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 <[email protected]>
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! |
Unify with micro-services image fix: nextcloud/docker#2309 Signed-off-by: Josh <[email protected]>
Unify with micro-services image fix: nextcloud/docker#2309 Signed-off-by: Josh <[email protected]>
According to the documentation, both
OBJECTSTORE_S3_SSLandOBJECTSTORE_S3_AUTOCREATEshould 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_SSLis not set andgetenv('OBJECTSTORE_S3_SSL')returns
false, the checkstrtolower($use_ssl) !== 'false'willevaluate to
true.With this fix, both values will be
trueif they arefalsewhen converted to lowercaseThis should now match the documented behavior.