From d4c8f91940ee8c648c9be3713994148fb53c1593 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Tue, 20 Jul 2021 01:19:05 +0200 Subject: [PATCH 01/24] add downloads backend --- Project.toml | 3 +- src/AWS.jl | 2 + src/utilities/downloads_backend.jl | 87 ++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 src/utilities/downloads_backend.jl diff --git a/Project.toml b/Project.toml index 1285eb2eba..933497f44e 100644 --- a/Project.toml +++ b/Project.toml @@ -7,6 +7,7 @@ version = "1.56.0" Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f" Compat = "34da2185-b29b-5c13-b0c7-acf172513d20" Dates = "ade2ca70-3891-5945-98fb-dc099432e06a" +Downloads = "f43a241f-c20a-4ad4-852c-f6b1247861c6" GitHub = "bc5e4493-9b4d-5f90-b8aa-2b2bcaad7a26" HTTP = "cd3eb016-35fb-5094-929b-558a96fad6f3" IniFile = "83e8ac13-25f8-5344-8a64-a9f2b223428f" @@ -32,7 +33,7 @@ OrderedCollections = "1" Retry = "0.3, 0.4" URIs = "1" XMLDict = "0.3, 0.4" -julia = "1" +julia = "1.3" [extras] Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f" diff --git a/src/AWS.jl b/src/AWS.jl index 7195d8b2e5..8148268007 100644 --- a/src/AWS.jl +++ b/src/AWS.jl @@ -3,6 +3,7 @@ module AWS using Compat: Compat, @something using Base64 using Dates +using Downloads using HTTP using MbedTLS using Mocking @@ -33,6 +34,7 @@ include("AWSMetadata.jl") include(joinpath("utilities", "request.jl")) include(joinpath("utilities", "sign.jl")) +include(joinpath("utilities", "downloads_backend.jl")) using ..AWSExceptions diff --git a/src/utilities/downloads_backend.jl b/src/utilities/downloads_backend.jl new file mode 100644 index 0000000000..c37681e6b5 --- /dev/null +++ b/src/utilities/downloads_backend.jl @@ -0,0 +1,87 @@ +struct DownloadsBackend <: AWS.AbstractBackend + downloader::Union{Nothing, Downloads.Downloader} +end + +DownloadsBackend() = DownloadsBackend(nothing) + +const AWS_DOWNLOADER = Ref{Union{Nothing, Downloader}}(nothing) +const AWS_DOWNLOAD_LOCK = ReentrantLock() + +# Here we mimic Download.jl's own setup for using a global downloader. +# We do this to have our own downloader (separate from Downloads.jl's global downloader) +# because we add a hook to avoid redirects in order to try to match the HTTPBackend's +# implementation, and we don't want to mutate the global downloader from Downloads.jl. +# https://github.com/JuliaLang/Downloads.jl/blob/84e948c02b8a0625552a764bf90f7d2ee97c949c/src/Downloads.jl#L293-L301 +function get_downloader(downloader=nothing) + lock(AWS_DOWNLOAD_LOCK) do + yield() # let other downloads finish + downloader isa Downloader && return + while true + downloader = AWS_DOWNLOADER[] + downloader isa Downloader && return + D = Downloader() + D.easy_hook = (easy, info) -> Curl.setopt(easy, Curl.CURLOPT_FOLLOWLOCATION, false) + AWS_DOWNLOADER[] = D + end + end + return downloader +end + + +function AWS._http_request(backend::DownloadsBackend, request) + # If we pass `output`, Downloads.jl will expect a message + # body in the response. Specifically, it sets + # + # only when we do not pass the `output` argument. + # + # When the method is `HEAD`, the response may have a Content-Length + # but not send any content back (which appears to be correct, + # ). + # + # Thus, if we did not set `CURLOPT_NOBODY`, and it gets a Content-Length + # back, it will hang waiting for that body. + # + # Therefore, we do not pass an `output` when the `request_method` is `HEAD`. + if request.request_method != "HEAD" + output = IOBuffer() + output_arg = (; output=output) + + # We set a callback so later on we know how to get the `body` back. + body_arg = () -> (; body = take!(output)) + else + output_arg = NamedTuple() + body_arg = () -> NamedTuple() + end + + # We pass an `input` only when we have content we wish to send. + if !isempty(request.content) + input = IOBuffer() + write(input, request.content) + input_arg = (; input=input) + else + input_arg = NamedTuple() + end + + @repeat 4 try + downloader = @something(backend.downloader, get_downloader()) + # set the hook so that we don't follow redirects. Only + # need to do this on per-request downloaders, because we + # set our global one with this hook already. + if backend.downloader !== nothing + downloader.easy_hook = (easy, info) -> Curl.setopt(easy, Curl.CURLOPT_FOLLOWLOCATION, false) + end + response = Downloads.request(request.url; input_arg..., output_arg..., + method = request.request_method, + request.headers, verbose=false, throw=true, + downloader) + http_response = HTTP.Response(response.status, response.headers; body_arg()..., request=nothing) + + if HTTP.iserror(http_response) + target = HTTP.resource(HTTP.URI(request.url)) + throw(HTTP.StatusError(http_response.status, request.request_method, target, http_response)) + end + return http_response + catch e + @delay_retry if ((isa(e, HTTP.StatusError) && AWS._http_status(e) >= 500) || isa(e, Downloads.RequestError)) end + end +end From cae56c4ae297a94b559f6bfde7c500b780ed77a7 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Tue, 20 Jul 2021 01:19:11 +0200 Subject: [PATCH 02/24] test with it --- test/runtests.jl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/runtests.jl b/test/runtests.jl index c79b758356..66cf990d68 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -34,6 +34,8 @@ function _now_formatted() return lowercase(Dates.format(now(Dates.UTC), dateformat"yyyymmdd\THHMMSSsss\Z")) end +AWS.DEFAULT_BACKEND[] = AWS.DownloadsBackend() + @testset "AWS.jl" begin include("AWS.jl") include("AWSCredentials.jl") From 9b917fa7522810bfab1eacd430546775b3055a6b Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Tue, 20 Jul 2021 01:22:07 +0200 Subject: [PATCH 03/24] bump CI to 1.3 --- .github/workflows/CI.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 64bd9f7a89..34bc225a4b 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -25,9 +25,9 @@ jobs: arch: - x64 include: - # Add a 1.0 job just to make sure we still support it + # Add a 1.3 job just to make sure we still support it - os: ubuntu-latest - version: 1.0.5 + version: 1.3 arch: x64 # Add a 1.5 job because that's what Invenia actually uses - os: ubuntu-latest From c8329a981eb4d10a41b692ea1cfbaa4deefaed70 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Tue, 20 Jul 2021 01:46:28 +0200 Subject: [PATCH 04/24] fix import --- src/AWS.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AWS.jl b/src/AWS.jl index 8148268007..40b33cf11d 100644 --- a/src/AWS.jl +++ b/src/AWS.jl @@ -3,7 +3,7 @@ module AWS using Compat: Compat, @something using Base64 using Dates -using Downloads +using Downloads: Downloads, Downloader, Curl using HTTP using MbedTLS using Mocking From e66dca92c50255cd3c113f747d9784e81d63c624 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Tue, 27 Jul 2021 19:31:01 +0200 Subject: [PATCH 05/24] Update .github/workflows/CI.yml Co-authored-by: Curtis Vogt --- .github/workflows/CI.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 34bc225a4b..f1875ddfd9 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -25,7 +25,7 @@ jobs: arch: - x64 include: - # Add a 1.3 job just to make sure we still support it + # Add a job using the earliest version of Julia supported by this package - os: ubuntu-latest version: 1.3 arch: x64 From 96a96ea2b87987ff5ae650065987935c1e9b12d4 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Tue, 27 Jul 2021 19:40:00 +0200 Subject: [PATCH 06/24] Update src/utilities/downloads_backend.jl --- src/utilities/downloads_backend.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/src/utilities/downloads_backend.jl b/src/utilities/downloads_backend.jl index c37681e6b5..53fd5adc5f 100644 --- a/src/utilities/downloads_backend.jl +++ b/src/utilities/downloads_backend.jl @@ -57,6 +57,7 @@ function AWS._http_request(backend::DownloadsBackend, request) if !isempty(request.content) input = IOBuffer() write(input, request.content) + seekstart(input) input_arg = (; input=input) else input_arg = NamedTuple() From 2fd84d40cc05dae165d30dd2a21b300b99542f46 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Tue, 27 Jul 2021 19:52:10 +0200 Subject: [PATCH 07/24] fix syntax for 1.3 --- src/utilities/downloads_backend.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utilities/downloads_backend.jl b/src/utilities/downloads_backend.jl index 53fd5adc5f..d94c100c93 100644 --- a/src/utilities/downloads_backend.jl +++ b/src/utilities/downloads_backend.jl @@ -73,8 +73,8 @@ function AWS._http_request(backend::DownloadsBackend, request) end response = Downloads.request(request.url; input_arg..., output_arg..., method = request.request_method, - request.headers, verbose=false, throw=true, - downloader) + headers = request.headers, verbose=false, throw=true, + downloader=downloader) http_response = HTTP.Response(response.status, response.headers; body_arg()..., request=nothing) if HTTP.iserror(http_response) From 8311723099d4a3c009a7bfcbedbc9325f93b3967 Mon Sep 17 00:00:00 2001 From: Eric Hanson <5846501+ericphanson@users.noreply.github.com> Date: Tue, 27 Jul 2021 20:13:41 +0200 Subject: [PATCH 08/24] add timeout --- .github/workflows/CI.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index f1875ddfd9..109be9756d 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -13,6 +13,7 @@ jobs: test: name: Julia ${{ matrix.version }} - ${{ matrix.os }} - ${{ matrix.arch }} runs-on: ${{ matrix.os }} + timeout-minutes: 30 continue-on-error: ${{ matrix.version == 'nightly' }} strategy: fail-fast: false From 31e126d93ca05dd9e45aaedf5936902dcc7c928d Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Thu, 12 Aug 2021 18:27:23 -0300 Subject: [PATCH 09/24] Seek to start of body on every attempt --- src/utilities/downloads_backend.jl | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/utilities/downloads_backend.jl b/src/utilities/downloads_backend.jl index d94c100c93..67a33f75f2 100644 --- a/src/utilities/downloads_backend.jl +++ b/src/utilities/downloads_backend.jl @@ -37,10 +37,10 @@ function AWS._http_request(backend::DownloadsBackend, request) # When the method is `HEAD`, the response may have a Content-Length # but not send any content back (which appears to be correct, # ). - # + # # Thus, if we did not set `CURLOPT_NOBODY`, and it gets a Content-Length # back, it will hang waiting for that body. - # + # # Therefore, we do not pass an `output` when the `request_method` is `HEAD`. if request.request_method != "HEAD" output = IOBuffer() @@ -54,16 +54,18 @@ function AWS._http_request(backend::DownloadsBackend, request) end # We pass an `input` only when we have content we wish to send. + input = IOBuffer() if !isempty(request.content) - input = IOBuffer() write(input, request.content) - seekstart(input) input_arg = (; input=input) else input_arg = NamedTuple() end @repeat 4 try + # We seekstart on every attempt, otherwise every attempt + # but the first will send an empty payload. + seekstart(input) downloader = @something(backend.downloader, get_downloader()) # set the hook so that we don't follow redirects. Only # need to do this on per-request downloaders, because we @@ -75,7 +77,7 @@ function AWS._http_request(backend::DownloadsBackend, request) method = request.request_method, headers = request.headers, verbose=false, throw=true, downloader=downloader) - http_response = HTTP.Response(response.status, response.headers; body_arg()..., request=nothing) + http_response = HTTP.Response(response.status, response.headers; body_arg()..., request=nothing) if HTTP.iserror(http_response) target = HTTP.resource(HTTP.URI(request.url)) From 383eced6e95c1ed87719a6fd69169de5bf26e46a Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Thu, 12 Aug 2021 20:21:16 -0300 Subject: [PATCH 10/24] Set Content-Length --- src/utilities/downloads_backend.jl | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/utilities/downloads_backend.jl b/src/utilities/downloads_backend.jl index 67a33f75f2..343739e378 100644 --- a/src/utilities/downloads_backend.jl +++ b/src/utilities/downloads_backend.jl @@ -53,6 +53,9 @@ function AWS._http_request(backend::DownloadsBackend, request) body_arg = () -> NamedTuple() end + # HTTP.jl sets this header automatically. + request.headers["Content-Length"] = string(length(request.content)) + # We pass an `input` only when we have content we wish to send. input = IOBuffer() if !isempty(request.content) @@ -63,9 +66,6 @@ function AWS._http_request(backend::DownloadsBackend, request) end @repeat 4 try - # We seekstart on every attempt, otherwise every attempt - # but the first will send an empty payload. - seekstart(input) downloader = @something(backend.downloader, get_downloader()) # set the hook so that we don't follow redirects. Only # need to do this on per-request downloaders, because we @@ -73,6 +73,11 @@ function AWS._http_request(backend::DownloadsBackend, request) if backend.downloader !== nothing downloader.easy_hook = (easy, info) -> Curl.setopt(easy, Curl.CURLOPT_FOLLOWLOCATION, false) end + + # We seekstart on every attempt, otherwise every attempt + # but the first will send an empty payload. + seekstart(input) + response = Downloads.request(request.url; input_arg..., output_arg..., method = request.request_method, headers = request.headers, verbose=false, throw=true, From d979da48fd8af490d030f505bcb69bbbca42c718 Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Thu, 12 Aug 2021 20:27:17 -0300 Subject: [PATCH 11/24] Test with explicit HTTPBackend --- test/AWS.jl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/AWS.jl b/test/AWS.jl index ca6fdcb96d..ed83ef0ee1 100644 --- a/test/AWS.jl +++ b/test/AWS.jl @@ -365,6 +365,7 @@ end api_version="api_version", request_method="GET", url="https://s3.us-east-1.amazonaws.com/sample-bucket", + backend=AWS.HTTPBackend(), ) apply(Patches._http_options_patch) do # No default options @@ -393,7 +394,7 @@ end api_version="api_version", request_method="GET", url="https://s3.us-east-1.amazonaws.com/sample-bucket", - backend = TestBackend(4) + backend = TestBackend(4), ) @test AWS._http_request(request.backend, request) == 4 From bffb900c8b6c59c43f40fc86d0ff1ccd8d8728fc Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Thu, 12 Aug 2021 20:39:02 -0300 Subject: [PATCH 12/24] More correct Content-Length --- src/utilities/downloads_backend.jl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/utilities/downloads_backend.jl b/src/utilities/downloads_backend.jl index 343739e378..bfc183ca48 100644 --- a/src/utilities/downloads_backend.jl +++ b/src/utilities/downloads_backend.jl @@ -27,6 +27,10 @@ function get_downloader(downloader=nothing) return downloader end +# https://github.com/JuliaWeb/HTTP.jl/blob/2a03ca76376162ffc3423ba7f15bd6d966edff9b/src/MessageRequest.jl#L84-L85 +body_length(x::Vector{UInt8}) = length(x) +body_length(x::String) = sizeof(x) + function AWS._http_request(backend::DownloadsBackend, request) # If we pass `output`, Downloads.jl will expect a message @@ -54,7 +58,7 @@ function AWS._http_request(backend::DownloadsBackend, request) end # HTTP.jl sets this header automatically. - request.headers["Content-Length"] = string(length(request.content)) + request.headers["Content-Length"] = string(body_length(request.content)) # We pass an `input` only when we have content we wish to send. input = IOBuffer() From a549d8708aec47121ed6c66452cf2f61b1a83dcd Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Fri, 13 Aug 2021 10:32:17 -0300 Subject: [PATCH 13/24] Support `response_stream` and `return_stream` --- src/utilities/downloads_backend.jl | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/utilities/downloads_backend.jl b/src/utilities/downloads_backend.jl index bfc183ca48..71cfb35a4c 100644 --- a/src/utilities/downloads_backend.jl +++ b/src/utilities/downloads_backend.jl @@ -46,15 +46,14 @@ function AWS._http_request(backend::DownloadsBackend, request) # back, it will hang waiting for that body. # # Therefore, we do not pass an `output` when the `request_method` is `HEAD`. - if request.request_method != "HEAD" - output = IOBuffer() - output_arg = (; output=output) - - # We set a callback so later on we know how to get the `body` back. - body_arg = () -> (; body = take!(output)) + if request.return_stream && request.response_stream === nothing + request.response_stream === IOBuffer() + end + output = @something(request.response_stream, IOBuffer()) + output_arg, body_arg = if request.request_method != "HEAD" + (; output=output), () -> (; body = take!(output)) else - output_arg = NamedTuple() - body_arg = () -> NamedTuple() + NamedTuple(), () -> NamedTuple() end # HTTP.jl sets this header automatically. @@ -62,11 +61,11 @@ function AWS._http_request(backend::DownloadsBackend, request) # We pass an `input` only when we have content we wish to send. input = IOBuffer() - if !isempty(request.content) + input_arg = if !isempty(request.content) write(input, request.content) - input_arg = (; input=input) + (; input=input) else - input_arg = NamedTuple() + NamedTuple() end @repeat 4 try @@ -86,6 +85,7 @@ function AWS._http_request(backend::DownloadsBackend, request) method = request.request_method, headers = request.headers, verbose=false, throw=true, downloader=downloader) + http_response = HTTP.Response(response.status, response.headers; body_arg()..., request=nothing) if HTTP.iserror(http_response) From a370fd1b2f2ed22ac8a8a102459418e62b7b81e5 Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Fri, 13 Aug 2021 11:33:12 -0300 Subject: [PATCH 14/24] Support other IOs, hopefully --- src/utilities/downloads_backend.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utilities/downloads_backend.jl b/src/utilities/downloads_backend.jl index 71cfb35a4c..6f805c6813 100644 --- a/src/utilities/downloads_backend.jl +++ b/src/utilities/downloads_backend.jl @@ -51,7 +51,7 @@ function AWS._http_request(backend::DownloadsBackend, request) end output = @something(request.response_stream, IOBuffer()) output_arg, body_arg = if request.request_method != "HEAD" - (; output=output), () -> (; body = take!(output)) + (; output=output), () -> (; body = readavailable(output)) else NamedTuple(), () -> NamedTuple() end From 93d4a29a0ce725f8f02c60cd4910fe0b170ea289 Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Fri, 13 Aug 2021 12:33:48 -0300 Subject: [PATCH 15/24] Fix for return_stream, make sure test doesn't hang --- src/utilities/downloads_backend.jl | 16 +++++++++++----- test/issues.jl | 6 +++++- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/utilities/downloads_backend.jl b/src/utilities/downloads_backend.jl index 6f805c6813..b11246a5dd 100644 --- a/src/utilities/downloads_backend.jl +++ b/src/utilities/downloads_backend.jl @@ -28,9 +28,11 @@ function get_downloader(downloader=nothing) end # https://github.com/JuliaWeb/HTTP.jl/blob/2a03ca76376162ffc3423ba7f15bd6d966edff9b/src/MessageRequest.jl#L84-L85 -body_length(x::Vector{UInt8}) = length(x) -body_length(x::String) = sizeof(x) +body_length(x::AbstractVector{UInt8}) = length(x) +body_length(x::AbstractString) = sizeof(x) +read_body(x::IOBuffer) = take!(x) +read_body(x::IO) = readavailable(x) function AWS._http_request(backend::DownloadsBackend, request) # If we pass `output`, Downloads.jl will expect a message @@ -50,10 +52,14 @@ function AWS._http_request(backend::DownloadsBackend, request) request.response_stream === IOBuffer() end output = @something(request.response_stream, IOBuffer()) - output_arg, body_arg = if request.request_method != "HEAD" - (; output=output), () -> (; body = readavailable(output)) + output_arg = request.request_method == "HEAD" ? NamedTuple() : (; output=output) + # If we're going to return the stream, we don't want to read the body into an + # HTTP.Response we're never going to use. If we do that, the returned stream + # will have no data available (and reading from it could hang forever). + body_arg = if request.request_method == "HEAD" || request.return_stream + NamedTuple() else - NamedTuple(), () -> NamedTuple() + (; body = read_body(output)) end # HTTP.jl sets this header automatically. diff --git a/test/issues.jl b/test/issues.jl index 3c9deb4863..4205ee98a5 100644 --- a/test/issues.jl +++ b/test/issues.jl @@ -64,7 +64,11 @@ end # => BUG: header `response_stream` is pushed into the query... io = Base.BufferStream() S3.get_object(bucket_name, file_name, Dict("response_stream"=>io, "return_stream"=>true)) - @test String(read(io)) == body + if bytesavailable(io) > 0 + @test String(readavailable(io)) == body + else + @test "no body data was available" == body + end finally S3.delete_object(bucket_name, file_name) From be7adb13cbd72204ddc2e5b5a8599aea0c25586f Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Fri, 13 Aug 2021 12:40:14 -0300 Subject: [PATCH 16/24] oops, it's a function --- src/utilities/downloads_backend.jl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/utilities/downloads_backend.jl b/src/utilities/downloads_backend.jl index b11246a5dd..1d7b360e46 100644 --- a/src/utilities/downloads_backend.jl +++ b/src/utilities/downloads_backend.jl @@ -53,13 +53,14 @@ function AWS._http_request(backend::DownloadsBackend, request) end output = @something(request.response_stream, IOBuffer()) output_arg = request.request_method == "HEAD" ? NamedTuple() : (; output=output) + # If we're going to return the stream, we don't want to read the body into an # HTTP.Response we're never going to use. If we do that, the returned stream # will have no data available (and reading from it could hang forever). body_arg = if request.request_method == "HEAD" || request.return_stream - NamedTuple() + () -> NamedTuple() else - (; body = read_body(output)) + () -> (; body = read_body(output)) end # HTTP.jl sets this header automatically. From 3bf0a31b3a9aa62985e20214e90c2cf647a121fc Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Fri, 13 Aug 2021 13:56:35 -0300 Subject: [PATCH 17/24] Always set `request.response_stream` to an IO --- src/utilities/downloads_backend.jl | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/utilities/downloads_backend.jl b/src/utilities/downloads_backend.jl index 1d7b360e46..a92bdc9163 100644 --- a/src/utilities/downloads_backend.jl +++ b/src/utilities/downloads_backend.jl @@ -48,11 +48,14 @@ function AWS._http_request(backend::DownloadsBackend, request) # back, it will hang waiting for that body. # # Therefore, we do not pass an `output` when the `request_method` is `HEAD`. - if request.return_stream && request.response_stream === nothing - request.response_stream === IOBuffer() + if request.response_stream === nothing + request.response_stream = IOBuffer() + end + output_arg = if request.request_method == "HEAD" + NamedTuple() + else + (; output=request.response_stream) end - output = @something(request.response_stream, IOBuffer()) - output_arg = request.request_method == "HEAD" ? NamedTuple() : (; output=output) # If we're going to return the stream, we don't want to read the body into an # HTTP.Response we're never going to use. If we do that, the returned stream @@ -60,7 +63,7 @@ function AWS._http_request(backend::DownloadsBackend, request) body_arg = if request.request_method == "HEAD" || request.return_stream () -> NamedTuple() else - () -> (; body = read_body(output)) + () -> (; body = read_body(request.response_stream)) end # HTTP.jl sets this header automatically. From 10d81ee15979f07bb0b8100cf9a312d4a084300f Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Fri, 13 Aug 2021 14:15:29 -0300 Subject: [PATCH 18/24] Apply suggestions from code review Co-authored-by: Eric Hanson <5846501+ericphanson@users.noreply.github.com> --- src/utilities/downloads_backend.jl | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/utilities/downloads_backend.jl b/src/utilities/downloads_backend.jl index a92bdc9163..830304835f 100644 --- a/src/utilities/downloads_backend.jl +++ b/src/utilities/downloads_backend.jl @@ -34,11 +34,12 @@ body_length(x::AbstractString) = sizeof(x) read_body(x::IOBuffer) = take!(x) read_body(x::IO) = readavailable(x) -function AWS._http_request(backend::DownloadsBackend, request) - # If we pass `output`, Downloads.jl will expect a message - # body in the response. Specifically, it sets +function _http_request(backend::DownloadsBackend, request) + # If we pass `output`, older versions of Downloads.jl will + # expect a message body in the response. Specifically, it sets # # only when we do not pass the `output` argument. + # See . # # When the method is `HEAD`, the response may have a Content-Length # but not send any content back (which appears to be correct, @@ -48,6 +49,8 @@ function AWS._http_request(backend::DownloadsBackend, request) # back, it will hang waiting for that body. # # Therefore, we do not pass an `output` when the `request_method` is `HEAD`. + # (Note: this is fixed on the latest Downloads.jl, but we include this workaround + # for compatability). if request.response_stream === nothing request.response_stream = IOBuffer() end @@ -83,7 +86,7 @@ function AWS._http_request(backend::DownloadsBackend, request) # set the hook so that we don't follow redirects. Only # need to do this on per-request downloaders, because we # set our global one with this hook already. - if backend.downloader !== nothing + if backend.downloader !== nothing && downloader.easy_hook === nothing downloader.easy_hook = (easy, info) -> Curl.setopt(easy, Curl.CURLOPT_FOLLOWLOCATION, false) end From 73c0bbfd51276568ce72424f8aff0397636db038 Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Fri, 13 Aug 2021 14:28:24 -0300 Subject: [PATCH 19/24] Safer(?) `read_body` Safe to assume that any `IO` has a `close`, right? --- src/utilities/downloads_backend.jl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/utilities/downloads_backend.jl b/src/utilities/downloads_backend.jl index 830304835f..36aefba9f7 100644 --- a/src/utilities/downloads_backend.jl +++ b/src/utilities/downloads_backend.jl @@ -32,7 +32,10 @@ body_length(x::AbstractVector{UInt8}) = length(x) body_length(x::AbstractString) = sizeof(x) read_body(x::IOBuffer) = take!(x) -read_body(x::IO) = readavailable(x) +function read_body(x::IO) + close(x) + return read(x) +end function _http_request(backend::DownloadsBackend, request) # If we pass `output`, older versions of Downloads.jl will From 5d50a615a257cafc59279a542decf824761b5ad4 Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Fri, 13 Aug 2021 14:31:53 -0300 Subject: [PATCH 20/24] Run some tests on both backends --- test/runtests.jl | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/test/runtests.jl b/test/runtests.jl index 66cf990d68..26f97e190e 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -34,18 +34,21 @@ function _now_formatted() return lowercase(Dates.format(now(Dates.UTC), dateformat"yyyymmdd\THHMMSSsss\Z")) end -AWS.DEFAULT_BACKEND[] = AWS.DownloadsBackend() - @testset "AWS.jl" begin - include("AWS.jl") - include("AWSCredentials.jl") include("AWSExceptions.jl") include("AWSMetadataUtilities.jl") - include("issues.jl") include("test_pkg.jl") include("utilities.jl") - if haskey(ENV, "TEST_MINIO") - include("minio.jl") + backends = [AWS.HTTPBackend, AWS.DownloadsBackend] + @testset "Backend: $(nameof(backend))" for backend in backends + AWS.DEFAULT_BACKEND[] = backend() + include("AWS.jl") + include("AWSCredentials.jl") + include("issues.jl") + + if haskey(ENV, "TEST_MINIO") + include("minio.jl") + end end end From 97838369447a3a59b9a771ea8a5a8e278e3da7c0 Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Fri, 13 Aug 2021 14:47:43 -0300 Subject: [PATCH 21/24] Replace old user agent after test --- test/AWS.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/test/AWS.jl b/test/AWS.jl index ed83ef0ee1..914fd49d5b 100644 --- a/test/AWS.jl +++ b/test/AWS.jl @@ -34,6 +34,7 @@ end @test AWS.user_agent[] == "AWS.jl/1.0.0" set_user_agent(new_user_agent) @test AWS.user_agent[] == new_user_agent + set_user_agent("AWS.jl/1.0.0") end @testset "sign" begin From 9c2da08fc1ac7a7266e8e0e0c6e73d7f7c3bc409 Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Fri, 13 Aug 2021 15:11:02 -0300 Subject: [PATCH 22/24] The tests are too fast --- src/AWSCredentials.jl | 2 +- test/AWSCredentials.jl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/AWSCredentials.jl b/src/AWSCredentials.jl index 35a4f9478b..990b0a5fb5 100644 --- a/src/AWSCredentials.jl +++ b/src/AWSCredentials.jl @@ -538,7 +538,7 @@ function credentials_from_webtoken() role_creds["SessionToken"], assumed_role_user["Arn"]; expiry=DateTime(rstrip(role_creds["Expiration"], 'Z')), - renew=credentials_from_webtoken + renew=credentials_from_webtoken, ) end diff --git a/test/AWSCredentials.jl b/test/AWSCredentials.jl index 9f14f62943..25f1ffb1f9 100644 --- a/test/AWSCredentials.jl +++ b/test/AWSCredentials.jl @@ -565,7 +565,7 @@ end @test result.user_arn == "$(role_arn)/$(session_name)" @test result.renew == credentials_from_webtoken expiry = result.expiry - + sleep(0.1) result = check_credentials(result) @test result.access_key_id == access_key From 1c6c1bfabe8c09621c2547a00bca7b7b4983e02e Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Fri, 13 Aug 2021 16:09:42 -0300 Subject: [PATCH 23/24] Safer user agent test --- test/AWS.jl | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/AWS.jl b/test/AWS.jl index 914fd49d5b..114be27621 100644 --- a/test/AWS.jl +++ b/test/AWS.jl @@ -29,12 +29,16 @@ end end @testset "set user agent" begin + old_user_agent = AWS.user_agent[] new_user_agent = "new user agent" - @test AWS.user_agent[] == "AWS.jl/1.0.0" - set_user_agent(new_user_agent) - @test AWS.user_agent[] == new_user_agent - set_user_agent("AWS.jl/1.0.0") + try + @test AWS.user_agent[] == "AWS.jl/1.0.0" + set_user_agent(new_user_agent) + @test AWS.user_agent[] == new_user_agent + finally + set_user_agent(old_user_agent) + end end @testset "sign" begin From 8d638537c077899562012a36ea5fa0808bf3eac8 Mon Sep 17 00:00:00 2001 From: Chris de Graaf Date: Fri, 13 Aug 2021 16:54:37 -0300 Subject: [PATCH 24/24] Update bors config to look for 1.3 status --- bors.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bors.toml b/bors.toml index 39d0db289c..9fcf79fdab 100644 --- a/bors.toml +++ b/bors.toml @@ -1,5 +1,5 @@ status = [ - "Julia 1.0.5 - ubuntu-latest - x64", + "Julia 1.3 - ubuntu-latest - x64", "Julia 1.5 - ubuntu-latest - x64", "Julia 1 - macOS-latest - x64", "Julia 1 - ubuntu-latest - x64",