Skip to content

Commit

Permalink
Use new decoder consistently
Browse files Browse the repository at this point in the history
  • Loading branch information
josevalim committed Mar 30, 2023
1 parent 712b875 commit f28d50d
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 79 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## v1.15.0-dev

### Enhancements

* Optimize query decoding by 15% to 45% - this removes the previously deprecated `:limit` MFA and `:include_unnamed_parts_at` from MULTIPART

## v1.14.2 (2023-03-23)

### Bug fixes
Expand Down
20 changes: 0 additions & 20 deletions lib/plug/conn.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1951,23 +1951,3 @@ defimpl Inspect, for: Plug.Conn do
defp no_adapter_data(conn, %{limit: :infinity}), do: conn
defp no_adapter_data(%{adapter: {adapter, _}} = conn, _), do: %{conn | adapter: {adapter, :...}}
end

defimpl Collectable, for: Plug.Conn do
def into(conn) do
IO.warn(
"using Enum.into/2 for conn is deprecated, use Plug.Conn.chunk/2 " <>
"and Enum.reduce_while/3 instead (see the Plug.Conn.chunk/2 docs for an example)"
)

fun = fn
conn, {:cont, x} ->
{:ok, conn} = Plug.Conn.chunk(conn, x)
conn

conn, _ ->
conn
end

{conn, fun}
end
end
30 changes: 23 additions & 7 deletions lib/plug/conn/query.ex
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ defmodule Plug.Conn.Query do
iex> encode(%{foo: %{bar: "baz"}})
"foo[bar]=baz"
For stateful decoding, see `decode_init/0`, `decode_each/2`, and `decode_done/2`.
"""

@doc """
Expand Down Expand Up @@ -87,8 +88,8 @@ defmodule Plug.Conn.Query do
parts = :binary.split(query, "&", [:global])

parts
|> Enum.reduce(init(), &decode_www_pair(&1, &2, invalid_exception, validate_utf8))
|> finalize(initial)
|> Enum.reduce(decode_init(), &decode_www_pair(&1, &2, invalid_exception, validate_utf8))
|> decode_done(initial)
end

defp decode_www_pair("", acc, _invalid_exception, _validate_utf8) do
Expand Down Expand Up @@ -126,13 +127,25 @@ defmodule Plug.Conn.Query do
end
end

defp init(), do: %{root: []}
@doc """
Starts a stateful decoder.
Use `decode_each/2` and `decode_done/2` to decode and complete.
"""
def decode_init(), do: %{root: []}

defp decode_each({"", value}, map) do
@doc """
Decodes the given tuple.
It parses the key and stores the value into the current
accumulator. The keys and values are not assumed to be
encoded in "x-www-form-urlencoded".
"""
def decode_each({"", value}, map) do
insert_keys([{:root, ""}], value, map)
end

defp decode_each({key, value}, map) do
def decode_each({key, value}, map) do
# Examples:
#
# users
Expand Down Expand Up @@ -187,7 +200,10 @@ defmodule Plug.Conn.Query do
map
end

defp finalize(map, initial), do: finalize_map(map.root, Enum.to_list(initial), map)
@doc """
Finishes stateful decoding and returns a map.
"""
def decode_done(map, initial \\ []), do: finalize_map(map.root, Enum.to_list(initial), map)

