Skip to content

Commit

Permalink
Deprecate cookie fields in favor of functions
Browse files Browse the repository at this point in the history
  • Loading branch information
josevalim committed Oct 31, 2024
1 parent dfa5085 commit f804daa
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 59 deletions.
39 changes: 34 additions & 5 deletions lib/plug/conn.ex
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,12 @@ defmodule Plug.Conn do
## Fetchable fields
Fetchable fields do not populate with request information until the corresponding
prefixed 'fetch_' function retrieves them, e.g., the `fetch_cookies/2` function
retrieves the `cookies` field.
prefixed 'fetch_' function retrieves them, e.g., the `fetch_query_params/2` function
retrieves the `query_params` field.
If you access these fields before fetching them, they will be returned as
`Plug.Conn.Unfetched` structs.
* `cookies`- the request cookies with the response cookies
* `body_params` - the request body params, populated through a `Plug.Parsers` parser.
* `query_params` - the request query params, populated through `fetch_query_params/2`
* `params` - the request params, the result of merging `:body_params` on top of
Expand Down Expand Up @@ -119,8 +118,14 @@ defmodule Plug.Conn do
## Deprecated fields
* `path_params` - the request path params, populated by routers such as `Plug.Router`
* `req_cookies` - the decoded request cookies (without decrypting or verifying them)
* `path_params` - the request path params, populated by routers such as `Plug.Router`.
Use `conn.params` instead.
* `req_cookies` - the decoded request cookies (without decrypting or verifying them).
Use `get_req_header/2` or `get_cookies/1` instead.
* `cookies`- the request cookies with the response cookies.
Use `get_cookies/1` instead.
* `resp_cookies`- the request cookies with the response cookies.
Use `get_resp_cookies/1` instead.
## Custom status codes
Expand Down Expand Up @@ -1547,6 +1552,30 @@ defmodule Plug.Conn do
end)
end

@doc """
Returns a map with both request and response cookies.
Raises if cookies have not been fetched.
"""
@spec get_cookies(t) :: %{optional(binary) => term}
def get_cookies(conn) do
case conn.cookies do
%Unfetched{} -> raise ArgumentError, "cookies not fetched, call fetch_cookies/2"
%{} = cookies -> cookies
end
end

@doc """
Returns a map with response cookies.
Each response cookie is represented as a map with the
metadata to be sent as part of the response.
"""
@spec get_resp_cookies(t) :: %{optional(binary) => map}
def get_resp_cookies(conn) do
conn.resp_cookies
end

