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

Non-iterable values get passed as [] to @pre_load @validates_schema et al. #164

Closed
tuukkamustonen opened this issue May 3, 2017 · 9 comments
Labels

Comments

@tuukkamustonen
Copy link
Contributor

With:

class Foo(StrictSchema):
    foo = fields.Str()

parser = FlaskParser()
    
req = Mock()
req.mimetype = 'application/json'
req.get_json = lambda *_, **__: {'foo': 1}  # or any non-iterable

parser.parse(Foo(strict=True, many=True), req=req, locations=('json', ))

@pre_load(pass_original=True) @validate_schema(pass_original=True) get [] as original value, instead of {'foo': 1}.

This makes it impossible to record errors about invalid base type / extra fields.

This happens for any non-iterable non-string non-mapping value.

The code that's related to this is in get_value() at https://github.com/sloria/webargs/blob/dev/webargs/core.py#L130-L132. Not yet sure what all that processing really does, but could that function return the data as is...?

@tuukkamustonen
Copy link
Contributor Author

Hmm, I think this is because of #267 (_parse_request() skipping on extra keys / invalid data).

@sloria sloria added the bug label Dec 31, 2018
@decaz
Copy link

decaz commented Jun 13, 2019

@tuukkamustonen @sloria I've found the test which I think related to the issue:

def test_parse_json_many_schema_ignore_malformed_data(self, testapp):
assert testapp.post_json("/echo_many_schema", {"extra": "data"}).json == []

The question is why invalid input type is implicitly converted into an empty list? Shouldn't the method return a response with the error "Invalid input type" ?

@lafrech
Copy link
Member

lafrech commented Jan 30, 2020

Is this fixed in webargs 6 beta?

@tuukkamustonen
Copy link
Contributor Author

Tried to check it, but so many things have changed and I don't have time to go deeper into it right now.

@lafrech
Copy link
Member

lafrech commented Jan 30, 2020

With

from marshmallow import Schema, fields
from webargs.flaskparser import FlaskParser
from unittest.mock import Mock


class Foo(Schema):
    foo = fields.Str()

parser = FlaskParser()

req = Mock()
req.mimetype = 'application/json'
req.get_data = lambda *_, **__: '{"foo": 1}'  # or any non-iterable

parser.parse(Foo(many=True), req=req, location='json')

I get

marshmallow.exceptions.ValidationError: {'_schema': ['Invalid input type.']}

werkzeug.exceptions.UnprocessableEntity: 422 Unprocessable Entity: The request was well-formed but was unable to be followed due to semantic errors.

I guess this means it is fixed.

@lafrech lafrech closed this as completed Jan 30, 2020
@tuukkamustonen
Copy link
Contributor Author

tuukkamustonen commented Jan 30, 2020

@lafrech I got the same result. But I don't think that the original issue is necessarily fixed.

The issue is about @pre_load(pass_original=True) and @validate_schema(pass_original=True) getting "manipulated" data passed in ([]), instead of the original data ({"foo": 1}).

But these decorators have changed, too, I'm not sure how they work these days, had to stop there.

@lafrech
Copy link
Member

lafrech commented Jan 30, 2020

What about

import marshmallow as ma
from marshmallow import Schema, fields
from webargs.flaskparser import FlaskParser
from unittest.mock import Mock


class Foo(Schema):

    # Both raise 
    @ma.pre_load
    def test_pre_load(self, data, **kwargs):
        print(data)
        return data

    @ma.validates_schema(pass_original=True)
    def test_validates_schema(self, data, original_data, **kwargs):
        print(data)
        print(original_data)
        return data

    foo = fields.Str()

parser = FlaskParser()

req = Mock()
req.mimetype = 'application/json'
req.get_data = lambda *_, **__: '{"foo": "1"}'  # or any non-iterable

parser.parse(Foo(), req=req, location='json')
{'foo': '1'}
{'foo': '1'}
{'foo': '1'}

@tuukkamustonen
Copy link
Contributor Author

tuukkamustonen commented Jan 31, 2020

The problem was with many=True. Added that and made @pre_load take pass_many=True:

import marshmallow as ma
from marshmallow import Schema, fields
from webargs.flaskparser import FlaskParser
from unittest.mock import Mock


class Foo(Schema):

    # Both raise
    @ma.pre_load(pass_many=True)
    def test_pre_load(self, data, **kwargs):
        print('@pre_load data:', data)
        return data

    @ma.validates_schema(pass_original=True)
    def test_validates_schema(self, data, original_data, **kwargs):
        print('@validates_schema data:', data)
        print('@validates_schema original_data:', original_data)
        return data

    foo = fields.Str()


parser = FlaskParser()

req = Mock()
req.mimetype = 'application/json'
req.get_data = lambda *_, **__: '{"foo": "1"}'  # or any non-iterable

parser.parse(Foo(many=True), req=req, location='json')

Prints:

@pre_load data: {'foo': '1'}
Traceback (most recent call last):
    ...
packages/marshmallow/schema.py", line 892, in _do_load
    raise exc
marshmallow.exceptions.ValidationError: {'_schema': ['Invalid input type.']}

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
    ...
werkzeug.exceptions.UnprocessableEntity: 422 Unprocessable Entity: The request was well-formed but was unable to be followed due to semantic errors.

So @pre_load works as expected, it now gets the original payload passed in, as should. This didn't work earlier so it's now fixed.

@validates_schema never evaluates now, because exception is raised before it. I don't know if this is how it should work by design, but I guess yes. IIRC, the problem with earlier versions was that they failed to ensure that given payload was actually an array, when many=True was passed in. Now it seems to do that, and error gets risen. I think current logic is good.

So yeah, let's assume this fixed.

@lafrech
Copy link
Member

lafrech commented Jan 31, 2020

Thanks for looking into this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants