Another refactor to move layers to a more functional architecture and

drastically reduce the interface requirements

src/ContentTypeRequest.jl

@@ -5,19 +5,18 @@ using ..Pairs: getkv, setkv
 import ..sniff
 import ..Form
 using ..Messages
-import ..MessageRequest: bodylength, bodybytes
+import ..IOExtras
 import ..@debug, ..DEBUG_LEVEL
 
 export contenttypedetectionlayer
-
+# f(::Handler) -> Handler
 function contenttypedetectionlayer(handler)
     return function(ctx, method, url, headers, body; detect_content_type::Bool=false, kw...)
         if detect_content_type && (getkv(headers, "Content-Type", "") == ""
             &&  !isa(body, Form)
-            &&  bodylength(body) != unknown_length
-            &&  bodylength(body) > 0)
+            &&  isbytes(body))
 
-            sn = sniff(bodybytes(body))
+            sn = sniff(bytes(body))
             setkv(headers, "Content-Type", sn)
             @debug 1 "setting Content-Type header to: $sn"
         end

src/CookieRequest.jl

@@ -24,7 +24,6 @@ Store new Cookies found in the response headers.
 """
 function cookielayer(handler)
     return function(ctx, req::Request; cookies=true, cookiejar::Dict{String, Set{Cookie}}=access_threaded(Dict{String, Set{Cookie}}, default_cookiejar), kw...)
-        println("cookielayer")
         if cookies === true || (cookies isa AbstractDict && !isempty(cookies))
             url = req.url
             hostcookies = get!(cookiejar, url.host, Set{Cookie}())
@@ -35,12 +34,21 @@ function cookielayer(handler)
                 end
             end
             if !isempty(cookiestosend)
-                setkv(req.headers, "Cookie", stringify(getkv(req.headers, "Cookie", ""), cookiestosend))
+                existingcookie = getkv(req.headers, "Cookie", "")
+                if existingcookie != "" && get(ctx, :includedCookies, nothing) !== nothing
+                    # 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, "")
+                    previouslyincluded = ctx[:includedCookies]
+                    filtered = filter(x -> !(x.name in previouslyincluded), previouscookies)
+                    existingcookie = stringify("", filtered)
+                end
+                setkv(req.headers, "Cookie", stringify(existingcookie, cookiestosend))
+                ctx[:includedCookies] = map(x -> x.name, cookiestosend)
             end
-            @show cookiestosend
             res = handler(ctx, req; kw...)
             setcookies(hostcookies, url.host, res.headers)
-            @show hostcookies
             return res
         else
             # skip

src/IOExtras.jl

@@ -10,27 +10,37 @@ module IOExtras
 using ..Sockets
 using MbedTLS: MbedException
 
-export bytes, isbytes, ByteView, nobytes, CodeUnits, IOError, isioerror,
+export bytes, isbytes, nbytes, ByteView, nobytes, IOError, isioerror,
        startwrite, closewrite, startread, closeread,
        tcpsocket, localport, safe_getpeername
 
-
 """
-    bytes(s::String)
+    bytes(x)
 
-Get a `Vector{UInt8}`, a vector of bytes of a string.
+If `x` is "castable" to an `AbstractVector{UInt8}`, then an
+`AbstractVector{UInt8}` is returned; otherwise `x` is returned.
 """
 function bytes end
-bytes(s::SubArray{UInt8}) = unsafe_wrap(Array, pointer(s), length(s))
+bytes(s::AbstractVector{UInt8}) = s
+bytes(s::AbstractString) = codeunits(s)
+bytes(x) = x
 
-const CodeUnits = Union{Vector{UInt8}, Base.CodeUnits}
-bytes(s::Base.CodeUnits) = bytes(String(s))
-bytes(s::String) = codeunits(s)
-bytes(s::SubString{String}) = codeunits(s)
+"""whether `x` is "castable" to an `AbstractVector{UInt8}`; i.e. you can call `bytes(x)` if `isbytes(x)` === true"""
+isbytes(x) = x isa AbstractVector{UInt8} || x isa AbstractString
 
-bytes(s::Vector{UInt8}) = s
+"""
+    nbytes(x) -> Int
 
