Use Content-Disposition for 3xx requests for filename detection (#761)

Use Content-Disposition for 3xx requests for filename detection in HTTP.download, fixes #760.

Co-authored-by: Lyndon White <lyndon.white@invenialabs.co.uk>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>

src/download.jl

@@ -16,9 +16,8 @@ function safer_joinpath(basepart, parts...)
     joinpath(basepart, parts...)
 end
 
-function try_get_filename_from_headers(resp)
-    content_disp = header(resp, "Content-Disposition")
-    if content_disp != ""
+function try_get_filename_from_headers(hdrs)
+    for content_disp in hdrs
         # extract out of Content-Disposition line
         # rough version of what is needed in https://github.com/JuliaWeb/HTTP.jl/issues/179
         filename_part = match(r"filename\s*=\s*(.*)", content_disp)
@@ -55,16 +54,16 @@ function try_get_filename_from_request(req)
 end
 
 
-determine_file(::Nothing, resp) = determine_file(tempdir(), resp)
+determine_file(::Nothing, resp, hdrs) = determine_file(tempdir(), resp, hdrs)
 # ^ We want to the filename if possible because extension is useful for FileIO.jl
 
-function determine_file(path, resp)
+function determine_file(path, resp, hdrs)
     # get the name
     name = if isdir(path)
         # we have been given a path to a directory
         # got to to workout what file to put there
         filename = something(
-                        try_get_filename_from_headers(resp),
+                        try_get_filename_from_headers(hdrs),
                         try_get_filename_from_request(resp.request),
                         basename(tempname())  # fallback, basically a random string
                     )
@@ -107,11 +106,15 @@ function download(url::AbstractString, local_path=nothing, headers=Header[]; upd
 
     @debug 1 "downloading $url"
     local file
+    hdrs = String[]
     HTTP.open("GET", url, headers; kw...) do stream
         resp = startread(stream)
+        # Store intermediate header from redirects to use for filename detection
+        content_disp = header(resp, "Content-Disposition")
+        !isempty(content_disp) && push!(hdrs, content_disp)
         eof(stream) && return  # don't do anything for streams we can't read (yet)
 
-        file = determine_file(local_path, resp)
+        file = determine_file(local_path, resp, hdrs)
         total_bytes = parse(Float64, header(resp, "Content-Length", "NaN"))
         downloaded_bytes = 0
         start_time = now()

test/download.jl

@@ -40,6 +40,18 @@ using HTTP
             @test isfile(escaped_content_disposition_fn)
             @test basename(escaped_content_disposition_fn) == "\"quoting\" tested.html"
         end
+
+        # HTTP#760
+        # This has a redirect, where the original Response has the Content-Disposition
+        # but the redirected one doesn't
+        # Neither https://httpbingo.julialang.org/ nor http://test.greenbytes.de/tech/tc2231/
+        # has a test-case for this. See: https://github.com/postmanlabs/httpbin/issues/652
+        # This test might stop validating the code-path if FigShare changes how they do
+        # redirects (which they have done before, causing issue HTTP#760, in the first place)
+        redirected_content_disposition_fn = HTTP.download(
+            "https://ndownloader.figshare.com/files/6294558")
+        @test isfile(redirected_content_disposition_fn)
+        @test basename(redirected_content_disposition_fn) == "rsta20150293_si_001.xlsx"
     end
 
     @testset "Provided Filename" begin