Skip to content

Commit

Permalink
[release-1.5] Backport bugfixes to release-1.5 branch (#175)
Browse files Browse the repository at this point in the history
* Before building and testing the package, make sure that the UUID has not been edited (#128)

(cherry picked from commit 21843d0)

* CI: Standardize the workflow for testing and changing the UUID (#129)

(cherry picked from commit cd002c3)

* fix #131 and add test (#132)

(cherry picked from commit adbb974)

* Improve inferability of download() (#133)

(cherry picked from commit 848d374)

* fix ci badge (#137)

(cherry picked from commit 3870614)

* Fix a handful of invalidations in expression-checking (#138)

ChainRulesCore defines `==(a, b::AbstractThunk)` and its converse,
and this invalidates a couple of poorly-typed Symbol checks.
This more "SSA-like" way of writing the code is easier to infer.

(cherry picked from commit 25f7af3)

* tests: skip wrong host test for SSL_NO_VERIFY (fix #139) (#140)

Since #114, we only turn
off peer verification, not host verification when the `SSL_NO_VERIFY`
variables are set. This means that the last set of tests in the "SSL no
verify override" testset *should* fail for `wrong.host.badssl.com`. That
is not what I was seeing, however — the test was still passing — which I
found puzzling but just moved on with my life at the time. It turns out
that the test *does* fail if libcurl is build with OpenSSL. Since
whether the test passes or not for that host depends on how things are
built, this change simply skips the test (by popping the URL from the
set of tested URLS for that testset).

The tests above that which use the easy hook mechanism are fixed in a
different way: for those I made the hook disable both host and peer
verification, which should fix the tests for any bad host including when
the server sends the wrong host name.

(cherry picked from commit e22219f)

* Fix input body size detection for IOBuffer(codeunits(str)) (#143)

Somewhat surprisingly, the type of this is not IOBuffer, but a related
type (Base.GenericIOBuffer{Base.CodeUnits{UInt8, String}}).

(cherry picked from commit 470b7f0)

* Typo fix: indiation -> indication (#144)

(cherry picked from commit 5f1509d)

* use Timer instead of libuv timer API

(cherry picked from commit 11493ff)

* use FDWatcher instead of libuv poll API

(cherry picked from commit 4c1d2af)

* fix wrong definition of curl_socket_t on Windows

(cherry picked from commit 2eb0491)

* Revert "stop using raw libuv API" (#156)

(cherry picked from commit c91876a)

* Revert "Revert "stop using raw libuv API" (#156)"

This reverts commit c91876a.

(cherry picked from commit 69acc13)

* add missing locks during Timer callbacks

(cherry picked from commit 43a3484)

* fix Timer usage (#158)

(cherry picked from commit 62b497e)

* Workaround for missing isopen check in FDWatcher (#161)

(possible multithread race with this still needs to be fixed)

(cherry picked from commit 7f91b8a)

* Check for timer isopen correctly (#162)

(cherry picked from commit 4250b35)

* remove trailing whitespace

(cherry picked from commit d8c626b)

* Avoid infinite recursion in `timer_callback` (#164)

Fixes #163

(cherry picked from commit a55825b)

* should also look into headers for input_size (#167)

If no content length is set while uploading some contents, Curl defaults to use
chunked transfer encoding. In some cases we want to prevent that because the
server may not support chunked transfers.

With this change, the request method will also look at the headers while
determining the input size and if found call `set_upload_size` as usual. So to
switch off chunked transfers, one must also know and set the content length
header while invoking `download` or `request` methods.

(cherry picked from commit ab628ab)

* rename: singularize add_{upload,seek}_callback

These only add one callback so having them be plural is weird.

(cherry picked from commit 5bd0826)

* add support for setting a debug callback

(cherry picked from commit 55a0c39)

* end-to-end tests for #167

This adds end-to-end tests for the changes introduced in #167.

Verbose mode is switched off for these tests, but switching it on would show that not setting content-length headers results in chunked transfer encoding while setting it prevents that. Both tests should pass.

(cherry picked from commit 911368d)

* tests: use debug option to test for non/chunked uploads

This combines the functionality from the previous two commits to not
only trigger both chunked and non-chunked uploads, but also test for
that difference by capturing and inspecting the debug events.

(cherry picked from commit 4e0408a)

* bump patch

Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
Co-authored-by: Jakob Nybo Nissen <jakobnybonissen@gmail.com>
Co-authored-by: Yuto Horikawa <hyrodium@gmail.com>
Co-authored-by: Tim Holy <tim.holy@gmail.com>
Co-authored-by: Stefan Karpinski <stefan@karpinski.org>
Co-authored-by: Chris Foster <chris42f@gmail.com>
Co-authored-by: Benoît Legat <benoit.legat@gmail.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Co-authored-by: Tanmay Mohapatra <tanmaykm@gmail.com>
  • Loading branch information
10 people authored Mar 3, 2022
1 parent 26d79af commit b0f23d0
Show file tree
Hide file tree
Showing 12 changed files with 338 additions and 199 deletions.
5 changes: 0 additions & 5 deletions .ci/change-uuid.jl

This file was deleted.

28 changes: 28 additions & 0 deletions .ci/test_and_change_uuid.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
@static if Base.VERSION >= v"1.6"
using TOML
using Test
else
using Pkg: TOML
using Test
end

# To generate the new UUID, we simply modify the first character of the original UUID
const original_uuid = "f43a241f-c20a-4ad4-852c-f6b1247861c6"
const new_uuid = "e43a241f-c20a-4ad4-852c-f6b1247861c6"

# `@__DIR__` is the `.ci/` folder.
# Therefore, `dirname(@__DIR__)` is the repository root.
const project_filename = joinpath(dirname(@__DIR__), "Project.toml")

@testset "Test that the UUID is unchanged" begin
project_dict = TOML.parsefile(project_filename)
@test project_dict["uuid"] == original_uuid
end

write(
project_filename,
replace(
read(project_filename, String),
r"uuid = .*?\n" => "uuid = \"$(new_uuid)\"\n",
),
)
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ jobs:
${{ runner.os }}-test-${{ env.cache-name }}-
${{ runner.os }}-test-${{ matrix.os }}
${{ runner.os }}-
- run: julia --color=yes .ci/change-uuid.jl
- run: julia --color=yes .ci/test_and_change_uuid.jl
- uses: julia-actions/julia-buildpkg@v1
- uses: julia-actions/julia-runtest@v1
- uses: julia-actions/julia-processcoverage@v1
Expand Down
3 changes: 2 additions & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
name = "Downloads"
uuid = "f43a241f-c20a-4ad4-852c-f6b1247861c6"
authors = ["Stefan Karpinski <stefan@karpinski.org> and contributors"]
version = "1.5.2"
version = "1.5.3"

[deps]
ArgTools = "0dad84c5-d112-42e6-8d28-ef12dabb789f"
FileWatching = "7b1f6079-737a-58dc-b8bc-7a2ca5c1b5ee"
LibCURL = "b27032c2-a3e7-50c8-80cd-2d36dbcbfd21"
NetworkOptions = "ca575930-c2e3-43a9-ace4-1e988b2c1908"

Expand Down
24 changes: 17 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Downloads

[![Build Status](https://travis-ci.com/JuliaLang/Downloads.jl.svg?branch=master)](https://travis-ci.com/JuliaLang/Downloads.jl)
[![Build Status](https://github.com/JuliaLang/Downloads.jl/workflows/CI/badge.svg)](https://github.com/JuliaLang/Downloads.jl/actions)
[![Codecov](https://codecov.io/gh/JuliaLang/Downloads.jl/branch/master/graph/badge.svg)](https://codecov.io/gh/JuliaLang/Downloads.jl)

The `Downloads` package provides a single function, `download`, which provides
Expand All @@ -16,10 +16,10 @@ Julia 1.3 through 1.5 as well.

The public API of `Downloads` consists of two functions and three types:

- `download` download a file from a URL, erroring if it can't be downloaded
- `download` download a file from a URL, erroring if it can't be downloaded
- `request` — request a URL, returning a `Response` object indicating success
- `Response` a type capturing the status and other metadata about a request
- `RequestError` an error type thrown by `download` and `request` on error
- `Response` a type capturing the status and other metadata about a request
- `RequestError` an error type thrown by `download` and `request` on error
- `Downloader` — an object encapsulating shared resources for downloading

### download
Expand All @@ -31,6 +31,7 @@ download(url, [ output = tempfile() ];
[ timeout = <none>, ]
[ progress = <none>, ]
[ verbose = false, ]
[ debug = <none>, ]
[ downloader = <default>, ]
) -> output
```
Expand All @@ -41,6 +42,7 @@ download(url, [ output = tempfile() ];
- `timeout :: Real`
- `progress :: (total::Integer, now::Integer) --> Any`
- `verbose :: Bool`
- `debug :: (type, message) --> Any`
- `downloader :: Downloader`

Download a file from the given url, saving it to `output` or if not specified, a
Expand All @@ -67,12 +69,18 @@ which will be called whenever there are updates about the size and status of the
ongoing download. The callback must take two integer arguments: `total` and
`now` which are the total size of the download in bytes, and the number of bytes
which have been downloaded so far. Note that `total` starts out as zero and
remains zero until the server gives an indiation of the total size of the
remains zero until the server gives an indication of the total size of the
download (e.g. with a `Content-Length` header), which may never happen. So a
well-behaved progress callback should handle a total size of zero gracefully.

If the `verbose` optoin is set to true, `libcurl`, which is used to implement
the download functionality will print debugging information to `stderr`.
If the `verbose` option is set to true, `libcurl`, which is used to implement
the download functionality will print debugging information to `stderr`. If the
`debug` option is set to a function accepting two `String` arguments, then the
verbose option is ignored and instead the data that would have been printed to
`stderr` is passed to the `debug` callback with `type` and `message` arguments.
The `type` argument indicates what kind of event has occurred, and is one of:
`TEXT`, `HEADER IN`, `HEADER OUT`, `DATA IN`, `DATA OUT`, `SSL DATA IN` or `SSL
DATA OUT`. The `message` argument is the description of the debug event.

### request

Expand All @@ -85,6 +93,7 @@ request(url;
[ timeout = <none>, ]
[ progress = <none>, ]
[ verbose = false, ]
[ debug = <none>, ]
[ throw = true, ]
[ downloader = <default>, ]
) -> Union{Response, RequestError}
Expand All @@ -97,6 +106,7 @@ request(url;
- `timeout :: Real`
- `progress :: (dl_total, dl_now, ul_total, ul_now) --> Any`
- `verbose :: Bool`
- `debug :: (type, message) --> Any`
- `throw :: Bool`
- `downloader :: Downloader`

Expand Down
28 changes: 26 additions & 2 deletions src/Curl/Curl.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export
set_url,
set_method,
set_verbose,
set_debug,
set_body,
set_upload_size,
set_seeker,
Expand All @@ -26,15 +27,38 @@ export
remove_handle

using LibCURL
using LibCURL: curl_off_t
using LibCURL: curl_off_t, libcurl
# not exported: https://github.com/JuliaWeb/LibCURL.jl/issues/87

# constants that LibCURL should have but doesn't
const CURLE_PEER_FAILED_VERIFICATION = 60
const CURLSSLOPT_REVOKE_BEST_EFFORT = 1 << 3

# these are incorrectly defined on Windows by LibCURL:
if Sys.iswindows()
const curl_socket_t = Base.OS_HANDLE
const CURL_SOCKET_TIMEOUT = Base.INVALID_OS_HANDLE
else
const curl_socket_t = Cint
const CURL_SOCKET_TIMEOUT = -1
end

# definitions affected by incorrect curl_socket_t (copied verbatim):
function curl_multi_socket_action(multi_handle, s, ev_bitmask, running_handles)
ccall((:curl_multi_socket_action, libcurl), CURLMcode, (Ptr{CURLM}, curl_socket_t, Cint, Ptr{Cint}), multi_handle, s, ev_bitmask, running_handles)
end
function curl_multi_assign(multi_handle, sockfd, sockp)
ccall((:curl_multi_assign, libcurl), CURLMcode, (Ptr{CURLM}, curl_socket_t, Ptr{Cvoid}), multi_handle, sockfd, sockp)
end

# additional curl_multi_socket_action method
function curl_multi_socket_action(multi_handle, s, ev_bitmask)
curl_multi_socket_action(multi_handle, s, ev_bitmask, Ref{Cint}())
end

using FileWatching
using NetworkOptions
using Base: preserve_handle, unpreserve_handle
using Base: OS_HANDLE, preserve_handle, unpreserve_handle

include("utils.jl")

Expand Down
73 changes: 65 additions & 8 deletions src/Curl/Easy.jl
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ mutable struct Easy
res_hdrs :: Vector{String}
code :: CURLcode
errbuf :: Vector{UInt8}
debug :: Union{Function,Nothing}
end

const EMPTY_BYTE_VECTOR = UInt8[]
Expand All @@ -25,6 +26,7 @@ function Easy()
String[],
typemax(CURLcode),
zeros(UInt8, CURL_ERROR_SIZE),
nothing,
)
finalizer(done!, easy)
add_callbacks(easy)
Expand Down Expand Up @@ -106,6 +108,19 @@ function set_verbose(easy::Easy, verbose::Bool)
setopt(easy, CURLOPT_VERBOSE, verbose)
end

function set_debug(easy::Easy, debug::Function)
hasmethod(debug, Tuple{String,String}) ||
throw(ArgumentError("debug callback must take (::String, ::String)"))
easy.debug = debug
add_debug_callback(easy)
set_verbose(easy, true)
end

function set_debug(easy::Easy, debug::Nothing)
easy.debug = nothing
remove_debug_callback(easy)
end

function set_body(easy::Easy, body::Bool)
setopt(easy, CURLOPT_NOBODY, !body)
end
Expand All @@ -116,7 +131,7 @@ function set_upload_size(easy::Easy, size::Integer)
end

function set_seeker(seeker::Function, easy::Easy)
add_seek_callbacks(easy)
add_seek_callback(easy)
easy.seeker = seeker
end

Expand Down Expand Up @@ -159,7 +174,7 @@ function enable_progress(easy::Easy, on::Bool=true)
end

function enable_upload(easy::Easy)
add_upload_callbacks(easy::Easy)
add_upload_callback(easy::Easy)
setopt(easy, CURLOPT_UPLOAD, true)
end

Expand Down Expand Up @@ -229,6 +244,17 @@ function status_ok(proto::AbstractString, status::Integer)
end
status_ok(proto::Nothing, status::Integer) = false

function info_type(type::curl_infotype)
type == 0 ? "TEXT" :
type == 1 ? "HEADER IN" :
type == 2 ? "HEADER OUT" :
type == 3 ? "DATA IN" :
type == 4 ? "DATA OUT" :
type == 5 ? "SSL DATA IN" :
type == 6 ? "SSL DATA OUT" :
"UNKNOWN"
end

function get_effective_url(easy::Easy)
url_ref = Ref{Ptr{Cchar}}()
@check curl_easy_getinfo(easy.handle, CURLINFO_EFFECTIVE_URL, url_ref)
Expand All @@ -252,11 +278,13 @@ function get_response_info(easy::Easy)
for hdr in easy.res_hdrs
if contains(hdr, r"^\s*$")
# ignore
elseif (m = match(r"^(HTTP/\d+(?:.\d+)?\s+\d+\b.*?)\s*$", hdr)) !== nothing
message = m.captures[1]
elseif (m = match(r"^(HTTP/\d+(?:.\d+)?\s+\d+\b.*?)\s*$", hdr); m) !== nothing
message = m.captures[1]::SubString{String}
empty!(headers)
elseif (m = match(r"^(\S[^:]*?)\s*:\s*(.*?)\s*$", hdr)) !== nothing
push!(headers, lowercase(m.captures[1]) => m.captures[2])
elseif (m = match(r"^(\S[^:]*?)\s*:\s*(.*?)\s*$", hdr); m) !== nothing
key = lowercase(m.captures[1]::SubString{String})
val = m.captures[2]::SubString{String}
push!(headers, key => val)
else
@warn "malformed HTTP header" url status header=hdr
end
Expand Down Expand Up @@ -374,6 +402,19 @@ function progress_callback(
return 0
end

function debug_callback(
handle :: Ptr{Cvoid},
type :: curl_infotype,
data :: Ptr{Cchar},
size :: Csize_t,
easy_p :: Ptr{Cvoid},
)::Cint
easy = unsafe_pointer_to_objref(easy_p)::Easy
@assert easy.handle == handle
easy.debug(info_type(type), unsafe_string(data, size))
return 0
end

function add_callbacks(easy::Easy)
# pointer to easy object
easy_p = pointer_from_objref(easy)
Expand Down Expand Up @@ -402,7 +443,7 @@ function add_callbacks(easy::Easy)
setopt(easy, CURLOPT_XFERINFODATA, easy_p)
end

function add_upload_callbacks(easy::Easy)
function add_upload_callback(easy::Easy)
# pointer to easy object
easy_p = pointer_from_objref(easy)

Expand All @@ -413,7 +454,7 @@ function add_upload_callbacks(easy::Easy)
setopt(easy, CURLOPT_READDATA, easy_p)
end

function add_seek_callbacks(easy::Easy)
function add_seek_callback(easy::Easy)
# pointer to easy object
easy_p = pointer_from_objref(easy)

Expand All @@ -423,3 +464,19 @@ function add_seek_callbacks(easy::Easy)
setopt(easy, CURLOPT_SEEKFUNCTION, seek_cb)
setopt(easy, CURLOPT_SEEKDATA, easy_p)
end

function add_debug_callback(easy::Easy)
# pointer to easy object
easy_p = pointer_from_objref(easy)

# set debug callback
debug_cb = @cfunction(debug_callback,
Cint, (Ptr{Cvoid}, curl_infotype, Ptr{Cchar}, Csize_t, Ptr{Cvoid}))
setopt(easy, CURLOPT_DEBUGFUNCTION, debug_cb)
setopt(easy, CURLOPT_DEBUGDATA, easy_p)
end

function remove_debug_callback(easy::Easy)
setopt(easy, CURLOPT_DEBUGFUNCTION, C_NULL)
setopt(easy, CURLOPT_DEBUGDATA, C_NULL)
end
Loading

3 comments on commit b0f23d0

@DilumAluthge
Copy link
Member

Choose a reason for hiding this comment

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

@JuliaRegistrator register branch=release-1.5

cc: @ericphanson

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

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

Registration pull request created: JuliaRegistries/General/55852

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v1.5.3 -m "<description of version>" b0f23d0ed40cf7c5c5896f957d8163a68dfbc805
git push origin v1.5.3

@DilumAluthge
Copy link
Member

Choose a reason for hiding this comment

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

Also cc: @StefanKarpinski

Please sign in to comment.