Skip to content

Commit

Permalink
Fix handling of config tri-state bool values (like acl_public)
Browse files Browse the repository at this point in the history
Commit bc38c2a added the ability to set
acl_public from the config file -- but it did not take into account that
it is a 'tri-state' value so the string is what is written, causing it
to ALWAYS be true.

This results in unexpected and insecure behavior if 'acl_public = False'
is used in the config file.

This patch modifies the Config.update_option to check the option type
AND the value if the original is None. If an option's default is None
and the value is a bool (true, false, yes, no, 0, 1) then it will set it
to a bool value.
  • Loading branch information
bcl committed Dec 7, 2017
1 parent 6fad62c commit ea12ce0
Showing 1 changed file with 23 additions and 3 deletions.
26 changes: 23 additions & 3 deletions S3/Config.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,24 @@ def config_unicodise(string, encoding = "utf-8", errors = "replace"):
except UnicodeDecodeError:
raise UnicodeDecodeError("Conversion to unicode failed: %r" % string)

def is_str_true(value):
"""Check to see if a string is true, yes, on, or 1
Return True if it is
"""
return str(value).lower() in ("true", "yes", "on", "1")

def is_str_false(value):
"""Check to see if a string is false, no, off, or 0
Return True if it is
"""
return str(value).lower() in ("false", "no", "off", "0")

def is_bool(value):
"""Check a string value to see if it is bool"""
return is_str_true(value) or is_str_false(value)

class Config(object):
_instance = None
_parsed_files = []
Expand Down Expand Up @@ -347,10 +365,12 @@ def update_option(self, option, value):
raise ValueError("Config: value of option %s must have suffix m, k, or nothing, not '%s'" % (option, value))

## allow yes/no, true/false, on/off and 1/0 for boolean options
elif type(getattr(Config, option)) is type(True): # bool
if str(value).lower() in ("true", "yes", "on", "1"):
## Some options default to None, if that's the case check the value to see if it is bool
elif (type(getattr(Config, option)) is type(True) or # Config is bool
(getattr(Config, option) is None and is_bool(value))): # Config is None and value is bool
if is_str_true(value):
value = True
elif str(value).lower() in ("false", "no", "off", "0"):
elif is_str_false(value):
value = False
else:
raise ValueError("Config: value of option '%s' must be Yes or No, not '%s'" % (option, value))
Expand Down

0 comments on commit ea12ce0

Please sign in to comment.