-isbytes(x) = x isa AbstractVector{UInt8}
+Length in bytes of `x` if `x` is `isbytes(x)`.
+"""
+function nbytes end
+nbytes(x) = nothing
+nbytes(x::AbstractVector{UInt8}) = length(x)
+nbytes(x::AbstractString) = sizeof(x)
+nbytes(x::Vector{T}) where T <: AbstractString = sum(sizeof, x)
+nbytes(x::Vector{T}) where T <: AbstractVector{UInt8} = sum(length, x)
+nbytes(x::IOBuffer) = bytesavailable(x)
+nbytes(x::Vector{IOBuffer}) = sum(bytesavailable, x)
 
 """
     isioerror(exception)

src/MessageRequest.jl

@@ -6,7 +6,6 @@ using ..Base64
 using ..IOExtras
 using URIs
 using ..Messages
-import ..Messages: bodylength
 import ..Headers
 import ..Form, ..content_type
 
@@ -35,8 +34,8 @@ function messagelayer(handler)
         if !hasheader(headers, "Content-Length") &&
         !hasheader(headers, "Transfer-Encoding") &&
         !hasheader(headers, "Upgrade")
-            l = bodylength(body)
-            if l != unknown_length
+            l = nbytes(body)
+            if l !== nothing
                 setheader(headers, "Content-Length" => string(l))
             elseif method == "GET" && iofunction isa Function
                 setheader(headers, "Content-Length" => "0")
@@ -47,7 +46,7 @@ function messagelayer(handler)
             setheader(headers, content_type(body))
         end
         parent = get(ctx, :parentrequest, nothing)
-        req = Request(method, resource(url), headers, bodybytes(body); url=url, version=http_version, responsebody=response_stream, parent=parent)
+        req = Request(method, resource(url), headers, body; url=url, version=http_version, responsebody=response_stream, parent=parent)
 
         return handler(ctx, req; iofunction=iofunction, kw...)
     end
@@ -67,20 +66,4 @@ function setuseragent!(x::Union{String, Nothing})
     return
 end
 
-bodylength(body) = unknown_length
-bodylength(body::AbstractVector{UInt8}) = length(body)
-bodylength(body::AbstractString) = sizeof(body)
-bodylength(body::Form) = length(body)
-bodylength(body::Vector{T}) where T <: AbstractString = sum(sizeof, body)
-bodylength(body::Vector{T}) where T <: AbstractArray{UInt8,1} = sum(length, body)
-bodylength(body::IOBuffer) = bytesavailable(body)
-bodylength(body::Vector{IOBuffer}) = sum(bytesavailable, body)
-
-bodybytes(body) = body
-bodybytes(body::Vector{UInt8}) = body
-bodybytes(body::IOBuffer) = read(body)
-bodybytes(body::AbstractVector{UInt8}) = Vector{UInt8}(body)
-bodybytes(body::AbstractString) = bytes(body)
-bodybytes(body::Vector) = length(body) == 1 ? bodybytes(body[1]) : body
-
 end # module MessageRequest

src/Messages.jl

@@ -115,8 +115,8 @@ mutable struct Response{T} <: Message
     @doc """
         Response(status::Int, headers=[]; body=UInt8[], request=nothing) -> HTTP.Response
     """
