flask-smorest icon indicating copy to clipboard operation
flask-smorest copied to clipboard

Multipart requests

Open asyncee opened this issue 5 years ago • 38 comments

Hello!

I did not found standard way to describe a multipart request. Is it supported?

Currently i had to write something like this to let uploaded files work:

request_body_description = {
    "content": {
        "multipart/form-data": {
            "schema": {
                "type": "object",
                "properties": {"logo": {"type": "string", "format": "binary"}},
            }
        }
    }
}


@bp.route("/update-logo")
class UpdateLogo(MethodView):
    @bp.response(code=204)
    @bp.doc(requestBody=request_body_description)
    def post(self):
        file = flask.request.files["logo"]
        filename = secure_filename(file.filename)
        binary = file.read()
        
        ... do something with file ...

This code allows swagger to render input type="file" with name "logo" and send multipart request when executing request from web interface.

Am i missing something?

Thanks!

asyncee avatar Mar 19 '19 12:03 asyncee

I think this is a bug/unimplemented feature, although I have not had time to investigate it or report it properly.

Form data in OAS3 should be in the requestBody['content'] description as you have indicated, however form and file locations are currently mapped to a formData OpenApi location (which was correct for OAS2, but is not valid anymore).

I suspect it could be fixed here to also move formData locations to the requestBody. I'm not sure how that code would distinguish between multipart/form-data and application/x-www-form-urlencoded though.

toonalbers avatar Mar 27 '19 11:03 toonalbers

Thanks for the report and investigation, and sorry for the delay.

https://swagger.io/docs/specification/describing-request-body/ https://swagger.io/docs/specification/describing-request-body/file-upload/

AFAIU, apispec's mapping is correct for OAS2. When using OAS3, files and form should be sent as requestBody, not parameters, so the mapping is not used (so files/form mapping is not really wrong in the OAS3 case, it is just useless).

However, what we do in flask-rest-api is a bit wrong. In arguments, we use the mapping from apispec to change parameters location. Then later on in _prepare_doc (at this stage, the doc has not been passed to apispec yet), we check for body and change that into requestBody if using OAS3 (so apispec never sees that body string and we could use any arbitrary string).

To maintain the OAS version abstraction, we should do the mapping in _prepare_doc. This way, in _prepare_doc, we'd have access to the string entered by the user before any mapping is done, and we'd do the right thing. In fact, we don't even need to do the mapping at all. This is apispec's job. All we need to do is ensure that when using OAS3, body/files/form are declared as requestBody.

Here's a PR implementing this: https://github.com/Nobatek/flask-rest-api/pull/81.


Now, we need to associate a content type to files and form. We may have to make an arbitrary choice here, just like we currently force application/json for body as it is complicated to parametrize.

What should the mapping be?

files: multipart/form-data? (And there would be no way to specify a specific type such as image/png)

form: multipart/form-data? application/x-www-form-urlencoded?

I'm thinking maybe we could modify arguments to allow passing the content type. It could be added to the temporary doc structure and used in _prepare_doc.

Guys, what do you think?

lafrech avatar Jul 12 '19 22:07 lafrech

Hi, thanks for looking into this issue. I was actually looking at this again a few days ago.

PR #81 makes sense to me, when we later extend it for locations other than json.

Flask/webargs don't really care what content-type is set, right, so the cost for supporting more formats is not that high? I would definitely prefer being able to specify the content types. Right now I use application/x-www-form-urlencoded but there are plenty of use-cases for multipart/form-data so if it can be resolved per-case by the user I would appreciate it. Perhaps an optional content argument (same name as in OAS3) would do.

Having a default of multipart/form-data for both files and forms seems safest to me. (Consistent behavior of form data regardless of a file being present.)

toonalbers avatar Jul 15 '19 10:07 toonalbers

Ho @toonalbers. Thanks for your help on this.

Here's a draft implementation: https://github.com/Nobatek/flask-rest-api/pull/83. It uses following mapping:

  • files: multipart/form-data
  • form: application/x-www-form-urlencoded

Those are default values than can be overridden in a Blueprint subclass

Next steps:

  • [x] allow overriding content type par resource in @argument with content or content_type argument.
  • [x] correctly set "consumes" in OAS2.

