litestar-org/polyfactory

Bug: constrained string regex + min/max length may produce invalid values

Open

#124 opened on Nov 21, 2022

 (23 comments) (0 reactions) (0 assignees)Python (114 forks)auto 404
buggood first issuehelp wanted

Repository metrics

Stars
 (1,491 stars)
PR merge metrics
 (PR metrics pending)

Description

When a Field specified both max_length and regex that includes start and end of string tokens and a repeatable pattern can lead to generation of invalid string value that leads to ValidationError. See reproduction:

from pydantic import BaseModel, Field
from pydantic_factories import ModelFactory
from pydantic_factories.value_generators.regex import RegexFactory

PATTERN = r'^a+b$'
GOOD_SEED = 0
BAD_SEED = 5

class A(BaseModel):
    a: str = Field(..., regex=pattern, min_length=2, max_length=10)

class AF(ModelFactory[A]):
    __model__=A

AF.seed_random(GOOD_SEED)
print(AF.build()) # a='aaaaaaab'

print(RegexFactory(seed=BAD_SEED)(pattern)) # aaaaaaaaaab
AF.seed_random(BAD_SEED)
print(AF.build()) # this breaks

Traceback (most recent call last):
  File "[redacted]reproduce-bug.py", line 18, in <module>
    print(AF.build()) # This breaks
  File "[redacted]/factory.py", line 724, in build
    return cast("T", cls.__model__(**kwargs))  # pyright: ignore
  File "pydantic/main.py", line 342, in pydantic.main.BaseModel.__init__
pydantic.error_wrappers.ValidationError: 1 validation error for A
a
  string does not match regex "^a+b$" (type=value_error.str.regex; pattern=^a+b$)

As far as I can tell, this is a result of this piece of code cutting off the end of string after calling RegexFactory. I was surprised that the test suite didn't catch this error earlier, but to my surprise the test case that is supposed to verify this behavior will not report any issues if the string produced by handle_constrained_string doesn't match the regex. Not sure if that was done on purpose, but adding assert match is not None leads to this test failing.

I can't think of a quick solution to this issue, however I think having a note in the documentation regarding regex fields could be helpful.

I am interested in working on a more structured solution to this issue, unless it's unlikely to be merged. :)

Contributor guide