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

multipart/form-data validation failing due to Plug.Conn.Query.decode #441

Open
christmoore opened this issue Mar 30, 2022 · 4 comments
Open

Comments

@christmoore
Copy link

christmoore commented Mar 30, 2022

A little backstory - operations which contain a multipart/form-data request_body , which within themselves contain an array field don't get sent as arrays via SwaggerUI. curl is being generated without appending to the array field:

 operation(
.....
 request_body:
      {"Multipart form", "multipart/form-data",
       MyApp.Multipart, required: true},
....
def Multipart do
...
OpenApiSpex.schema(%{
      type: :object,
      properties: %{
        array_items: %Schema{
          description: """
          array of files
          """,
          type: :array,
          minItems: 1,
          items: %Schema{
            type: :string,
            format: :binary
          }
        }
      }
}

This generates the following Swagger UI:
image

Note the curl here. array_items needs to be array_items[] in order for Plug to create an array of Plug.Upload constructs. Without this, the conn object contains:

"array_items" => %Plug.Upload{
      content_type: "application/pdf",
      filename: "empty_pdf_2.pdf",
      path: "/var/folders/lx/8gvgf9jd24x6mcc11g747j500000gn/T//plug-1648/multipart-1648671114-838101121406784-2"
    }

This is a known issue with swagger. The solution has been to add brackets to the name of the fields so that curl interpolates currently. See https://stackoverflow.com/questions/53213067/swagger-ui-open-api-3-multipart-form-data-array-problem

Taking that curl from the example above and modifying to array_items[] creates the array as expected:

"array_items" => [
      %Plug.Upload{
        content_type: "application/pdf",
        filename: "empty_pdf_1.pdf",
        path: "/var/folders/lx/8gvgf9jd24x6mcc11g747j500000gn/T//plug-1648/multipart-1648671649-87151574074633-4"
      },
      %Plug.Upload{
        content_type: "application/pdf",
        filename: "empty_pdf_2.pdf",
        path: "/var/folders/lx/8gvgf9jd24x6mcc11g747j500000gn/T//plug-1648/multipart-1648671649-724027952677069-4"
      }
    ]

To resolve this, I've got a hacky solution that involves changing the field name to include brackets:

OpenApiSpex.schema(%{
      type: :object,
      properties: %{
        :"array_items[]": %Schema{
          description: """
          array of files
          """,
          type: :array,
          minItems: 1,
          items: %Schema{
            type: :string,
            format: :binary
          }
        }
      }
}

But downstream, validations will fail with required fields for :"array_items[]" due to the way this ends up getting interpolated. Plug.Conn.Query.decode cuts out brackets. If I end up just putting :array_items in the required field, I won't be able to submit my request in Swagger due to missing a required field (Swagger sees array_items[] not array_items)

OpenApiSpex.schema(%{
      type: :object,
      properties: %{
        :"array_items[]": %Schema{
          description: """
          array of files
          """,
          type: :array,
          minItems: 1,
          items: %Schema{
            type: :string,
            format: :binary
          }
        }
      },
    required: [:"array_items[]"]
}
..... open api request sent, request passes through MULTIPART parser, which runs through Plug.Conn.Query for decoding array_items[] to array_items list

params: %{
    "array_items" => [
      %Plug.Upload{
        content_type: "application/pdf",
        filename: "empty_pdf_1.pdf",
        path: "/var/folders/lx/8gvgf9jd24x6mcc11g747j500000gn/T//plug-1648/multipart-1648673751-336866617448758-3"
      },
      %Plug.Upload{
        content_type: "application/pdf",
        filename: "empty_pdf_2.pdf",
        path: "/var/folders/lx/8gvgf9jd24x6mcc11g747j500000gn/T//plug-1648/multipart-1648673751-11087878838975-3"
      }
    ]
  }
....
.... validation error
{
  "errors": [
    {
      "detail": "Missing field: array_items[]",
      "source": {
        "pointer": "/array_items[]"
      },
      "title": "Invalid value"
    }
  ]
}

Ideally validations follow the conventions for removing brackets on field names for multipart data. Ergo, if required: [:"array_items[]"] is set, on validation it will be checked against array_items


EDIT: It's Plug.Conn.Query.decode that cuts out brackets from field names.

iex(1)> Plug.Conn.Query.decode "a[]=1&a[]=2"
%{"a" => ["1", "2"]}
iex(2)> Plug.Conn.Query.encode %{"a" => ["1", "2"]}
"a[]=1&a[]=2"
iex(3)> Plug.Conn.Query.decode "a[1]=1&a[2]=2"
%{"a" => %{"1" => "1", "2" => "2"}}
iex(4)> Plug.Conn.Query.encode %{"a" => %{"1" => "1", "2" => "2"}}
"a[1]=1&a[2]=2"

https://hexdocs.pm/plug/Plug.Conn.Query.html#content

@christmoore christmoore changed the title Support String for Property Names in Schema multipart/form-data validation failing due to Plug.Parsers.MULTIPART Mar 30, 2022
@christmoore
Copy link
Author

christmoore commented Mar 30, 2022

I've updated to remove requirement for changing properties to support string, as the core issue here is that multipart fields that are arrays are parsed to remove brackets in field names by Plug.Conn.Query. Since this is convention, should open_api_spex be modified to accommodate for this?

@christmoore christmoore changed the title multipart/form-data validation failing due to Plug.Parsers.MULTIPART multipart/form-data validation failing due to Plug.Conn.Query.decode Mar 30, 2022
@AndriiKlymchuk
Copy link

AndriiKlymchuk commented Apr 5, 2022

@MrManicotti, as workaround I have created plug which splits specified parameters by pattern and updates both conn.body_params and conn.params:

defmodule MyAppWeb.StringBodyParamToListPlug do
  @behaviour Plug

  @impl Plug
  def init(opts), do: opts

  @impl Plug
  def call(conn, opts)do
    params =
      Enum.reduce(opts[:keys], %{}, fn key, acc ->
        case conn.body_params do
          %{^key => string_val} when is_binary(string_val) ->
            list_val = String.split(string_val, opts[:pattern])
            Map.put(acc, key, list_val)
          _ ->
            conn
        end
      end)

    conn
    |> Map.update!(:body_params, & Map.merge(&1, params))
    |> Map.update!(:params, & Map.merge(&1, params))
  end
end

Call it before OpenApiSpex.Plug.CastAndValidate

plug MyAppWeb.StringBodyParamToListPlug, [keys: ["field_name"], pattern: ","]

plug OpenApiSpex.Plug.CastAndValidate

@christmoore
Copy link
Author

christmoore commented Apr 5, 2022

I'm not quite sure this resolves the issue with validation here, as the problem specifically is in regards to what field name gets validated by CastAndValidate, rather than the actual structure of the data.

Plug.Conn.Query will cast

params: %{
    "array_items[]" => [
      %Plug.Upload{
        content_type: "application/pdf",
        filename: "empty_pdf_1.pdf",
        path: "/var/folders/lx/8gvgf9jd24x6mcc11g747j500000gn/T//plug-1648/multipart-1648673751-336866617448758-3"
      },
      %Plug.Upload{
        content_type: "application/pdf",
        filename: "empty_pdf_2.pdf",
        path: "/var/folders/lx/8gvgf9jd24x6mcc11g747j500000gn/T//plug-1648/multipart-1648673751-11087878838975-3"
      }
    ]
  }

to

params: %{
    "array_items" => [
      %Plug.Upload{
        content_type: "application/pdf",
        filename: "empty_pdf_1.pdf",
        path: "/var/folders/lx/8gvgf9jd24x6mcc11g747j500000gn/T//plug-1648/multipart-1648673751-336866617448758-3"
      },
      %Plug.Upload{
        content_type: "application/pdf",
        filename: "empty_pdf_2.pdf",
        path: "/var/folders/lx/8gvgf9jd24x6mcc11g747j500000gn/T//plug-1648/multipart-1648673751-11087878838975-3"
      }
    ]
  }

The later is correct and what's expected conventionally, however there's a problem with open-api-spex's validation. The issue is two-fold really:

  1. If I required: [:"array_items[]"], I can send my request through swagger, but open-api-spex will not see the array_items[] field due to the aforementioned decoding.
{
 "errors": [
   {
     "detail": "Missing field: array_items[]",
     "source": {
       "pointer": "/array_items[]"
     },
     "title": "Invalid value"
   }
 ]
}
  1. If I, knowing this instead required: [:"array_items"], I cannot send my request through swagger because the array_items field is not what's provided to it. It instead sees array_items[]

I believe to resolve this we may need to pass field names through the decoder prior to validation, so that array_items[] has its requirements checked against array_items on validation

@christmoore
Copy link
Author

christmoore commented Apr 11, 2022

I've got a hacky solution in the meantime to fix the spec prior to validation:

spec =
      spec
      |> update_in(
        [Access.key(:components), Access.key(:schemas), "RequestMultiPart", Access.key(:properties)],
        &with({value, map} <- Map.pop(&1, :"array_items[]"), do: Map.put(map, :array_items, value))
      )
      |> update_in(
        [Access.key(:components), Access.key(:schemas), "RequestMultiPart", Access.key(:required)],
        &[:array_items | Enum.reject(&1, fn x -> x == :"array_items[]" end)]
      )
....
case OpenApiSpex.cast_and_validate(spec, operation, conn, content_type) do
      {:ok, _} ->
        conn

      {:error, errors} ->
        errors = render_error.init(errors)
...

I had to create my own cast_and_validate plug and inject this modified spec

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