Note that the content type here is only used for documentation purpose. Using location=json, content_type="application/xml" won't make the application parse xml. To actually receive xml, the user would need to subclass FlaskParser to add a parse_xml method and complete the mappings with xml -> body (perhaps this could be added to flask-rest-api core, but I'd rather not add an external dependency for it, and I don't think it is such a common case). The form/files case is a bit simpler because webargs does nothing, it just injects the relevant part of the request into the view function, leaving it the job of actually reading the input.

lafrech avatar Jul 15 '19 12:07 lafrech

Hi @lafrech. Looks good. Though, I'm wondering what a mixed form/file request (such as this) would look like with these default mappings. Would the result be something like this? I have not tried file requests myself, so maybe I'm also missing something in how the schema would be used.

"requestBody": {
  "content": {
    "application/x-www-form-urlencoded": {
      "schema": {
        "type": "object",
        "properties": {
          "orderId": { "type": "integer" },
          "userId": { "type": "integer" }
    } } },
    "multipart/form-data": {
      "schema": {
        "type": "object",
        "properties": {
          "fileName": {
            "type": "string",
            "format": "binary"
          }
    } } }
  }
}

I think the above would not be correct. So if this is the case, I would prefer just always having multipart/form-data. Or — with more effort — automatically changing to multipart/form-data if there are file parameters present. What do you think? Overriding the defaults would work around this in any case though, so thanks for that!

I agree with you on not adding more parsers yourself. If anyone should add it by default it would be webargs, like they do with JSON, but there's not much need.

toonalbers avatar Jul 15 '19 14:07 toonalbers

Yes, I overlooked multipart. In fact, my implementation assumes there is only one request body parameter (only one parameter entered as json, form or files).

IIUC, when using multipart, there can be several parameters stuffed into the request, and those are documented as properties of the multipart content schema.

I think I understand your point. You're saying that if there is both files and form, form must be part of the multipart as well.

We'd need to merge mutipart parameters into the same multipart schema, rather than having 2 distincts multipart descriptions.

We could add some logic to check if there are multiple arguments in form/files so as to enforce multipart. It might be easier to give the user the ability (and responsibility) to provide the proper content type. But even then, maybe adding that logic would be useful to provide sensible defaults.

lafrech avatar Jul 15 '19 15:07 lafrech

Ah, so it would not be as simple as I had hoped. Single file uploads and regular forms should work with #83 already, so that is nice. Merging arguments with the same location+content_type combination makes sense to me, would be a nice feature :)

toonalbers avatar Jul 15 '19 15:07 toonalbers

Yes.

Just to be sure, IIUC, we'd even need to merge arguments with location form and files if they both have the multipart content type.

lafrech avatar Jul 15 '19 15:07 lafrech

Yes, I think you are right.

toonalbers avatar Jul 16 '19 08:07 toonalbers

I pushed more commits in https://github.com/Nobatek/flask-rest-api/pull/83 to add the content_types argument to the arguments decorator. (Plural because the user may specify several types, for instance several image types.)

Feedback welcome.

This is almost good to go. Only the multipart case is not handled completely (see above). If it is too complicated to achieve, maybe I'll leave it for later, as a "known issue".

lafrech avatar Aug 13 '19 16:08 lafrech

Hello, I am willing to dive into the issue. According to https://github.com/pallets/flask/issues/2653, we need to implment our own form parser to parser the parts of form, and then put them into correct attribute of flask request, finally throw the request to webargs to parse, am I right? I put a block of codes to make webargs(https://github.com/ssfdust/flask-rest-api/tree/form_files_request_body) to parse multiple locations according to schema, it works as expected. I don't know if this break principles of flask-rest-api, and I can't figure out a better way.

ssfdust avatar Aug 21 '19 02:08 ssfdust

Thanks @ssfdust. That's interesting.

#83 is about correctly documenting resources. I'll limit its scope to that.

But of course, it's only useful if this lib doesn't get in the way when it comes to actually parsing a multipart request (not just documenting the resource). So in a further PR, we can do what is needed to allow that: do the necessary changes and either provide a helper or a piece of doc explaining how to do it in user code.

I'd need to take a deeper look at your implementation. Meanwhile, what you can do is try the branch in #83 and see what would be needed to correctly document the multipart resource. I think we'd need to merge the different parts location into a single multipart. That would happen in _prepare_doc.

lafrech avatar Aug 21 '19 08:08 lafrech

