Trailing blanks in envvars of boolean options cause otherwise valid values to be rejected

[andy-maier]

Actual Behavior

When defining a boolean option that also can be set via envvar, trailing blanks in the envvar value cause otherwise valid values to be rejected with this error message:

Error: Invalid value for '--something': true  is not a valid boolean

Because the value is shown in the error message unquoted, the trailing blank cannot easily be seen, but it is there.

Why is this an issue?

Invoking the command on Linux/macOS as follows does not trigger the issue:

$ SOMETHING=true command 

However, invoking it like this on Windows does trigger the issue:

> set SOMETHING=true & command

Apparently, Windows sets the envvar with a trailing blank in this case.

The issue can be circumvented like this:

> set SOMETHING=true& command

Here is the code that causes this behavior:

click/types.py at line 406:

class BoolParamType(ParamType):
    name = "boolean"

    def convert(self, value, param, ctx):
        if isinstance(value, bool):
            return bool(value)
        value = value.lower()
        if value in ("true", "t", "1", "yes", "y"):
            return True
        elif value in ("false", "f", "0", "no", "n"):
            return False
        self.fail("{!r} is not a valid boolean".format(value), param, ctx)

    def __repr__(self):
        return "BOOL"

Expected Behavior

I would like to see click strip any leading or trailing blanks from the boolean envvar before it is evaluated.

Environment

• Python version: any
• Click version: 7.1.2

[andy-maier]

Addressed by PR #1606.

[davidism]

I'm not sure how I feel about this. It changes the behavior of env vars only for booleans, while other values (strings, numbers, etc.) coudl still have a trailing space. I've never seen the set & command form on Windows before, is that common?

I think a more consistent solution here would be to quote the value in the message. In general I'm trying to use f"{var!r}" in f-strings for argument names and values so they look distinct in error messages.

[davidism]

On the other hand, maybe all types that would never have valid surrounding space should strip the value before processing. Could still use better quoting in the error message too.

[jab]

+1 for just making the unintended whitespace more obvious in the error message. Hopefully Windows users can quote the value when setting a variable to explicitly exclude any unintended whitespace. Likely that env var parsing libraries like environs don’t automatically strip whitespace when parsing bools (or ints, etc.) either.