elixir-plug / plug

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

Plug.Conn.Query has changed the way it decodes parameters from 1.14 to 1.15 without warning #1182

Closed tmbb closed 1 year ago

tmbb commented 1 year ago

Plug used to be able to decode lists of maps the obvious way, but now it doesn't anymore. Example code:

defmodule PlugRegression do
  alias Plug.Conn.Query

  def test() do
    params = """
    a[][id]=1&\
    a[][id]=2&\
    a[][id]=3&\
    a[][id]=4&\
    """

    reordered_params = """
    a[][id]=3&\
    a[][id]=1&\
    a[][id]=2&\
    a[][id]=4&\
    """

    decoded = Query.decode(params)
    decoded_reordered = Query.decode(reordered_params)

    IO.puts("\nDecoded parameters:")
    IO.puts("─────────────────────────────")
    IO.inspect(decoded)
    IO.puts("─────────────────────────────\n")
    IO.puts("Decoded parameters after reordering:")
    IO.puts("─────────────────────────────")
    IO.inspect(decoded_reordered)
    IO.puts("─────────────────────────────\n")
  end
end

Result from running the code with Plug 1.14.0:

iex(18)> Application.spec(:plug)[:vsn]
'1.14.0'
iex(19)> PlugRegression.test

Decoded parameters:
─────────────────────────────
%{"a" => [%{"id" => "1"}, %{"id" => "2"}, %{"id" => "3"}, %{"id" => "4"}]}
─────────────────────────────

Decoded parameters after reordering:
─────────────────────────────
%{"a" => [%{"id" => "3"}, %{"id" => "1"}, %{"id" => "2"}, %{"id" => "4"}]}
─────────────────────────────

:ok

Result from running the code with Plug 1.15.0:

iex(2)> Application.spec(:plug)[:vsn]
'1.15.0'
iex(3)> PlugRegression.test()        

Decoded parameters:
─────────────────────────────
%{"a" => [%{"id" => "4"}]}
─────────────────────────────

Decoded parameters after reordering:
─────────────────────────────
%{"a" => [%{"id" => "4"}]}
─────────────────────────────

:ok

This breaks some code I'm using in practice (and even if it didn't, I don't think this behaviour should change from 1.14 to 1.15 because it's totally backward-incompatible)

josevalim commented 1 year ago

This behaviour is ambiguous and it was never officially supported. I recommend not relying on it at all. I will add a note to the CHANGELOG.