Content-disposition: filename safe, filename* unsafe => use filename (#271)

This is very much an edge case, but it was bugging me. If the filename
attribute is safe to use but the filename* attribute is unsafe then we
use the filename instead of whatever the fallback would have been.

Author: StefanKarpinski

src/Downloads.jl

@@ -265,10 +265,7 @@ function download(
     if output === nothing
         try_rename = true
         # guess file name from URL (might not be final name)
-        name = url_filename(url)
-        if !is_safe_filename(name)
-            name = DEFAULT_FILENAME
-        end
+        name = something(url_filename(url), DEFAULT_FILENAME)
         output = joinpath(mktempdir(), name)
     end
     local response # capture outside closure
@@ -290,7 +287,7 @@ function download(
     # fix file suffix based on headers & redirected URL
     if try_rename && ispath(path)
         name = get_filename(response)
-        if is_safe_filename(name)
+        if name !== nothing
             path′ = joinpath(dirname(path), name)
             @assert dirname(path) == dirname(path′)
             if path != path′

src/filenames.jl

@@ -35,8 +35,11 @@ end
 
 function url_filename(url::AbstractString)
     m = match(r"^[a-z][a-z+._-]*://[^#?]*/([^/#?]+)(?:[#?]|$)"i, url)
-    m === nothing && return
-    url_unescape(m[1])
+    if m !== nothing
+        name = url_unescape(m[1])
+        is_safe_filename(name) && return name
+    end
+    return nothing
 end
 
 let # build some complex regular expressions
@@ -109,11 +112,10 @@ function get_filename(response::Response)
             end
         end
     end
-    filename⁺ !== nothing && return filename⁺
-    filename !== nothing && return filename
-    # no usable content disposition header
-    # extract from URL after redirects
-    return url_filename(response.url)
+    filename⁺ !== nothing && is_safe_filename(filename⁺) && return filename⁺
+    filename !== nothing && is_safe_filename(filename) && return filename
+    # no usable content disposition header, extract from URL after redirects
+    return url_filename(response.url) # calls is_safe_filename
 end
 
 # Special names on Windows: CON PRN AUX NUL COM1-9 LPT1-9

test/runtests.jl

@@ -227,7 +227,11 @@ include("setup.jl")
             end
             for name in file_names,
                 url in urls_with_filename(name)
-                @test name === Downloads.url_filename(url)
+                if Sys.iswindows() && '"' in name
+                    @test nothing === Downloads.url_filename(url)
+                else
+                    @test name === Downloads.url_filename(url)
+                end
             end
             # URLs that we shouldn't get a file name from
             for url in [
@@ -316,6 +320,10 @@ include("setup.jl")
                 url = content_disposition_url(:utf8 => name, :latin1 => name⁺)
                 @test name⁺ == splitdir(download(url))[2]
             end
+            let name = "valid.txt", name⁺ = "invalid\0.txt"
+                url = content_disposition_url(:ascii => name, :utf8 => name⁺)
+                @test name == splitdir(download(url))[2]
+            end
         end
         @testset "invalid content disposition" begin
             # invalid content disposition header syntax