litestar-org / litestar-org/polyfactory

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

Open
#124 24 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug good first issue help wanted
Dominant language
Python
Stars
1.5k
Forks
120
PR merge metrics
No merged PRs in 30d

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with pydantic_factories/constraints/strings.py around the constrained-string handling and tests/constraints/test_string_constraints.py around the existing test. Reproduce the BAD_SEED case, then inspect how RegexFactory output is combined with min_length and max_length. Done means generated values satisfy both the regex and length constraints, with a regression test asserting the match.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
64/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.