Webargs 6.0.0 has broken Flaskparser @use_kwargs: Never parses query string or formdata

kirsle commented 4 years ago

Hello,

Your recent release of 6.0.0 of webargs has unexpectedly broken our Flask apps. It seems now the @use_kwargs decorator from the webargs.flaskparser completely fails to parse GET query string parameters or POST form-data parameters. Only JSON request bodies ever get parsed anymore.

Here is an example Flask app that shows the problem:

from flask import Flask, jsonify
from webargs import fields
from webargs.flaskparser import use_kwargs

app = Flask(__name__)

@app.route("/test1", methods=["GET", "POST", "PUT"])
@use_kwargs({
    "page": fields.Int(missing=1),
    "per_page": fields.Int(missing=20),
    "full": fields.Bool(missing=False),
})
def test1(**kwargs):
    return jsonify(kwargs)

@app.route("/test2", methods=["GET", "POST", "PUT"])
@use_kwargs({
    "name": fields.Str(required=True),
})
def test2(**kwargs):
    return jsonify(kwargs)

app.run()

The endpoint "/test1" has three optional parameters each with default values when missing. If I make a GET request (with query parameters) or a POST request with application/x-www-form-urlencoded format, webargs completely fails to pick up my parameters and only sees the defaults.

In the endpoint "/test2" I have a required parameter, which again fails to parse on GET or form-data post but works on JSON post only. Here are some curl examples:

# GET request doesn't parse any param
% curl 'http://localhost:5000/test1?page=5&per_page=2&full=1'
{"full":false,"page":1,"per_page":20}

# Normal POST doesn't parse any param
% curl -X POST -d page=5 -d per_page=2 -d full=1 'http://localhost:5000/test1'
{"full":false,"page":1,"per_page":20}

# JSON POST actually does parse params
% curl -X POST -H 'Content-Type: application/json' -d '{"page": 5, "per_page": 2, "full": true}' 'http://localhost:5000/test1'
{"full":true,"page":5,"per_page":2}

# GET is "missing required parameter"
% curl 'http://localhost:5000/test2?name=alice'
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">
<title>422 Unprocessable Entity</title>
<h1>Unprocessable Entity</h1>
<p>The request was well-formed but was unable to be followed due to semantic errors.</p>

# POST is "missing required parameter"
% curl -X POST -d name=alice 'http://localhost:5000/test2'
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">
<title>422 Unprocessable Entity</title>
<h1>Unprocessable Entity</h1>
<p>The request was well-formed but was unable to be followed due to semantic errors.</p>

# POST JSON actually works
% curl -X POST -H 'Content-Type: application/json' -d '{"name": "alice"}' 'http://localhost:5000/test2'
{"name":"alice"}

sloria commented 4 years ago

See the CHANGELOG https://webargs.readthedocs.io/en/latest/changelog.html#b1-2020-01-06 : use_args doesn't check multiple locations by default, and the default location is the JSON payload. To parse from the query params, you should pass location="query" to use_kwargs.

kirsle commented 4 years ago

Is there a way to restore the behavior of parsing query, form-data AND JSON together? This is an important feature for us: we use Swagger UI via flasgger, and document our APIs using form-data because Swagger provides useful text boxes and form controls, but apps talking to our API use exclusively JSON and query parameters.

sloria commented 4 years ago

You can use multiple calls to use_kwargs, each with a different location.

captainkw commented 4 years ago

For complex API endpoints used by developers, mobile apps, Unity games, and websites, this is going to require a lot of code changes and redundant, repeated usages of multiple use_kwargs.

This 6.0.0 change even broke your own README example on: https://github.com/marshmallow-code/webargs#webargs Screen_Shot_2020-02-28_at_6_38_22_PM

foo@bar:~/scratch/derp (processing-ctx)$ curl http://localhost:5000/\?name\='World'
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">
<title>422 Unprocessable Entity</title>
<h1>Unprocessable Entity</h1>
<p>The request was well-formed but was unable to be followed due to semantic errors.</p>

