Skip to content
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

Permit mixed casing of string values for BooleanField validation #8970

Merged

Conversation

neckenth
Copy link
Contributor

@neckenth neckenth commented May 3, 2023

Description

In this PR, all values of type str being serialized to bool for fields typed BooleanField are first cast to lowercase and then validated against the lists of TRUE_VALUES, FALSE_VALUES, and NULL_VALUES. Discovered this restriction when a user tried to send "Yes" as a valid value for a Boolean field to my team's Public-facing API.

@auvipy auvipy self-requested a review May 4, 2023 04:01
Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need more detail here. how django handle boolean values?

@encode encode deleted a comment from phichawee May 4, 2023
@saadullahaleem
Copy link
Contributor

This is a good fix.

I initially thought that Django itself simply works with bool to cast a str but that's not the case. Here's Django's BooleanField.

class BooleanField(Field):
    widget = CheckboxInput

    def to_python(self, value):
        """Return a Python boolean object."""
        # Explicitly check for the string 'False', which is what a hidden field
        # will submit for False. Also check for '0', since this is what
        # RadioSelect will provide. Because bool("True") == bool('1') == True,
        # we don't need to handle that explicitly.
        if isinstance(value, str) and value.lower() in ("false", "0"):
            value = False
        else:
            value = bool(value)
        return super().to_python(value)

    def validate(self, value):
        if not value and self.required:
            raise ValidationError(self.error_messages["required"], code="required")

    def has_changed(self, initial, data):
        if self.disabled:
            return False
        # Sometimes data or initial may be a string equivalent of a boolean
        # so we should run it through to_python first to get a boolean value
        return self.to_python(initial) != self.to_python(data)

Django performs explicit checks too. The comments suggest that they're there to handle output of the form fields.

@auvipy auvipy requested a review from kevin-brown May 7, 2023 13:11
@auvipy auvipy removed the request for review from kevin-brown June 12, 2023 15:19
@auvipy auvipy added this to the 3.15 milestone Jun 12, 2023
@auvipy auvipy merged commit a180bde into encode:master Jun 12, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants