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

Swagger submitting incorrect format for Model-based form inputs #566

Open
BrendanJM opened this issue Oct 12, 2023 · 4 comments
Open

Swagger submitting incorrect format for Model-based form inputs #566

BrendanJM opened this issue Oct 12, 2023 · 4 comments

Comments

@BrendanJM
Copy link

Hi, this is a bit of an edge case I'm running into, but I hope it's solvable with some configuration. It is also possible this is a bug with swagger itself, or I'm just using things in an unintended way.

The Problem

The swagger configuration generated for my form endpoint does not submit list data in a valid way. All other usage works fine, everything passes tests, etc. It is only when trying to use swagger UI's "Try it out" does this fail.

The failure mode is that swagger submits lists of items like my_values=a,b, but should be submitting via the my_values=a&my_values=b, i.e. the way of explode=True, style='form' in OpenAPI 3.0+. As form data, my backend does not properly parse the comma-separated method.

Reproducing

Assume the following schema and endpoint:

from marshmallow import Schema, fields

# Schema
class MyFormData(Schema):
    my_values = fields.List(fields.String())

# API
blueprint = Blueprint(
    "forms", 
    "forms", 
    url_prefix="/forms"
)

@blueprint.route("/my-form")
class MyForm(MethodView):
    @blueprint.arguments(MyFormData, location="form")
    def post(self, data):
        """
        Submit form
        """
        print(data)
        pass

POST-ing to this endpoint via swagger with values and the form encoding like this:

Screenshot 2023-10-12 at 3 39 42 PM

yields a curl statement like this:

curl -X 'POST' \
  'http://localhost:8060/forms/my-form' \
  -H 'accept: application/json' \
  -H 'Content-Type: application/x-www-form-urlencoded' \
  -d 'my_values=a,b'

Which we can confirm by seeing the print output that this is not parsed into two values:

# in post handler
print(data)
> OrderedDict([('my_values', ['a,b'])])

The relevant bits of openapi.json looks something like:

  "paths": {
    "/forms/my-form": {
      "post": {
        "requestBody": {
          "required": true,
          "content": {
            "application/x-www-form-urlencoded": {
              "schema": {
                "$ref": "#/components/schemas/MyFormData"
              }
            }
          }
        },
        "tags": [
          "forms"
        ]
      }
    },
...
      "MyFormData": {
        "type": "object",
        "properties": {
          "my_values": {
            "type": "array",
            "items": {
              "type": "string"
            }
          }
        }
      },

Related Work

This appears at least tangentially related to marshmallow-code/apispec#500 and the fix marshmallow-code/apispec#778. I wasn't able to parse out if there is anything in there that I can action on though.

Thanks for taking the time to read - any thoughts on a potential workaround for fix would be appreciated.

@lafrech
Copy link
Member

lafrech commented Oct 13, 2023

I don't know who's wrong, here. I'd assume Swagger UI is right.

From the Swagger UI created curl statement, it looks like values lists in forms are sent as comma delimited lists. I'd try to use DelimitedList, then.

from webargs.fields import DelimitedList

# Schema
class MyFormData(Schema):
    my_values = DelimitedList(fields.String())

You may want to search on the Internet how values list in forms are supposed to be passed (to double-check what SwaggerUI does) and how it should be documented (to ensure flask-smorest / apispec does it right).

@BrendanJM
Copy link
Author

Thanks for the quick response! I gave DelimitedList a quick try. It seemed to break my tests, which use FlaskClient and regular dicts of data:

response = client.post(
    "/forms/my-form/",
    data={
        "my_values": [
            "a",
            "b",
        ],
    },
)

# Fails, it does not parse out items beyond the first. We get OrderedDict([('my_fields', ['a'])])
assert response['my_values'] == ['a', 'b']

Looking at the form specification from Swagger's OpenAPI guide, it seems to suggest that form data generally uses the exploded (non-comma separated) values: https://swagger.io/docs/specification/serialization/

The relevant bits here:
Screenshot 2023-10-13 at 1 12 48 PM

It seems like the actual behavior of fields.List is appropriate and consistent with this, but there seems to be some disconnect between that and what ends up generated in the openapi.json spec. Diving in a bit more to the spec, we can see this section, which feels relevant:

4.8.14.4 Support for x-www-form-urlencoded Request Bodies §

To submit content using form url encoding via [RFC1866], the following definition may be used:

requestBody:
  content:
    application/x-www-form-urlencoded:
      schema:
        type: object
        properties:
          id:
            type: string
            format: uuid
          address:
            # complex types are stringified to support RFC 1866
            type: object
            properties: {}

In this example, the contents in the requestBody MUST be stringified per [RFC1866] when passed to the server. In addition, the address field complex object will be stringified.

When passing complex objects in the application/x-www-form-urlencoded content type, the default serialization strategy of such properties is described in the Encoding Object’s style property as form.

And

4.8.12.2 Fixed Fields

Screenshot 2023-10-13 at 1 53 54 PM

Based on this, Swagger's docs are consistent with this in that form/explode should be default notation for array objects. So it seems to be that there may be something missing (or accidentally overriding) for the inner array data inside this data type as defined in the generated openapi.json spec.

That said, it's a pretty complex spec, so I have a hard time seeing how this all fits together enough to make a concrete recommendation of exactly what would need to be adjusted. I imagine it may be as "simple" as adding "explode": true, "style": "form" in the right spot. Would definitely be interested in your thoughts.

@lafrech
Copy link
Member

lafrech commented Oct 13, 2023

The DelimitedList failure is strange.

I don't know when I'll have time and motivation to look into this.

What you could do to help is try to override that specific part of the generated openapi spec with combinations of explode/style from the docs until you find one that works well with Swagger UI. Then we could try to fix apispec to render that in the spec.

@BrendanJM
Copy link
Author

Good call! That didn't take too long once I started fiddling. It looks like we're missing an encoding section under the requestBody's "application/x-www-form-urlencoded" section:

  "paths": {
    "/forms/my-form": {
      "post": {
        "requestBody": {
          "required": true,
          "content": {
            "application/x-www-form-urlencoded": {
              "schema": {
                "$ref": "#/components/schemas/MyFormData"
              },
              ####
              "encoding": {
                "my_values": {
                  "explode": true,
                  "style": "form"
                }
              }
              ####
            }
          }
        },
        "tags": [
          "forms"
        ]
      }
    },
...
      "MyFormData": {
        "type": "object",
        "properties": {
          "my_values": {
            "type": "array",
            "items": {
              "type": "string"
            }
          }
        }
      },

Reference here: https://spec.openapis.org/oas/v3.1.0#encoding-object

It saw (but didn't read too closely) there was an open issue for multi-part form support. It looks like this may potentially make headway towards that as well.

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

2 participants