I'd like that. In fact, I'd like to achieve json data in multipart/form-data. So I create a decorator for the view function. The decorator works by injecting the request instance before parsing data. https://github.com/ssfdust/flask-jwt-app/blob/master/app/modules/storages/decorators.py But to be a part of a framework, I think table-driven approach for every content type could be better. The front end could package the json in their post data as a file, as described here. https://stackoverflow.com/questions/50774176/sending-file-and-json-in-post-multipart-form-data-request-with-axios In swagger a simple upload field, but only *.json is allowed is appreciate.

ssfdust avatar Aug 21 '19 10:08 ssfdust

Good news. I finally found some time to sort this out.

Turns out multipart could be achieved nicely with very few code in this lib, relying on webargs/apispec features.

I merged #83 already (Fix doc of form and files arguments (content type, requestBody in OAS3), allow specifying content types).

The remaining work to support and document multipart relies on features in apispec that will be released (hopefully soon) in 3.0. Demonstration in https://github.com/Nobatek/flask-rest-api/tree/document_multipart_files.

Here's what it would look like.

        class MultipartSchema(ma.Schema):
            file_1 = Upload()
            file_2 = Upload()

        @blp.route('/', methods=['POST'])
        @blp.arguments(MultipartSchema, location='files')
        def func(files):
                print(files['file_1'].read().decode())
                print(files['file_2'].read().decode())
            )

        files = {
            'file_1': (io.BytesIO('Test 1'.encode()), 'file_1.txt'),
            'file_2': (io.BytesIO('Test 2'.encode()), 'file_2.txt'),
        }

        response = client.post('/test/', data=files)

The trick (which could also be seen as a limitation) is to declare a single Schema with a 'files' location.

I still need to properly test mixed content (file + other type in multipart) but I think we're close.

As usual, feedback welcome.

lafrech avatar Sep 06 '19 21:09 lafrech

The code in the OP could read:

from flask_rest_api.fields import Upload

class MultipartSchema(ma.Schema):
    file = Upload()

@bp.route("/update-logo")
class UpdateLogo(MethodView):
    @bp.arguments(MultipartSchema, location='files')
    @bp.response(code=204)
    def post(self):
        file = files["logo"]
        filename = secure_filename(file.filename)
        binary = file.read()

lafrech avatar Sep 06 '19 21:09 lafrech

IIUC, this won't work for file + other data, because when sending a multipart request with file + data, files end up in request.files while other data is in request.form (sounds obvious but took me some time to figure out...).

To actually get the data, one needs to put it in another Schema with location=form. And then, the multipart is not automagically documented. And we're back to https://github.com/Nobatek/flask-rest-api/issues/46#issuecomment-511415780...

lafrech avatar Sep 06 '19 22:09 lafrech

@ssfdust There's a huge rework going on in webargs and the location argument in field will be removed. Schemas will be deserialized from a singe location. You'll need one Schema per location.

lafrech avatar Sep 06 '19 22:09 lafrech

Got it. Sorry for delay, I am really busy these days.

ssfdust avatar Sep 16 '19 07:09 ssfdust

I've been banging my head against this a little more. Unfortunately, auto-documenting mixed multipart request bodies is a bit complicated.

There are issues because for instance arguments allows to pass description, but this is a requestBody level attribute, not a part (as in multi-part) level attribute, therefore, what we expect from the following would be undetermined:

        @blp.arguments(FilesSchema, location='files', description='Some description')
        @blp.arguments(FormSchema, location='form', description='Some other description')

This implementation issues make me think that the sanest way to do that would be to require more explicitness from the user.

We could add a multipart_arguments decorator so that we don't have to detect multipart cases and we can ask for all needed parameters.

Things would go relatively smooth if we could pass apispec a schema that would be the aggregation of the different schemas involved ( a single schema inheriting from all). This could avoid duplicating apispec code, or having to pull functions from there to do the job here.

To be able to do so, we'd need to add a restriction to multipart_argument: only pass schema classes (or perhaps field dicts), not schema instances. Otherwise, modifiers would be lost in the process. I think we can live with this limitation.


I don't think I'll do that for the next release.

Once https://github.com/marshmallow-code/flask-smorest/tree/document_multipart_files is merged (almost done, just waiting for apispec 3.0), we'll be able to send files (using multipart), and to properly set a content type. This is already an improvement.

We shall add a note in the docs saying mixed multipart is not supported.

lafrech avatar Sep 16 '19 08:09 lafrech

@lafrech Hey, just wanted to check on the status of this last part? I am currently using a schema that has both files and data (using multipart), and while the following seems to work:

    @ns.arguments(myParameters, location='files')
    @ns.arguments(myParameters, location='form')
    def post(self, files, payload):

I wonder if there will be a way to do that in a cleaner way?

zedrdave avatar Apr 24 '20 12:04 zedrdave

@lafrech Hey, I am currently struggling with this issue in my project. I am just not able to get this to work. I need at least an id information together with the upload file. But when I add it in any location it does not work. If I add the id in the same schema as the file, the values somehow are not transmitted.

Is there any solution or workaround available yet? Could you maybe provide an example for uploading a file with additional fields, so that the file can be associated with another entity.

Thank you very much!

max-gartz avatar Nov 26 '20 10:11 max-gartz

Any progress here? i am currently looking for a way how to work with this.

aprilahijriyan avatar Apr 09 '21 08:04 aprilahijriyan

Can this issue be closed, considering #83 landed a while ago? (Great work, @lafrech : )) @aprilahijriyan I stumbled upon this issue today too and looks like the behaviour is now well documented at File Upload. Tested on my project and it works perfectly fine.

ghostwriternr avatar Jun 05 '21 21:06 ghostwriternr

Can this issue be closed, considering #83 landed a while ago?

Not really because the feature is not 100% complete.

As I wrote in https://github.com/marshmallow-code/flask-smorest/issues/46#issuecomment-531677646

mixed multipart is not supported

Yet, current implementation allows to upload files in a properly documented way so I don't expect to find the time to spend much effort on this anytime soon.

lafrech avatar Jun 21 '21 09:06 lafrech

I added following decorator. It's super ugly, but parses the form fields and generates the correct documentation:

def multipart_arguments(self,
                            schema,
                            *,
                            content_type=None,
                            required=True,
                            description=None,
                            example=None,
                            examples=None,
                            **kwargs):
        # At this stage, put schema instance in doc dictionary. Il will be
        # replaced later on by $ref or json.
        parameters = {
            'in': 'files',
            'required': required,
            'schema': schema,
        }
        if content_type is not None:
            parameters['content_type'] = content_type
        if example is not None:
            parameters['example'] = example
        if examples is not None:
            parameters['examples'] = examples
        if description is not None:
            parameters['description'] = description

        error_status_code = kwargs.get('error_status_code', self.ARGUMENTS_PARSER.DEFAULT_VALIDATION_STATUS)

        def decorator(func):

            arg_parser = self.ARGUMENTS_PARSER.use_args(schema, location='files', **kwargs)

            @wraps(func)
            def wrapper(*f_args, **f_kwargs):
                req = self.ARGUMENTS_PARSER.get_default_request()
                from werkzeug.datastructures import ImmutableMultiDict

                data = []
                orig_files = req.files
                orig_form = req.form
                for key in req.files:
                    data.append((key, req.files[key]))
                for key in req.form:
                    data.append((key, req.form[key]))
                req.form = None
                req.files = ImmutableMultiDict(data)

                try:
                    result = arg_parser(func)(*f_args, **f_kwargs)
                except Exception as e:
                    req.files = orig_files
                    req.form = orig_form
                    raise e

                req.files = orig_files
                req.form = orig_form

                return result

            # Add parameter to parameters list in doc info in function object
            # The deepcopy avoids modifying the wrapped function doc
            wrapper._apidoc = deepcopy(getattr(wrapper, '_apidoc', {}))
            docs = wrapper._apidoc.setdefault('arguments', {})
            docs.setdefault('parameters', []).append(parameters)
            docs.setdefault('responses', {})[error_status_code] = http.HTTPStatus(error_status_code).name

            # Call use_args (from webargs) to inject params in function
            return wrapper

        return decorator

claudius-kienle avatar Dec 15 '22 23:12 claudius-kienle

Hello All, I am using the suggested code by lafrech

My code as below:

from flask import request
from flask.views import MethodView
from flask_smorest import Blueprint, abort
from werkzeug.utils import secure_filename
from flask import jsonify
from PIL import Image
import logging
blp = Blueprint("Aadhar", "Aadhar Card", description="Operations on Aadhar")

logger = logging.getLogger(__name__)
ALLOWED_EXTENSIONS = set(['pdf','png', 'jpg', 'jpeg', 'gif'])
from flask_smorest.fields import Upload
import marshmallow as ma

class MultipartSchema(ma.Schema):
	file = Upload()

