open-api-spex / open_api_spex

Open API Specifications for Elixir Plug applications
Mozilla Public License 2.0
706 stars 183 forks source link

Add failing test for multipart array parameters #630

Open stevehodgkiss opened 1 month ago

stevehodgkiss commented 1 month ago

Adds a failing spec for the issue described in https://github.com/open-api-spex/open_api_spex/issues/441

  1) test query params - basics handles multipart/form-data array parameters (OpenApiSpex.Plug.CastTest)
     test/plug/cast_test.exs:54
     match (=) failed
     code:  assert %{"data" => ["file1.txt", "file2.txt"]} = body
     left:  %{"data" => ["file1.txt", "file2.txt"]}
     right: %{"errors" => [%{"detail" => "Missing field: files[]", "source" => %{"pointer" => "/files[]"}, "title" => "Invalid value"}]}
     stacktrace:
       test/plug/cast_test.exs:79: (test)

Array parameters need to be supplied with [] on the end of the parameter name, because plug won't parse them into lists without it:

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

This means the key for the openapi spec needs to have [] at the end of array parameters, otherwise clients generated from the openapi.json file will be using the incorrect name in requests (i.e. files instead of files[]).

Plug's param parsing happens before the CastAndValidate plug, so the plug sees files where the user actually provided files[] in the request.

How can this be handled?

One option could be to add a parsed_param_name option to Schema, and use that to cast/validate. Users would then have to provide parsed_param_name: "files" in this scenario. Maybe there's a way to automatically derive the parsed name from the key name.

Another option is to strip [] from the end of key names in the cast/validate plug. This is what the solution provided by @christmoore is doing in https://github.com/open-api-spex/open_api_spex/issues/441#issuecomment-1095288320 (thanks for that @christmoore!). It wouldn't be possible to support replace_params: true with this approach though.