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

Option to throw error on extra arguments #87

Closed
ParthGandhi opened this issue Feb 12, 2016 · 6 comments
Closed

Option to throw error on extra arguments #87

ParthGandhi opened this issue Feb 12, 2016 · 6 comments

Comments

@ParthGandhi
Copy link

When a request has more arguments than are specified in the parser it silently ignores them. I see a related discussion in #28.

Personally I don't think silently ignoring is the correct thing to do, but changing the default behavior would be backwards incompatible. How about parse takes a strict option that throws an error in cases like this?

@ParthGandhi
Copy link
Author

This isn't as simple as I originally thought. Theres no easy, generic, way of knowing what parts of the request body haven't been parsed yet. Any suggestions?

@sloria
Copy link
Member

sloria commented Feb 14, 2016

You might be able to do the validation for extra arguments in a pre_load method of a marshmallow Schema.

from marshmallow import Schema, pre_load

class MySchema(Schema):

    @pre_load
    def validate_extra(self, in_data):
        for key in in_data.keys():
            if key not in self.fields:
                raise ValidationError('Extra argument passed: {}'.format(key))


parser.parse(MySchema(strict=True), request)

@ParthGandhi
Copy link
Author

@sloria Don't think that works. parse sends the result from _parse_request to load. Since _parse_request specifically uses the schema to parse the request it doesn't include any extra data.

I've been able to find a workaround for my use case (using Flask) where I patch _parse_request to include any extra data from the request, and then do a validation against the defined schema. This, of course, doesn't work for other frameworks.

@sloria
Copy link
Member

sloria commented Apr 9, 2016

@ParthGandhi Indeed, there isn't really a straightforward, framework-agnostic solution for this that I can think of. I'm going to close this for now, but I'd be glad to to see your workaround so that others may benefit from it.

@sloria sloria closed this as completed Apr 9, 2016
@tuukkamustonen
Copy link
Contributor

@ParthGandhi I would be curious to see your solution as well. It seems the flexibility in supporting multiple/varying locations per argument / set of arguments makes this one problematic...

@tuukkamustonen
Copy link
Contributor

@sloria I sketched something from your pointers. It's possibly close to what @ParthGandhi has been doing:

class NoExtrasSchema(StrictSchema):

    class Meta:
        strict = True

    @pre_load
    def validate_extra(self, in_data):
        if not isinstance(in_data, dict):
            return

        extra_args = []
        for key in in_data.keys():
            if key not in self.fields:
                extra_args.append(key)

        if extra_args:
            raise ValidationError('Extra arguments passed: {}'.format(extra_args))

class StrictFlaskParser(FlaskParser):

    def _parse_request(self, schema, req, locations):
        parsed = super(StrictFlaskParser, self)._parse_request(schema, req, locations)

        # Add remaining fields
        for location in locations:
            if location == 'json':
                force = is_json_request(req)
                json_ = req.get_json(force=force, silent=True)
                if not isinstance(json_, dict):
                    continue
                source = json_
            elif location == 'querystring' or location == 'query':
                source = req.args
            elif location == 'headers':
                source = req.headers
            elif location == 'cookies':
                source = req.cookies
            elif location == 'form':
                source = req.form
            elif location == 'files':
                source = req.files

            for k, v in six.iteritems(source):
                parsed[k] = parsed.get(k, v)

        return parsed

The implementation is bound to Flask now, and I do see the problem of making it cross-framework compatible. I only tested JSON location yet, so not sure if the others really work.

Also, this "no extras" feature is probably only wanted for JSON payloads, forms, files and maybe query parameters. It doesn't really make sense for cookies and headers, imo.

In any case, I would expect support for this to exist in the library out of the box. Do you see any reasonable possibilities to maybe generalize the implementation (like adding more parse_ methods for returning the collections or missing keys)?

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

No branches or pull requests

3 participants