@doc """
Puts a response cookie in the connection.
Expand Down
2 changes: 1 addition & 1 deletion lib/plug/session.ex
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ defmodule Plug.Session do

fn conn ->
{sid, session} =
if cookie = conn.cookies[key] do
if cookie = Plug.Conn.get_cookies(conn)[key] do
store.get(conn, cookie, store_config)
else
{nil, %{}}
Expand Down
2 changes: 1 addition & 1 deletion lib/plug/test.ex
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ defmodule Plug.Test do
req_cookies = Plug.Conn.fetch_cookies(old_conn).req_cookies

resp_cookies =
Enum.reduce(old_conn.resp_cookies, req_cookies, fn {key, opts}, acc ->
Enum.reduce(Plug.Conn.get_resp_cookies(old_conn), req_cookies, fn {key, opts}, acc ->
if value = Map.get(opts, :value) do
Map.put(acc, key, value)
else
Expand Down
82 changes: 41 additions & 41 deletions test/plug/conn_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ defmodule Plug.ConnTest do
|> register_before_send(&put_resp_cookie(&1, "hello", "world"))
|> send_resp(200, "")

assert conn.resp_cookies["hello"] == %{value: "world"}
assert get_resp_cookies(conn)["hello"] == %{value: "world"}
end

test "send_resp/1 raises if the connection was unset" do
Expand Down Expand Up @@ -981,7 +981,7 @@ defmodule Plug.ConnTest do

conn = conn(:get, "/") |> put_resp_cookie("foo", "baz", path: "/baz") |> send_resp(200, "ok")

assert conn.resp_cookies["foo"] == %{value: "baz", path: "/baz"}
assert get_resp_cookies(conn)["foo"] == %{value: "baz", path: "/baz"}
assert get_resp_header(conn, "set-cookie") == ["foo=baz; path=/baz; HttpOnly"]

conn =
Expand All @@ -990,7 +990,7 @@ defmodule Plug.ConnTest do
|> delete_resp_cookie("foo", path: "/baz")
|> send_resp(200, "ok")

assert conn.resp_cookies["foo"] ==
assert get_resp_cookies(conn)["foo"] ==
%{max_age: 0, universal_time: {{1970, 1, 1}, {0, 0, 0}}, path: "/baz"}

assert get_resp_header(conn, "set-cookie") ==
Expand Down Expand Up @@ -1019,14 +1019,14 @@ defmodule Plug.ConnTest do
|> put_resp_cookie("foo", "baz", path: "/baz")
|> send_resp(200, "ok")

assert conn.resp_cookies["foo"] == %{value: "baz", path: "/baz", secure: true}
assert get_resp_cookies(conn)["foo"] == %{value: "baz", path: "/baz", secure: true}

conn =
conn(:get, "https://example.com/")
|> put_resp_cookie("foo", "baz", path: "/baz", secure: false)
|> send_resp(200, "ok")

assert conn.resp_cookies["foo"] == %{value: "baz", path: "/baz", secure: false}
assert get_resp_cookies(conn)["foo"] == %{value: "baz", path: "/baz", secure: false}
end

test "delete_resp_cookie/3 ignores max_age and universal_time" do
Expand All @@ -1041,8 +1041,8 @@ defmodule Plug.ConnTest do
|> delete_resp_cookie("foo", cookie_opts)
|> send_resp(200, "ok")

assert conn.resp_cookies["foo"].max_age == 0
assert conn.resp_cookies["foo"].universal_time == {{1970, 1, 1}, {0, 0, 0}}
assert get_resp_cookies(conn)["foo"].max_age == 0
assert get_resp_cookies(conn)["foo"].universal_time == {{1970, 1, 1}, {0, 0, 0}}
end

test "delete_resp_cookie/3 uses connection scheme to detect secure" do
Expand All @@ -1057,9 +1057,9 @@ defmodule Plug.ConnTest do
|> delete_resp_cookie("foo", cookie_opts)
|> send_resp(200, "ok")

assert conn.resp_cookies["foo"].secure == true
assert conn.resp_cookies["foo"].max_age == 0
assert conn.resp_cookies["foo"].universal_time == {{1970, 1, 1}, {0, 0, 0}}
assert get_resp_cookies(conn)["foo"].secure == true
assert get_resp_cookies(conn)["foo"].max_age == 0
assert get_resp_cookies(conn)["foo"].universal_time == {{1970, 1, 1}, {0, 0, 0}}
end

test "put_resp_cookie/4 and delete_resp_cookie/3 raise when the connection was already sent" do
Expand Down Expand Up @@ -1098,7 +1098,7 @@ defmodule Plug.ConnTest do

conn = conn(:get, "/") |> recycle_cookies(conn) |> fetch_cookies()

assert conn.cookies == %{
assert get_cookies(conn) == %{
"req_cookie" => "req_cookie",
"over_cookie" => "pos_cookie",
"resp_cookie" => "resp_cookie"
Expand All @@ -1107,24 +1107,24 @@ defmodule Plug.ConnTest do

test "fetch_cookies/2 loaded early" do
conn = put_req_cookie(conn(:get, "/"), "foo", "bar")
assert conn.cookies == %Plug.Conn.Unfetched{aspect: :cookies}
assert_raise ArgumentError, fn -> get_cookies(conn) end

conn = fetch_cookies(conn)
assert conn.cookies["foo"] == "bar"
assert get_cookies(conn)["foo"] == "bar"

conn = put_resp_cookie(conn, "bar", "baz")
assert conn.cookies["bar"] == "baz"
assert get_cookies(conn)["bar"] == "baz"

conn = put_resp_cookie(conn, "foo", "baz")
assert conn.cookies["foo"] == "baz"
assert get_cookies(conn)["foo"] == "baz"

conn = delete_resp_cookie(conn, "foo")
refute conn.cookies["foo"]
refute get_cookies(conn)["foo"]
end

test "fetch_cookies/2 loaded late" do
conn = conn(:get, "/") |> put_req_cookie("foo", "bar") |> put_req_cookie("bar", "baz")
assert conn.cookies == %Plug.Conn.Unfetched{aspect: :cookies}
assert_raise ArgumentError, fn -> get_cookies(conn) end

conn =
conn
Expand All @@ -1133,9 +1133,9 @@ defmodule Plug.ConnTest do
|> delete_resp_cookie("bar")
|> fetch_cookies

assert conn.cookies["foo"] == "baz"
assert conn.cookies["baz"] == "bat"
refute conn.cookies["bar"]
assert get_cookies(conn)["foo"] == "baz"
assert get_cookies(conn)["baz"] == "bat"
refute get_cookies(conn)["bar"]
end

describe "signed and encrypted cookies" do
Expand All @@ -1150,10 +1150,10 @@ defmodule Plug.ConnTest do
|> put_resp_cookie("foo", {:signed, :cookie}, sign: true)
|> send_resp(200, "OK")

# The non-signed value is stored in the conn.cookies and
# the signed value in conn.resp_cookies
assert conn.cookies["foo"] == {:signed, :cookie}
value = conn.resp_cookies["foo"][:value]
# The non-signed value is stored in the get_cookies(conn) and
# the signed value in get_resp_cookies(conn)
assert get_cookies(conn)["foo"] == {:signed, :cookie}
value = get_resp_cookies(conn)["foo"][:value]
assert value =~ ~r"[\w\_\.]+"

[set_cookie] = get_resp_header(conn, "set-cookie")
Expand All @@ -1162,13 +1162,13 @@ defmodule Plug.ConnTest do
# Recycling can handle signed cookies and they can be verified on demand
new_conn = secret_conn() |> recycle_cookies(conn) |> fetch_cookies()
assert new_conn.req_cookies["foo"] == value
assert new_conn.cookies["foo"] == value
assert get_cookies(new_conn)["foo"] == value
assert fetch_cookies(new_conn, signed: "foo").cookies["foo"] == {:signed, :cookie}

# Recycling can handle signed cookies and they can be verified upfront
new_conn = secret_conn() |> recycle_cookies(conn) |> fetch_cookies(signed: ~w(foo))
assert new_conn.req_cookies["foo"] == value
assert new_conn.cookies["foo"] == {:signed, :cookie}
assert get_cookies(new_conn)["foo"] == {:signed, :cookie}
end

test "put_resp_cookie/4 with encrypt: true" do
Expand All @@ -1178,10 +1178,10 @@ defmodule Plug.ConnTest do
|> put_resp_cookie("foo", {:encrypted, :cookie}, encrypt: true)
|> send_resp(200, "OK")

# The decrypted value is stored in the conn.cookies and
# the encrypted value in conn.resp_cookies
assert conn.cookies["foo"] == {:encrypted, :cookie}
value = conn.resp_cookies["foo"][:value]
# The decrypted value is stored in the get_cookies(conn) and
# the encrypted value in get_resp_cookies(conn)
assert get_cookies(conn)["foo"] == {:encrypted, :cookie}
value = get_resp_cookies(conn)["foo"][:value]
assert value =~ ~r"[\w\_\.]+"

[set_cookie] = get_resp_header(conn, "set-cookie")
Expand All @@ -1190,13 +1190,13 @@ defmodule Plug.ConnTest do
# Recycling can handle encrypted cookies and they can be decrypted on demand
new_conn = secret_conn() |> recycle_cookies(conn) |> fetch_cookies()
assert new_conn.req_cookies["foo"] == value
assert new_conn.cookies["foo"] == value
assert get_cookies(new_conn)["foo"] == value
assert fetch_cookies(new_conn, encrypted: "foo").cookies["foo"] == {:encrypted, :cookie}

# Recycling can handle encrypted cookies and they can be decrypted upfront
new_conn = secret_conn() |> recycle_cookies(conn) |> fetch_cookies(encrypted: ~w(foo))
assert new_conn.req_cookies["foo"] == value
assert new_conn.cookies["foo"] == {:encrypted, :cookie}
assert get_cookies(new_conn)["foo"] == {:encrypted, :cookie}
end

test "put_resp_cookie/4 with sign: true and max_age: 0" do
Expand All @@ -1206,12 +1206,12 @@ defmodule Plug.ConnTest do
|> put_resp_cookie("foo", {:signed, :cookie}, sign: true, max_age: 0)
|> send_resp(200, "OK")

assert conn.cookies["foo"] == {:signed, :cookie}
assert conn.resp_cookies["foo"][:value] =~ ~r"[\w\_\.]+"
assert get_cookies(conn)["foo"] == {:signed, :cookie}
assert get_resp_cookies(conn)["foo"][:value] =~ ~r"[\w\_\.]+"

new_conn = secret_conn() |> recycle_cookies(conn) |> fetch_cookies()
assert new_conn.req_cookies["foo"] == conn.resp_cookies["foo"][:value]
assert new_conn.cookies["foo"] == conn.resp_cookies["foo"][:value]
assert new_conn.req_cookies["foo"] == get_resp_cookies(conn)["foo"][:value]
assert get_cookies(new_conn)["foo"] == get_resp_cookies(conn)["foo"][:value]
refute Map.has_key?(fetch_cookies(new_conn, signed: "foo").cookies, "foo")
end

Expand All @@ -1223,7 +1223,7 @@ defmodule Plug.ConnTest do
|> send_resp(200, "OK")

new_conn = secret_conn() |> recycle_cookies(conn) |> fetch_cookies(signed: ~w(foo))
assert Map.get(new_conn.cookies, "foo") == {:signed, :cookie}
assert Map.get(get_cookies(new_conn), "foo") == {:signed, :cookie}
end

test "put_resp_cookie/4 with encrypt: true and max_age: 0" do
Expand All @@ -1233,12 +1233,12 @@ defmodule Plug.ConnTest do
|> put_resp_cookie("foo", {:encrypted, :cookie}, encrypt: true, max_age: 0)
|> send_resp(200, "OK")

assert conn.cookies["foo"] == {:encrypted, :cookie}
assert conn.resp_cookies["foo"][:value] =~ ~r"[\w\_\.]+"
assert get_cookies(conn)["foo"] == {:encrypted, :cookie}
assert get_resp_cookies(conn)["foo"][:value] =~ ~r"[\w\_\.]+"

new_conn = secret_conn() |> recycle_cookies(conn) |> fetch_cookies()
assert new_conn.req_cookies["foo"] == conn.resp_cookies["foo"][:value]
assert new_conn.cookies["foo"] == conn.resp_cookies["foo"][:value]
assert new_conn.req_cookies["foo"] == get_resp_cookies(conn)["foo"][:value]
assert get_cookies(new_conn)["foo"] == get_resp_cookies(conn)["foo"][:value]
refute Map.has_key?(fetch_cookies(new_conn, encrypted: "foo").cookies, "foo")
end
end
Expand Down
22 changes: 11 additions & 11 deletions test/plug/session_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ defmodule Plug.SessionTest do
opts = Plug.Session.init(store: ProcessStore, key: "foobar")
conn = conn |> Plug.Session.call(opts) |> fetch_session()
conn = send_resp(conn, 200, "")
assert conn.resp_cookies == %{}
assert get_resp_cookies(conn) == %{}

conn = fetch_cookies(conn(:get, "/"))

Expand All @@ -28,9 +28,9 @@ defmodule Plug.SessionTest do
conn = send_resp(conn, 200, "")

assert %{"foobar" => %{value: _, secure: true, path: "some/path", extra: "extra"}} =
conn.resp_cookies
get_resp_cookies(conn)

refute Map.has_key?(conn.resp_cookies["foobar"], :http_only)
refute Map.has_key?(get_resp_cookies(conn)["foobar"], :http_only)

conn = fetch_cookies(conn(:get, "/"))

Expand All @@ -47,7 +47,7 @@ defmodule Plug.SessionTest do
conn = send_resp(conn, 200, "")

assert %{"unsafe_foobar" => %{value: _, http_only: false, path: "some/path"}} =
conn.resp_cookies
get_resp_cookies(conn)
end

test "put session" do
Expand Down Expand Up @@ -83,7 +83,7 @@ defmodule Plug.SessionTest do
conn = configure_session(conn, drop: true)
conn = send_resp(conn, 200, "")

assert conn.resp_cookies ==
assert get_resp_cookies(conn) ==
%{"foobar" => %{max_age: 0, universal_time: {{1970, 1, 1}, {0, 0, 0}}}}
end

Expand All @@ -94,7 +94,7 @@ defmodule Plug.SessionTest do
conn = put_session(conn, "foo", "bar")
conn = configure_session(conn, drop: true)
conn = send_resp(conn, 200, "")
assert conn.resp_cookies == %{}
assert get_resp_cookies(conn) == %{}
end

test "renew session" do
Expand All @@ -107,8 +107,8 @@ defmodule Plug.SessionTest do
conn = configure_session(conn, renew: true)
conn = send_resp(conn, 200, "")

assert match?(%{"foobar" => %{value: _}}, conn.resp_cookies)
refute match?(%{"foobar" => %{value: "sid"}}, conn.resp_cookies)
assert match?(%{"foobar" => %{value: _}}, get_resp_cookies(conn))
refute match?(%{"foobar" => %{value: "sid"}}, get_resp_cookies(conn))
end

test "ignore changes to session" do
Expand All @@ -119,7 +119,7 @@ defmodule Plug.SessionTest do
conn = put_session(conn, "foo", "bar")
conn = send_resp(conn, 200, "")

assert conn.resp_cookies == %{}
assert get_resp_cookies(conn) == %{}
end

test "reuses sid and as such does not generate new cookie" do
Expand All @@ -131,7 +131,7 @@ defmodule Plug.SessionTest do
conn = conn |> Plug.Session.call(opts) |> fetch_session()
conn = send_resp(conn, 200, "")

assert conn.resp_cookies == %{}
assert get_resp_cookies(conn) == %{}
end

test "generates sid" do
Expand All @@ -142,7 +142,7 @@ defmodule Plug.SessionTest do
conn = put_session(conn, "foo", "bar")
conn = send_resp(conn, 200, "")

assert %{value: sid} = conn.resp_cookies["foobar"]
assert %{value: sid} = get_resp_cookies(conn)["foobar"]
assert Process.get({:session, sid}) == %{"foo" => "bar"}
end

Expand Down

0 comments on commit f804daa

Please sign in to comment.