Cornices/cornice

Always HTTP 400 if marshmallow==3.0.0b13

wrkhenddher opened this issue · 3 comments

Hi,

Just ran across this:

>               "Bad response: %s (not %s)\n%s", res_status, status, res)
E           webtest.app.AppError: Bad response: 400 Bad Request (not 200)
E           b'{"status": "error", "errors": [{"location": "cookies", "name": "cookies", "description": ["Unknown field."]}, {"location": "header", "name": "header", "description": ["Unknown field."]}, {"location": "method", "name": "method", "description": ["Unknown field."]}, {"location": "url", "name": "url", "description": ["Unknown field."]}, {"location": "path", "name": "path", "description": ["Unknown field."]}, {"location": "querystring", "name": "querystring", "description": ["Unknown field."]}]}'

../../../../.pyenv/versions/3.6.3/envs/p2server_py36/lib/python3.6/site-packages/webtest/app.py:686: AppError

The reason is marshmallow 3.0.0b13 marshmallow-code/marshmallow#911

Perhaps unknown = marshmallow.EXCLUDE is now needed here:

class Meta(object):
strict = True
ordered = True

The catch is that marshmallow wants to be more strict (than before) so relaxing it seems questionable.

Up to marshmallow 3.0.0b12, it all worked well. In 3.0.0b13, the unknown=EXCLUDE fixes the issue.

You all right @wjehenddher, it makes sense, I tested your suggestioh and it worked as expected, however, I wonder why I can't simply use the unknown=EXCLUDE in the class Meta inside my own schema definition.
The way it is now, the change must be done in cornice/validators/_marshmallow.py
:|

... I wonder why I can't simply use the unknown=EXCLUDE in the class Meta inside my own schema definition.
The way it is now, the change must be done in cornice/validators/_marshmallow.py

That's indeed the bummer, @calopez. Your "custom schema" sits on top of cornice's schema that deals with body (marshmallow_body_validator). The other schemas cornice creates for headers, querystring, etc. need to be configured as well.

I.e, cornice creates multiple schemas to handle all parts of the request and delegates onto "custom schemas" depending on which marshmallow_XXX_validator one uses. That's my understanding at least. Will this qualify cornice as too strongly opinionated in this case? That's a philosophical argument.

Perhaps a configuration setting in cornice/validators/_marshmallow.py that allows implementors to override those schemas might be sufficient.

@ergo, @leplatrem?

ergo commented

This should be fixed in PR #495 .

I've reintroduced compatibility across both versions - in my opinion we should fall back to unknown=EXCLUDE again and cornice should drop the keys that are not explictly enabled.

I think we could also add an option to override meta from schema Meta - but I'm not 100% sure what potential implications that could have so I didn't add that functionality.