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

[BREAKING]: Change internal layers stack to be value-based instead of type-based #789

Merged
merged 19 commits into from
Mar 22, 2022
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion docs/src/internal_architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ HTTP.BasicAuthLayer
HTTP.CookieLayer
HTTP.CanonicalizeLayer
HTTP.MessageLayer
HTTP.AWS4AuthLayer
HTTP.RetryLayer
HTTP.ExceptionLayer
HTTP.ConnectionPoolLayer
Expand Down
197 changes: 0 additions & 197 deletions src/AWS4AuthRequest.jl

This file was deleted.

31 changes: 13 additions & 18 deletions src/BasicAuthRequest.jl
Original file line number Diff line number Diff line change
@@ -1,32 +1,27 @@
module BasicAuthRequest

using ..Base64

import ..Layer, ..request
using URIs
using ..Pairs: getkv, setkv
import ..Messages: setheader, hasheader
import ..@debug, ..DEBUG_LEVEL

export basicauthlayer
"""
request(BasicAuthLayer, method, ::URI, headers, body) -> HTTP.Response
basicauthlayer(req) -> HTTP.Response

Add `Authorization: Basic` header using credentials from url userinfo.
"""
abstract type BasicAuthLayer{Next <: Layer} <: Layer{Next} end
export BasicAuthLayer

function request(::Type{BasicAuthLayer{Next}},
method::String, url::URI, headers, body; kw...) where Next

userinfo = unescapeuri(url.userinfo)

if !isempty(userinfo) && getkv(headers, "Authorization", "") == ""
@debug 1 "Adding Authorization: Basic header."
setkv(headers, "Authorization", "Basic $(base64encode(userinfo))")
function basicauthlayer(handler)
return function(req; basicauth::Bool=true, kw...)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the stack traces look like now when everything are anonymous functions?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also would it improve stacktraces if these were named functions?

e.g.

Suggested change
return function(req; basicauth::Bool=true, kw...)
return function basicauthhandler(req; basicauth::Bool=true, kw...)

so they have names like (::var"#basicauthhandler#5") (generic function with 1 method)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the backtraces are as legible as they used to be: the use of closures means we still see a nested list of the layers, and the use of a module per layer means its pretty clear where the layer is coming from.

image

if basicauth
userinfo = unescapeuri(req.url.userinfo)
if !isempty(userinfo) && !hasheader(req.headers, "Authorization")
@debug 1 "Adding Authorization: Basic header."
setheader(req.headers, "Authorization" => "Basic $(base64encode(userinfo))")
end
end
return handler(req; kw...)
end

return request(Next, method, url, headers, body; kw...)
end


end # module BasicAuthRequest
29 changes: 14 additions & 15 deletions src/CanonicalizeRequest.jl
Original file line number Diff line number Diff line change
@@ -1,27 +1,26 @@
module CanonicalizeRequest

import ..Layer, ..request
using ..Messages
using ..Strings: tocameldash

export canonicalizelayer

"""
request(CanonicalizeLayer, method, ::URI, headers, body) -> HTTP.Response
canonicalizelayer(req) -> HTTP.Response

Rewrite request and response headers in Canonical-Camel-Dash-Format.
"""
abstract type CanonicalizeLayer{Next <: Layer} <: Layer{Next} end
export CanonicalizeLayer

function request(::Type{CanonicalizeLayer{Next}},
method::String, url, headers, body; kw...) where Next

headers = canonicalizeheaders(headers)

res = request(Next, method, url, headers, body; kw...)

res.headers = canonicalizeheaders(res.headers)

return res
function canonicalizelayer(handler)
return function(req; canonicalize_headers::Bool=false, kw...)
if canonicalize_headers
req.headers = canonicalizeheaders(req.headers)
end
res = handler(req; kw...)
if canonicalize_headers
res.headers = canonicalizeheaders(res.headers)
end
return res
end
end

canonicalizeheaders(h::T) where {T} = T([tocameldash(k) => v for (k,v) in h])
Expand Down