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

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

Closed
kirsle opened this issue Feb 29, 2020 · 17 comments

Comments

@kirsle
Copy link

kirsle commented Feb 29, 2020

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
Copy link
Member

sloria commented Feb 29, 2020

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
Copy link
Author

kirsle commented Feb 29, 2020

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
Copy link
Member

sloria commented Feb 29, 2020

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

@captainkw
Copy link

captainkw commented Feb 29, 2020

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
Copy link
Member

lafrech commented Feb 29, 2020

@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
Copy link
Member

lafrech commented Feb 29, 2020

This 6.0.0 change even broke your own README example

Fixed in dev branch.

@killswitch-GUI
Copy link

This bit us today :( ty for updating docs!

@itsmehemant123
Copy link

itsmehemant123 commented Mar 6, 2020

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
Copy link
Member

lafrech commented Mar 6, 2020

I guess that's what major versions are for.

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

@reinhard-mueller
Copy link

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
Copy link
Member

lafrech commented Mar 31, 2020

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

@reinhard-mueller
Copy link

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
Copy link
Member

lafrech commented Mar 31, 2020

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
Copy link
Author

kirsle commented Mar 31, 2020

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
Copy link
Member

lafrech commented Mar 31, 2020

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

@reinhard-mueller
Copy link

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
Copy link
Member

lafrech commented Mar 31, 2020

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

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

7 participants