-    function Response(status::Integer, headers=[]; body=nobody, request=nothing)
-        b = body isa IO ? body : bytes(body)
+    function Response(status::Integer, headers=[]; body=nothing, request=nothing)
+        b = isbytes(body) ? bytes(body) : something(body, nobody)
         return new{typeof(b)}(
             v"1.1",
             status,
@@ -141,9 +141,9 @@ HTTP.Response(200, headers; body = "Hello")
 Response() = Request().response
 
 Response(s::Int, body::AbstractVector{UInt8}) = Response(s; body=body)
-Response(s::Int, body::AbstractString) = Response(s, bytes(body))
+Response(s::Int, body::AbstractString) = Response(s; body=bytes(body))
 
-Response(body) = Response(200, body)
+Response(body) = Response(200; body=body)
 
 Base.convert(::Type{Response}, s::AbstractString) = Response(s)
 
@@ -231,13 +231,13 @@ For daily use, see [`HTTP.request`](@ref).
 """
 function Request(method::String, target, headers=[], body=nobody;
                  version=v"1.1", url::URI=URI(), responsebody=nothing, parent=nothing)
-    b = body isa IO ? body : bytes(something(body, nobody))
+    b = isbytes(body) ? bytes(body) : something(body, nobody)
     r = Request{typeof(b)}(method,
                 target == "" ? "/" : target,
                 version,
                 mkheaders(headers),
                 b,
-                Response(0; body=something(responsebody, nobody)),
+                Response(0; body=responsebody),
                 url,
                 parent)
     r.response.request = r
@@ -591,7 +591,7 @@ body_show_max = 1000
 
 The first chunk of the Message Body (for display purposes).
 """
-bodysummary(bytes) = view(bytes, 1:min(length(bytes), body_show_max))
+bodysummary(body) = isbytes(body) ? view(bytes(body), 1:min(nbytes(body), body_show_max)) : "[Message Body was streamed]"
 
 function compactstartline(m::Message)
     b = IOBuffer()
@@ -617,8 +617,8 @@ function Base.show(io::IO, m::Message)
     summary = bodysummary(m.body)
     validsummary = isvalidstr(summary)
     validsummary && write(io, summary)
-    if !validsummary || length(m.body) > length(summary)
-        println(io, "\n⋮\n$(length(m.body))-byte body")
+    if !validsummary || something(nbytes(m.body), 0) > length(summary)
+        println(io, "\n⋮\n$(nbytes(m.body))-byte body")
     end
     print(io, "\"\"\"")
     return

src/RedirectRequest.jl

@@ -15,7 +15,6 @@ Redirects the request in the case of 3xx response status.
 """
 function redirectlayer(handler)
     return function(ctx, method, url, headers, body; redirect::Bool=true, redirect_limit::Int=3, forwardheaders::Bool=true, kw...)
-        println("redirectlayer")
         if !redirect || redirect_limit == 0
             # no redirecting
             return handler(ctx, method, url, headers, body; kw...)
@@ -54,8 +53,7 @@ function redirectlayer(handler)
             else
                 headers = Header[]
             end
-            @show 1 "➡️  Redirect: $url"
-            @show headers
+            @debug 1 "➡️  Redirect: $url"
             count += 1
         end
         @assert false "Unreachable!"

src/RetryRequest.jl

@@ -54,8 +54,8 @@ isrecoverable(e::HTTP.StatusError) = e.status == 403 || # Forbidden
 
 isrecoverable(e, req, retry_non_idempotent, retrycount) =
     isrecoverable(e) &&
-    !(req.body === body_was_streamed) &&
-    !(req.response.body === body_was_streamed) &&
+    isbytes(req.body) &&
+    isbytes(req.response.body) &&
     (retry_non_idempotent || retrycount == 0 || isidempotent(req))
     # "MUST NOT automatically retry a request with a non-idempotent method"
     # https://tools.ietf.org/html/rfc7230#section-6.3.1
@@ -67,8 +67,8 @@ function no_retry_reason(ex, req)
     print(buf, ", ",
         ex isa HTTP.StatusError ? "HTTP $(ex.status): " :
         !isrecoverable(ex) ?  "$ex not recoverable, " : "",
-        (req.body === body_was_streamed) ? "request streamed, " : "",
-        (req.response.body === body_was_streamed) ? "response streamed, " : "",
+        !isbytes(req.body) ? "request streamed, " : "",
+        !isbytes(req.response.body) ? "response streamed, " : "",
         !isidempotent(req) ? "$(req.method) non-idempotent" : "")
     return String(take!(buf))
 end

src/StreamRequest.jl

@@ -23,6 +23,7 @@ indicates that the server does not wish to receive the message body.
 function streamlayer(ctx, stream::Stream; iofunction=nothing, verbose=0, kw...)::Response
     response = stream.message
     req = response.request
+    io = stream.stream
     verbose == 1 && printlncompact(req)
     @debug 2 "client startwrite"
     startwrite(stream)
@@ -53,7 +54,6 @@ function streamlayer(ctx, stream::Stream; iofunction=nothing, verbose=0, kw...):
             else
                 iofunction(stream)
             end
-
             if isaborted(stream)
                 # The server may have closed the connection.
                 # Don't propagate such errors.

src/multipart.jl

@@ -10,6 +10,7 @@ Base.eof(f::Form) = f.index > length(f.data)
 Base.isopen(f::Form) = false
 Base.close(f::Form) = nothing
 Base.length(f::Form) = sum(x->isa(x, IOStream) ? filesize(x) - position(x) : bytesavailable(x), f.data)
+IOExtras.nbytes(x::Form) = length(x)
 function Base.position(f::Form)
     index = f.index
     foreach(mark, f.data)

src/sniff.jl

@@ -1,3 +1,5 @@
+const CodeUnits = Union{Vector{UInt8}, Base.CodeUnits}
+
 # compression detection
 const ZIP = UInt8[0x50, 0x4b, 0x03, 0x04]
 const GZIP = UInt8[0x1f, 0x8b, 0x08]

test/client.jl

@@ -54,7 +54,9 @@ end
         @test replace(replace(body, " "=>""), "\n"=>"")  == "{\"cookies\":{\"foo\":\"bar\",\"hey\":\"sailor\"}}"
 
         r = HTTP.get("$sch://httpbin.org/cookies/delete?hey")
-        @test isempty(JSON.parse(String(r.body))["cookies"])
+        cookies = JSON.parse(String(r.body))["cookies"]
+        @test length(cookies) == 1
+        @test cookies["foo"] == "bar"
     end
 
     @testset "Client Streaming Test" begin

test/cookies.jl

@@ -1,3 +1,8 @@
+module TestCookies
+
+using HTTP
+using Sockets, Test
+
 @testset "Cookies" begin
     c = HTTP.Cookies.Cookie()
     @test c.name == ""
@@ -173,3 +178,5 @@
         @test istaskdone(tsk)
     end
 end
+
+end # module

test/loopback.jl

@@ -1,10 +1,11 @@
+module TestLoopback
+
 using Test
 using HTTP
 using HTTP.IOExtras
 using HTTP.Parsers
 using HTTP.Messages
 using HTTP.Sockets
-using HTTP.MessageRequest: bodylength
 
 mutable struct FunctionIO <: IO
     f::Function
@@ -289,21 +290,20 @@ end
             FunctionIO(()->(sleep(0.1); " World!"))])
         @test String(r.body) == "Hello World!"
 
-        hello_sent = false
-        world_sent = false
+        hello_sent = Ref(false)
+        world_sent = Ref(false)
         @test_throws HTTP.StatusError begin
             r = lbreq("abort", [], [
-                FunctionIO(()->(hello_sent = true; sleep(0.1); "Hello")),
-                FunctionIO(()->(world_sent = true; " World!"))])
+                FunctionIO(()->(println("hello_sent"); hello_sent[] = true; sleep(0.5); "Hello")),
+                FunctionIO(()->(println("world_sent"); world_sent[] = true; " World!"))])
         end
-        @test hello_sent
-        @test !world_sent
+        @test hello_sent[]
+        @test !world_sent[]
     end
 
     @testset "ASync - Pipeline limit = 0" begin
         server_events = []
         t = async_test(;pipeline_limit=0)
-        @show t
         if haskey(ENV, "HTTP_JL_TEST_TIMING_SENSITIVE")
             @test server_events == [
                 "Request: GET /delay1 HTTP/1.1",
@@ -322,7 +322,6 @@ end
     @testset "ASync - " begin
         server_events = []
         t = async_test()
-        @show t
         if haskey(ENV, "HTTP_JL_TEST_TIMING_SENSITIVE")
             @test server_events == [
                 "Request: GET /delay1 HTTP/1.1",
@@ -350,3 +349,5 @@ end
             "Response: HTTP/1.1 200 OK <= (POST /delay1 HTTP/1.1)"]
     end
 end
+
+end # module