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] Pydantic query validation with extra data not defined in model raise's KeyError #247

Open
sagarsabu-tait opened this issue May 24, 2024 · 5 comments · May be fixed by #259
Open

[Bug] Pydantic query validation with extra data not defined in model raise's KeyError #247

sagarsabu-tait opened this issue May 24, 2024 · 5 comments · May be fixed by #259

Comments

@sagarsabu-tait
Copy link

Describe the bug
Using a pydantic model for query parameter validation fails if extra query parameters are passed to the route. This is not a pydantic validation failure (as by default pydantic ignores extra data passed to the model initializer).

Screenshots
image

To Reproduce

from sanic import Sanic, response, request
from pydantic import BaseModel
from sanic_ext import validate


class MyQueryData(BaseModel):
    name: str


app = Sanic("Example")


@app.get('/test')
@validate(query=MyQueryData)
async def handler(request: request.Request, query: MyQueryData):
    print(f"{query=}")
    return response.empty()
# Works as expected. Return 200 OK
curl <the-url>/test?name=abcd 

# Works as expected. validation error as require params not set
curl <the-url>/test

# Unexpected failure. Return 500
curl <the-url>/test?name=abcd&another=abcd

Expected behavior

Pydantic has model based settings for handling extra data being passed to model initializer. Let it handle the extra data accordingly
https://docs.pydantic.dev/2.7/api/config/#pydantic.config.ConfigDict.extra

# Expected behviour is to return 200 OK via silently ignoring extra data
curl <the-url>/test?name=abcd&another=abcd

Environment (please complete the following information):

  • OS: Ubuntu
  • Browser: N/A
  • Version
    sanic==23.12.0
    sanic-ext==23.12.0
    sanic-routing==23.12.0
@sagarsabu-tait sagarsabu-tait changed the title Pydantic query validation with extra data not defined in model raise's KeyError [Bug] Pydantic query validation with extra data not defined in model raise's KeyError May 24, 2024
@chinu27
Copy link

chinu27 commented Jun 27, 2024

Any updates on this issue?. Urgent resolution is required.

@sagarsabu-tait
Copy link
Author

Should be a one liner fix.

# In sanic_ext/extras/validation/clean.py
def clean_data(model: Type[object], data: Dict[str, Any]) -> Dict[str, Any]:
    hints = get_type_hints(model)
    # was - return {key: _coerce(hints[key], value) for key, value in data.items()}
    return { key: _coerce(hints[key], value) for key, value in data.items() if key in hints }

@Panaetius
Copy link

I do not think that the proposed one-liner fix above fully solves the problem, as it'd just ignore all extra fields.

While this is the expected behavior for models like

class MyQueryData(BaseModel):
    model_config = ConfigDict(extra='ignore')

    name: str

it wouldn't be correct for

class MyQueryData(BaseModel):
    model_config = ConfigDict(extra='forbid')

    name: str

which should fail with a validation error for unknown properties.

@Panaetius
Copy link

I took a stab at fixing this in #259

Do note that this fix only solves these issues for models inheriting from pydantic.BaseModel, NOT when using pydantic.dataclass.

In the dataclass case, the is_pydantic() method is returns False, so regular validation (_validate_annotations()) is used, which has a separate bug.
Namely, line 165 of check.py sig.bind(**data) will fail because it gets an extra argument from the querystring that isn't in the type signature. This is not affected by pydantic configuration like @pydantid.dataclass(config=pydantic.ConfigDict(extra="ignore")) as the code in question fails before any pydantic code is run.
For me this isn't an issue, as we don't use pydantic.dataclass, but probably warrants a follow-up issue.

@shaoerkuai
Copy link

And not work for pydantic v2.

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 a pull request may close this issue.

4 participants