lafrech commented 4 years ago

@kirsle

You can do

@use_args(..., location="query"')
@use_args(..., location="json_or_form")
def view_func(...):

@captainkw, good catch. We'll fix the README example.

Indeed this change impacts code relying on a single call to use_args to pass both body and query params. Those codes will need to be adapted. I don't think it makes the use case impossible to achieve. It just takes an adaptation. Verbose in huge code base, but feasible.

Meanwhile, the app can still use webargs 5.

This changes makes webargs code clearer and more maintainable, and most importantly, it allows the Schema features (pre/post load decorators, unknown fields management) to be used, which was an issue and a source of questions for a lot of users.

lafrech commented 4 years ago

This 6.0.0 change even broke your own README example

Fixed in dev branch.

killswitch-GUI commented 4 years ago

This bit us today :( ty for updating docs!

itsmehemant123 commented 4 years ago

Same @killswitch-GUI ! I spent way too much time looking/tweaking the nginx configuration before looking at the package version. Should've done that earlier.

For what its worth: I was using the webargs.falconparser, and use_args with a single query parameter.

lafrech commented 4 years ago

I guess that's what major versions are for.

Closing this. Please comment if there are still issues with this change.

reinhard-mueller commented 4 years ago

About allowing a parameter to be present in either the query string or the form data:

You can use multiple calls to use_kwargs, each with a different location.

Unfortunately, this does not work when one of the parameters is required. No matter where the parameter is present, the other call to use_kwargs will report it as missing.

If I am not mistaken, there is no direct way to search for required parameters in multiple locations at all, something that was totally easy and straightforward in webargs 5.x

lafrech commented 4 years ago

This is an "advanced usage": https://webargs.readthedocs.io/en/latest/advanced.html#meta-locations.

reinhard-mueller commented 4 years ago

Thank you for the pointer!

I still find it odd that now "advanced usage" is necessary to achieve something which was a (fairly reasonable, IMHO) default behavior before. It would already be very helpful if there was a predefined "location" for the common use-case of query+form+json, but even better would be to allow for location=("query", "form", "json") as in the good ol' 5.x times.

lafrech commented 4 years ago

It's a trade-off. Discussion in #419 and perhaps #420.

We thought json/form might be common but query/json/form less common. I find it strange to mix query and json. Anyway, it doesn't work out-of-the-box but it can be achieved.

kirsle commented 4 years ago

I would vote to make "form_or_json" be a default location for POST requests.

I do agree that mixing query params AND form params in the same request is strange and doesn't need to be supported. But if it's a POST request, it should be able to parse form-data or JSON depending on the Content-Type of the request coming in.

i.e. when the client request says "Content-Type: application/x-www-form-urlencoded" webargs would parse as form-data; when it's "application/json", parse it as JSON data. The 5.x.x versions of webargs supported parsing data from either type of POST request and it's a bit tedious now to have to copy-paste a location="form_json" param to every single @use_kwargs when we already have 300 endpoints to update.

As the state of webargs 6 currently stands, at work we're pinned on 5.5.2 and are considering either forking webargs internally or otherwise write our own drop-in replacement; or for future new APIs we write, to pick a different library to use altogether.

lafrech commented 4 years ago

Can't you just override use_kwargs to change the default value ?

reinhard-mueller commented 4 years ago

The reason why I would like to support both query params and form params is that I have views which support being called with GET (using query params) or POST (using form params).

BTW @kirsle you could also set parser.location="json_or_form" to change the default for all use_kwargs calls that don't explicity set a location.

lafrech commented 4 years ago

BTW @kirsle you could also set parser.location="json_or_form" to change the default for all use_kwargs calls that don't explicity set a location.

Yes, forget my message above. This is already configurable in Parser.

def __init__(self, location=None, *, error_handler=None, schema_class=None):
    self.location = location or self.DEFAULT_LOCATION

marshmallow-code / webargs

Webargs 6.0.0 has broken Flaskparser @use_kwargs: Never parses query string or formdata #483