@blp.route("/aadhar")
class Aadhar(MethodView):
	@blp.arguments(MultipartSchema, location='files')
	@blp.response(status_code=204)
	def post(self):
		logger.debug("Aadhar st method called...")
		file = files["logo"]
		filename = secure_filename(file.filename)
		binary = file.read()

and getting an error below:

Traceback (most recent call last):
  File "/home/dinesh/pyworkspace/.VSMOREST/lib/python3.8/site-packages/flask/app.py", line 2548, in __call__
	return self.wsgi_app(environ, start_response)
  File "/home/dinesh/pyworkspace/.VSMOREST/lib/python3.8/site-packages/flask/app.py", line 2528, in wsgi_app
	response = self.handle_exception(e)
  File "/home/dinesh/pyworkspace/.VSMOREST/lib/python3.8/site-packages/flask/app.py", line 2525, in wsgi_app
	response = self.full_dispatch_request()
  File "/home/dinesh/pyworkspace/.VSMOREST/lib/python3.8/site-packages/flask/app.py", line 1822, in full_dispatch_request
	rv = self.handle_user_exception(e)
  File "/home/dinesh/pyworkspace/.VSMOREST/lib/python3.8/site-packages/flask/app.py", line 1820, in full_dispatch_request
	rv = self.dispatch_request()
  File "/home/dinesh/pyworkspace/.VSMOREST/lib/python3.8/site-packages/flask/app.py", line 1796, in dispatch_request
	return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
  File "/home/dinesh/pyworkspace/.VSMOREST/lib/python3.8/site-packages/flask/views.py", line 107, in view
	return current_app.ensure_sync(self.dispatch_request)(**kwargs)
  File "/home/dinesh/pyworkspace/.VSMOREST/lib/python3.8/site-packages/flask/views.py", line 188, in dispatch_request
	return current_app.ensure_sync(meth)(**kwargs)
  File "/home/dinesh/pyworkspace/.VSMOREST/lib/python3.8/site-packages/webargs/core.py", line 594, in wrapper
	return func(*args, **kwargs)
  File "/home/dinesh/pyworkspace/.VSMOREST/lib/python3.8/site-packages/flask_smorest/arguments.py", line 82, in wrapper
	return func(*f_args, **f_kwargs)
  File "/home/dinesh/pyworkspace/.VSMOREST/lib/python3.8/site-packages/flask_smorest/response.py", line 90, in wrapper
	func(*args, **kwargs)
TypeError: post() takes 1 positional argument but 2 were given

Please help me.

Thank you in advance.

DineshGuptaa avatar Dec 18 '22 16:12 DineshGuptaa

@DineshGuptaa

You're missing a parameter in the post() method that will contain the data of the multipart schema.

So you have to replace

def post(self):

with

def post(self, data):

claudius-kienle avatar Dec 18 '22 16:12 claudius-kienle

@DineshGuptaa

You're missing a parameter in the post() method that will contain the data of the multipart schema.

So you have to replace

def post(self):

with

def post(self, data):

Please guide me on how may I send additional payload parameters. Also those parameters should be visible to the Swagger-UI

DineshGuptaa avatar Dec 18 '22 16:12 DineshGuptaa

Hi All, I want to send some other parameters with a file. How may I achieve this task

from flask import request
from flask.views import MethodView
from flask_smorest import Blueprint, abort
from werkzeug.utils import secure_filename
import pytesseract, re
from flask import jsonify
from PIL import Image
import logging
import time
import os
blp = Blueprint("Aadhar", "Aadhar Card", description="Operations on Aadhar")

logger = logging.getLogger(__name__)
ALLOWED_EXTENSIONS = set(['pdf','png', 'jpg', 'jpeg', 'gif'])
from flask_smorest.fields import Upload
import marshmallow as ma

class MultipartSchema(ma.Schema):
	file = Upload()
	name = ma.fields.String()
	age= ma.fields.Integer()


@blp.route("/aadhar")
class Aadhar(MethodView):
	@blp.arguments(MultipartSchema, location="files")
	@blp.response(status_code=204)
	def post(self, data):
		logger.debug("Aadhar st method called...")
		file = data["file"]
		name = data["name"]
		age = data["name"]

I am getting only file param nothing else. Please suggest

Thanks in advance

DineshGuptaa avatar Dec 24 '22 06:12 DineshGuptaa

@DineshGuptaa You won't be able to get file and form data from one schema, as Multipart requests with mixed types (file, form, etc.) are not supported. . BTW, are there any updates on this issue?

relyativist avatar Mar 17 '23 15:03 relyativist