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

1.15 changes decoding behaviour of [] #1175

Closed
SteffenDE opened this issue Oct 9, 2023 · 5 comments
Closed

1.15 changes decoding behaviour of [] #1175

SteffenDE opened this issue Oct 9, 2023 · 5 comments

Comments

@SteffenDE
Copy link

Hi there,

updating to Plug 1.15, we noticed a breaking change with how parameters from a phoenix form are parsed:

%{
  "_target" => "role[permissions][][foo]",
  "role[permissions][][foo]" => "false",
  "role[permissions][][bar]" => "false",
  "role[permissions][][baz]" => "true",
}
|> URI.encode_query()
|> Plug.Conn.Query.decode()

1.14:

%{
  "_target" => "role[permissions][][foo]",
  "role" => %{"permissions" => [%{"bar" => "false"}, %{"baz" => "true"}, %{"foo" => "false"}]}
}

1.15:

%{
  "_target" => "role[permissions][][foo]",
  "role" => %{"permissions" => [%{"bar" => "false", "baz" => "true", "foo" => "false"}]}
}

Just wanted to ask whether this is an expected change or not.

@josevalim
Copy link
Member

I will try to revert it to the previous behaviour but please note this has always been unspecified, as it has always been ambiguous. :) If you can avoid relying on it, it would be beter. What is your use case?

@SteffenDE
Copy link
Author

If you can avoid relying on it, it would be beter. What is your use case?

Some legacy code that was quickly hacked together and an Ecto schema refactor leading to the changeset wanting other parameters than we use in the form.

(The form sends %{"role" => %{"permissions" => [%{"bar" => "false"}, %{"baz" => "true"}, %{"foo" => "false"}]}} - we use checkboxes for each permission - and Ecto wants %{"role" => %{"permissions" => [%{"permission" => "baz"}]}. In the database, there is one row for each permission that is active.)

I think you're right that we should not rely on it. I'll probably refactor this when I find some spare time...

@josevalim
Copy link
Member

You can also fix it by adding a continuous number:

%{
  "_target" => "role[permissions][][foo]",
  "role[permissions][0][foo]" => "false",
  "role[permissions][1][bar]" => "false",
  "role[permissions][2][baz]" => "true",
}
|> URI.encode_query()
|> Plug.Conn.Query.decode()

And then getting Map.values of the permissions map.

@josevalim
Copy link
Member

Unfortunately this is not a straight-forward change :( I have added warnings to the docs and CHANGELOG.

@SteffenDE
Copy link
Author

Just to be clear: this was more of an "is this expected" issue than a real problem. The fix in our code was straightforward, just unexpected because there was nothing in the changelog. So no worries, I'm happy that this is documented as unspecified behaviour.

Thank you!

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