elixir-plug / plug

Compose web applications with functions
https://hex.pm/packages/plug
Other
2.87k stars 586 forks source link

`Plug.Parsers` Incorrectly parse request body for array field #1072

Closed Skoda091 closed 2 years ago

Skoda091 commented 2 years ago

After bumping plug from 1.10.4 to 1.13, it started to parse the request body differently.

Change in parsing between plug 1.10.4 and plug 1.13

From what I see, the correct option is with an empty array based on the result of URI module.

iex(1)> URI.decode("files%5B%5D")
"files[]"

I've written a simple test to reproduce the problem.

defmodule PlugParsersTest do
  use Plug.Test

  test "Plug.Parser should parse without an empty string in array" do
    body = "files%5B%5D"

    %{params: params} =
      conn(:post, "/", body)
      |> put_req_header("content-type", "application/x-www-form-urlencoded")
      |> Plug.Parsers.call(
        Plug.Parsers.init(json_decoder: Jason, parsers: [:urlencoded, :multipart])
      )

    assert %{"files" => []} == params
  end
end
josevalim commented 2 years ago

According to the spec, foo is the same as foo= but that’s not how it worked before. So this was a bug fix applied to both Elixir and Plug.

Skoda091 commented 2 years ago

Ok, so this is expected behavior?

But when I run this on latest elixir 1.13.3-otp-24, it gives different results.

iex(1)> URI.decode("files%5B%5D")
"files[]"
iex(2)> URI.decode("foo")
"foo"
iex(3)> URI.decode("foo=")
"foo=
josevalim commented 2 years ago

decode is for URI segments. You want to compare it with decode_query instead:

iex(7)> URI.decode_query "foo="
%{"foo" => ""}
josevalim commented 2 years ago

Closing this per the above.