elixir-plug / plug

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

Update reason phrase for HTTP 422 to match current standard #1174

Open vanjacosic opened 11 months ago

vanjacosic commented 11 months ago

While working on a Phoenix project, I noticed that the atom :unprocessable_entity representing 422, doesn't match the reason phrase I've seen elsewhere.

Turns out that HTTP status code 422 (with the reason phrase "Unprocessable Entity") was initially proposed for WebDAV (in RFC 2518 and RFC 4918).

RFC 9110 (the HTTP Semantics standard) was published in June 2022. 422 is defined in section 15.5.21 with reason phrase "Unprocessable Content".

That is also how it is currently defined in the IANA HTTP Status Code Registry.

MDN also lists the newer phrase: 422 Unprocessable Content | MDN. So it would be great to get plug up to date with the current standard 😊

But since this will change the atom it would be a breaking change - I'll let the maintainers comment on how they wish to handle this.

PS. I also noticed a couple of other codes that seemed misaligned with the IANA registry, let me know if I should create updates to those as well πŸ˜‡

josevalim commented 11 months ago

Unfortunately this is a breaking change, so we would need to support multiple reasons, at least so :unprocessable_entity can still be used as status but otherwise returns the current specification.

whatyouhide commented 11 months ago

@josevalim why breaking change?

josevalim commented 11 months ago

Because you can do send_resp(conn, :unprocessable_entity, β€œhello”) today, no?

whatyouhide commented 11 months ago

Ouch, I didn't remember that we use the string there to build the atom status too. My bad!

Gazler commented 11 months ago

Users have the option to override their status codes by doing:

https://hexdocs.pm/plug/Plug.Conn.html#module-custom-status-codes

If we did allow this with lists I think we should still default to the current string, otherwise the result of this code changes:

send_resp(conn, 422, β€œhello”)

Would change from sending:

422 Unprocessable Entity

To:

422 Unprocessable Content

fertapric commented 10 months ago

@Gazler From my point of view, changing the reason phrase in the response shouldn't be seen as a breaking change, since clients should be compliant with the new standard. In the worst case scenario, users could resort to the custom status codes feature that you pointed out to keep the previous reason phrase.

Regarding the atom, as @josevalim mentioned, I would suggest to support multiple reasons, aliasing :unprocessable_entity to :unprocessable_content, and emit a warning to guide users towards the new standard.

Munksgaard commented 10 months ago

Thanks for the chat yesterday @vanjacosic!

After a cursory look, couldn't we add a new map of code aliases and then add a new block here?

The result would look something like this (untested, I haven't worked a lot with macros):

  aliased_statuses = %{ unprocessable_entity: :unprocessable_content }

  ...

  # This ensures backwards compatibility with changed reason phrases
  for {old_reason_phrase, new_reason_phrase} <- aliased_statuses do
    def code(unquote(old_reason_phrase)), do: code(unquote(new_reason_phrase))
  end

If you wanted to make it more efficient, you could probably do the lookup of new_reason_phrase at compile time instead.

It would mean that x == reason_atom(code(x)) would no longer hold for all x, but I don't know how important that identity is. Will it break existing code? Perhaps.

Edit: Oh! Emitting a warning is a good idea @fertapric, but I don't know what the preferred way to do that is.

josevalim commented 9 months ago

aliased_statuses = %{ unprocessable_entity: :unprocessable_content }

This ensures backwards compatibility with changed reason phrases

for {old_reason_phrase, new_reason_phrase} <- aliased_statuses do def code(unquote(old_reason_phrase)), do: code(unquote(new_reason_phrase)) end

Yes, this is exactly what we need. No need to warn, it is fine to maintain some additional aliases.