defp finalize_pointer(key, map) do
case Map.fetch!(map, key) do
Expand Down Expand Up @@ -230,7 +246,7 @@ defmodule Plug.Conn.Query do
Parameter lists are added to the accumulator in reverse
order, so be sure to pass the parameters in reverse order.
"""
@spec decode_pair({String.t(), term()}, acc) :: acc when acc: term()
@deprecated "Use decode_init/0, decode_each/2, and decode_done/2 instead"
def decode_pair({key, value} = _pair, acc) do
if key != "" and :binary.last(key) == ?] do
# Remove trailing ]
Expand Down
58 changes: 20 additions & 38 deletions lib/plug/parsers/multipart.ex
Original file line number Diff line number Diff line change
Expand Up @@ -39,27 +39,29 @@ defmodule Plug.Parsers.MULTIPART do
is equivalent to:
def multipart_to_params(parts, conn) do
params =
for {name, _headers, body} <- parts,
acc =
for {name, _headers, body} <- Enum.reverse(parts),
name != nil,
reduce: %{} do
acc -> Plug.Conn.Query.decode_pair({name, body}, acc)
reduce: Plug.Conn.Query.decode_init() do
acc -> Plug.Conn.Query.decode_each({name, body}, acc)
end
{:ok, params, conn}
{:ok, Plug.Conn.Query.decode_done(acc), conn}
end
As you can notice, it discards all multiparts without a name. If you want
to keep the unnamed parts, you can store all of them under a known prefix,
such as:
def multipart_to_params(parts, conn) do
params =
for {name, _headers, body} <- parts, reduce: %{} do
acc -> Plug.Conn.Query.decode_pair({name || "_parts[]", body}, acc)
acc =
for {name, _headers, body} <- Enum.reverse(parts),
name != nil,
reduce: Plug.Conn.Query.decode_init() do
acc -> Plug.Conn.Query.decode_each({name || "_parts[]", body}, acc)
end
{:ok, params, conn}
{:ok, Plug.Conn.Query.decode_done(acc), conn}
end
## Dynamic configuration
Expand Down Expand Up @@ -103,17 +105,8 @@ defmodule Plug.Parsers.MULTIPART do
# The header options are handled individually.
{headers_opts, opts} = Keyword.pop(opts, :headers, [])

with {_, _, _} <- limit do
IO.warn(
"passing a {module, function, args} tuple to Plug.Parsers.MULTIPART is deprecated. " <>
"Please see Plug.Parsers.MULTIPART module docs for better approaches to configuration"
)
end

if opts[:include_unnamed_parts_at] do
IO.warn(
":include_unnamed_parts_at for multipart is deprecated. Use :multipart_to_params instead"
)
unless is_integer(limit) do
raise ":length option for Plug.Parsers.MULTIPART must be an integer"
end

m2p = opts[:multipart_to_params] || {__MODULE__, :multipart_to_params, [opts]}
Expand Down Expand Up @@ -141,31 +134,20 @@ defmodule Plug.Parsers.MULTIPART do
end

@doc false
def multipart_to_params(acc, conn, opts) do
unnamed_at = opts[:include_unnamed_parts_at]

params =
Enum.reduce(acc, %{}, fn
{nil, headers, body}, acc when unnamed_at != nil ->
Plug.Conn.Query.decode_pair(
{unnamed_at <> "[]", %{headers: headers, body: body}},
acc
)

{nil, _headers, _body}, acc ->
acc

{name, _headers, body}, acc ->
Plug.Conn.Query.decode_pair({name, body}, acc)
def multipart_to_params(acc, conn, _opts) do
acc =
List.foldr(acc, Plug.Conn.Query.decode_init(), fn
{nil, _headers, _body}, acc -> acc
{name, _headers, body}, acc -> Plug.Conn.Query.decode_each({name, body}, acc)
end)

{:ok, params, conn}
{:ok, Plug.Conn.Query.decode_done(acc), conn}
end

## Multipart

defp parse_multipart(conn, {m2p, {module, fun, args}, header_opts, opts}) do
# TODO: This is deprecated. Remove me on Plug 2.0.
# TODO: Remove me once the deprecation is removed
limit = apply(module, fun, args)
parse_multipart(conn, {m2p, limit, header_opts, opts})
end
Expand Down
1 change: 0 additions & 1 deletion lib/plug/router/utils.ex
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ defmodule Plug.Router.Utils do
[]
"""
# TODO: Make me private in Plug v2.0
def build_path_params_match(params, context \\ nil)

def build_path_params_match([param | _] = params, context) when is_binary(param) do
Expand Down
11 changes: 0 additions & 11 deletions test/plug/conn_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ defmodule Plug.ConnTest do
alias Plug.Conn
alias Plug.ProcessStore

import ExUnit.CaptureIO

test "test adapter builds on connection" do
conn =
Plug.Adapters.Test.Conn.conn(%Plug.Conn{private: %{hello: :world}}, :post, "/hello", nil)
Expand Down Expand Up @@ -379,15 +377,6 @@ defmodule Plug.ConnTest do
assert conn.resp_body == "HELLO\nWORLD\n"
end

test "send_chunked/3 with collectable" do
conn = send_chunked(conn(:get, "/foo"), 200)

capture_io(:stderr, fn ->
conn = Enum.into(~w(hello world), conn)
assert conn.resp_body == "helloworld"
end)
end

test "send_chunked/3 sends self a message" do
refute_received {:plug_conn, :sent}
send_chunked(conn(:get, "/foo"), 200)
Expand Down
6 changes: 4 additions & 2 deletions test/plug/parsers_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,11 @@ defmodule Plug.ParsersTest do

def multipart_to_params(acc, conn) do
params =
Enum.reduce(acc, %{}, fn {name, headers, body}, acc ->
Plug.Conn.Query.decode_pair({name || "_parts[]", %{headers: headers, body: body}}, acc)
acc
|> List.foldr(Plug.Conn.Query.decode_init(), fn {name, headers, body}, acc ->
Plug.Conn.Query.decode_each({name || "_parts[]", %{headers: headers, body: body}}, acc)
end)
|> Plug.Conn.Query.decode_done()

{:ok, params, conn}
end
Expand Down

0 comments on commit f28d50d

Please sign in to comment.