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

[Bug]: forcebool inconsistency with BooleanCaster #942

Open
indigane opened this issue Nov 26, 2024 · 0 comments
Open

[Bug]: forcebool inconsistency with BooleanCaster #942

indigane opened this issue Nov 26, 2024 · 0 comments
Labels
kind/bug Indicates an issue

Comments

@indigane
Copy link

indigane commented Nov 26, 2024

Actual Behavior

I have API tests similar to

class TestForcebool:
@pytest.mark.parametrize("val", ["y", "yes", "t", "true", "on", "1", True])
def test_true(self, val):
result = forcebool(val)
assert result is True
@pytest.mark.parametrize(
"val", ["n", "no", "f", "false", "off", "0", False]
)
def test_false(self, val):
result = forcebool(val)
assert result is False
@pytest.mark.parametrize("val", ["random", "idontknow", ""])
def test_value_error(self, val):
with pytest.raises(ValueError):
forcebool(val)
that test 'True', 'true' and '1' for a boolean query parameter.

The last one, '1', no longer passes validation.

Expected Behavior

This behavior has changed at some point after 0.16.x where the test passed. This came up attempting to update past 0.16.x where the test started failing due to CastingError.

As for expected behavior, it may be that the new behavior is more correct but it is also somewhat common to allow '1' in APIs.

The older versions used forcebool as the caster/validator but the current version first calls BooleanCaster.validate before calling forcebool.

Steps to Reproduce

Pass for example '1' to BooleanCaster.__call__.

OpenAPI Core Version

0.19.2

OpenAPI Core Integration

django

Affected Area(s)

casting, validation

References

Old code:

PRIMITIVE_CASTERS: Dict[str, CasterCallable] = {
"integer": int,
"number": float,
"boolean": forcebool,
}

def forcebool(val: Any) -> bool:
if isinstance(val, str):
val = val.lower()
if val in ("y", "yes", "t", "true", "on", "1"):
return True
elif val in ("n", "no", "f", "false", "off", "0"):
return False
else:
raise ValueError(f"invalid truth value {val!r}")
return bool(val)

Current code:

oas30_casters_dict = OrderedDict(
[
("object", ObjectCaster),
("array", ArrayCaster),
("boolean", BooleanCaster),
("integer", IntegerCaster),
("number", NumberCaster),
("string", PrimitiveCaster),
]
)

class BooleanCaster(PrimitiveTypeCaster[bool]):
primitive_type = bool
def __call__(self, value: Union[str, bytes]) -> Any:
self.validate(value)
return self.primitive_type(forcebool(value))
def validate(self, value: Any) -> None:
super().validate(value)
# FIXME: don't cast data from media type deserializer
# See https://github.com/python-openapi/openapi-core/issues/706
if isinstance(value, bool):
return
if value.lower() not in ["false", "true"]:
raise ValueError("not a boolean format")

Anything else we need to know?

The main thing I want to point out is that forcebool is only ever called after validate which means that values other than 'true' and 'false' can never get to forcebool.

If this is on purpose, then I assume the way around this is to provide a custom BooleanCaster?

Or are there other suggested solutions if I do want to allow '1' in the API?

Would you like to implement a fix?

None

@indigane indigane added the kind/bug Indicates an issue label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Indicates an issue
Projects
None yet
Development

No branches or pull requests

1 participant