Skip to content

Commit

Permalink
Big update to cookie code (#836)
Browse files Browse the repository at this point in the history
* Big update to cookie code

Our cookie code was originally ported from the golang stdlib
implementation and this represents a (slightly overdue) update
from the original source code. The biggest changes are a more
solidified cookiejar implementation that is threadsafe and includes
all the "getcookies!" and "setcookies!" implementations that we
were hand-rolling previously. I also ported over some udpated tests.

All in all, this seems like a great improvement IMO, and I'm going
to go thru open cookie-related issues and make sure we get everything
fixed.

* fix compat

* fix compat again

* fix compat finally

* Add addcookie! method for convenience for adding cookies to Request/Response
  • Loading branch information
quinnj authored May 27, 2022
1 parent 917fdb8 commit 70f174e
Show file tree
Hide file tree
Showing 7 changed files with 606 additions and 254 deletions.
52 changes: 8 additions & 44 deletions src/CookieRequest.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,10 @@ using ..Cookies
using ..Messages: Request, ascii_lc_isequal, header, setheader
import ..@debug, ..DEBUG_LEVEL, ..access_threaded

const default_cookiejar = Dict{String, Set{Cookie}}[]
# default global cookie jar
const COOKIEJAR = CookieJar()

function __init__()
resize!(empty!(default_cookiejar), Threads.nthreads())
return
end

export cookielayer
export cookielayer, COOKIEJAR

"""
cookielayer(req) -> HTTP.Response
Expand All @@ -22,23 +18,22 @@ Add locally stored Cookies to the request headers.
Store new Cookies found in the response headers.
"""
function cookielayer(handler)
return function(req::Request; cookies=true, cookiejar::Dict{String, Set{Cookie}}=access_threaded(Dict{String, Set{Cookie}}, default_cookiejar), kw...)
return function(req::Request; cookies=true, cookiejar::CookieJar=COOKIEJAR, kw...)
if cookies === true || (cookies isa AbstractDict && !isempty(cookies))
url = req.url
hostcookies = get!(cookiejar, url.host, Set{Cookie}())
cookiestosend = getcookies(hostcookies, url)
cookiestosend = Cookies.getcookies!(cookiejar, url)
if !(cookies isa Bool)
for (name, value) in cookies
push!(cookiestosend, Cookie(name, value))
end
end
if !isempty(cookiestosend)
existingcookie = header(req.headers, "Cookie")
if existingcookie != "" && get(req.context, :includedCookies, nothing) !== nothing
if existingcookie != "" && haskey(req.context, :includedCookies)
# this is a redirect where we previously included cookies
# we want to filter those out to avoid duplicate cookie sending
# and the case where a cookie was set to expire from the 1st request
previouscookies = Cookies.readcookies(req.headers, "")
previouscookies = Cookies.cookies(req)
previouslyincluded = req.context[:includedCookies]
filtered = filter(x -> !(x.name in previouslyincluded), previouscookies)
existingcookie = stringify("", filtered)
Expand All @@ -47,7 +42,7 @@ function cookielayer(handler)
req.context[:includedCookies] = map(x -> x.name, cookiestosend)
end
res = handler(req; kw...)
setcookies(hostcookies, url.host, res.headers)
Cookies.setcookies!(cookiejar, url, res.headers)
return res
else
# skip
Expand All @@ -56,35 +51,4 @@ function cookielayer(handler)
end
end

function getcookies(cookies, url)

tosend = Vector{Cookie}()
expired = Vector{Cookie}()

# Check if cookies should be added to outgoing request based on host...
for cookie in cookies
if Cookies.shouldsend(cookie, url.scheme == "https",
url.host, url.path)
t = cookie.expires
if t != Dates.DateTime(1) && t < Dates.now(Dates.UTC)
@debug 1 "Deleting expired Cookie: $cookie.name"
push!(expired, cookie)
else
@debug 1 "Sending Cookie: $cookie.name to $url.host"
push!(tosend, cookie)
end
end
end
setdiff!(cookies, expired)
return tosend
end

function setcookies(cookies, host, headers)
for (k, v) in headers
ascii_lc_isequal(k, "set-cookie") || continue
@debug 1 "Set-Cookie: $v (from $host)"
push!(cookies, Cookies.readsetcookie(host, v))
end
end

end # module CookieRequest
2 changes: 1 addition & 1 deletion src/HTTP.jl
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ Cookie options
- `cookies::Union{Bool, Dict{<:AbstractString, <:AbstractString}} = false`, enable cookies, or alternatively,
pass a `Dict{AbstractString, AbstractString}` of name-value pairs to manually pass cookies
- `cookiejar::Dict{String, Set{Cookie}}=default_cookiejar`,
- `cookiejar::HTTP.CookieJar=HTTP.COOKIEJAR`: threadsafe cookie jar struct for keeping track of cookies per host
Canonicalization options
Expand Down
6 changes: 6 additions & 0 deletions src/Messages.jl
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,12 @@ header(m::Message, k, d="") = header(m.headers, k, d)
header(h::Headers, k::AbstractString, d="") =
getbyfirst(h, k, k => d, field_name_isequal)[2]

"get all headers with key `k` or empty if none"
headers(h::Headers, k::AbstractString) =
map(x -> x[2], filter(x -> field_name_isequal(x[1], k), h))
headers(m::Message, k::AbstractString) =
headers(headers(m), k)

"""
hasheader(::Message, key) -> Bool
Expand Down
Loading

0 comments on commit 70f174e

Please sign in to comment.