diff --git a/components/engine/builder/dockerfile/copy.go b/components/engine/builder/dockerfile/copy.go index 223623ccd6..3de63d542e 100644 --- a/components/engine/builder/dockerfile/copy.go +++ b/components/engine/builder/dockerfile/copy.go @@ -3,6 +3,7 @@ package dockerfile import ( "fmt" "io" + "mime" "net/http" "net/url" "os" @@ -24,6 +25,8 @@ import ( "github.com/pkg/errors" ) +const unnamedFilename = "__unnamed__" + type pathCache interface { Load(key interface{}) (value interface{}, ok bool) Store(key, value interface{}) @@ -85,8 +88,7 @@ func (o *copier) createCopyInstruction(args []string, cmdName string) (copyInstr // Work in daemon-specific filepath semantics inst.dest = filepath.FromSlash(args[last]) - - infos, err := o.getCopyInfosForSourcePaths(args[0:last]) + infos, err := o.getCopyInfosForSourcePaths(args[0:last], inst.dest) if err != nil { return inst, errors.Wrapf(err, "%s failed", cmdName) } @@ -99,10 +101,11 @@ func (o *copier) createCopyInstruction(args []string, cmdName string) (copyInstr // getCopyInfosForSourcePaths iterates over the source files and calculate the info // needed to copy (e.g. hash value if cached) -func (o *copier) getCopyInfosForSourcePaths(sources []string) ([]copyInfo, error) { +// The dest is used in case source is URL (and ends with "/") +func (o *copier) getCopyInfosForSourcePaths(sources []string, dest string) ([]copyInfo, error) { var infos []copyInfo for _, orig := range sources { - subinfos, err := o.getCopyInfoForSourcePath(orig) + subinfos, err := o.getCopyInfoForSourcePath(orig, dest) if err != nil { return nil, err } @@ -115,7 +118,7 @@ func (o *copier) getCopyInfosForSourcePaths(sources []string) ([]copyInfo, error return infos, nil } -func (o *copier) getCopyInfoForSourcePath(orig string) ([]copyInfo, error) { +func (o *copier) getCopyInfoForSourcePath(orig, dest string) ([]copyInfo, error) { if !urlutil.IsURL(orig) { return o.calcCopyInfo(orig, true) } @@ -123,6 +126,14 @@ func (o *copier) getCopyInfoForSourcePath(orig string) ([]copyInfo, error) { if err != nil { return nil, err } + // If path == "" then we are unable to determine filename from src + // We have to make sure dest is available + if path == "" { + if strings.HasSuffix(dest, "/") { + return nil, errors.Errorf("cannot determine filename for source %s", orig) + } + path = unnamedFilename + } o.tmpPaths = append(o.tmpPaths, remote.Root()) hash, err := remote.Hash(path) @@ -301,22 +312,40 @@ func errOnSourceDownload(_ string) (builder.Source, string, error) { return nil, "", errors.New("source can't be a URL for COPY") } +func getFilenameForDownload(path string, resp *http.Response) string { + // Guess filename based on source + if path != "" && !strings.HasSuffix(path, "/") { + if filename := filepath.Base(filepath.FromSlash(path)); filename != "" { + return filename + } + } + + // Guess filename based on Content-Disposition + if contentDisposition := resp.Header.Get("Content-Disposition"); contentDisposition != "" { + if _, params, err := mime.ParseMediaType(contentDisposition); err == nil { + if params["filename"] != "" && !strings.HasSuffix(params["filename"], "/") { + if filename := filepath.Base(filepath.FromSlash(params["filename"])); filename != "" { + return filename + } + } + } + } + return "" +} + func downloadSource(output io.Writer, stdout io.Writer, srcURL string) (remote builder.Source, p string, err error) { u, err := url.Parse(srcURL) if err != nil { return } - filename := filepath.Base(filepath.FromSlash(u.Path)) // Ensure in platform semantics - if filename == "" { - err = errors.Errorf("cannot determine filename from url: %s", u) - return - } resp, err := remotecontext.GetWithStatusError(srcURL) if err != nil { return } + filename := getFilenameForDownload(u.Path, resp) + // Prepare file in a tmp dir tmpDir, err := ioutils.TempDir("", "docker-remote") if err != nil { @@ -327,7 +356,13 @@ func downloadSource(output io.Writer, stdout io.Writer, srcURL string) (remote b os.RemoveAll(tmpDir) } }() - tmpFileName := filepath.Join(tmpDir, filename) + // If filename is empty, the returned filename will be "" but + // the tmp filename will be created as "__unnamed__" + tmpFileName := filename + if filename == "" { + tmpFileName = unnamedFilename + } + tmpFileName = filepath.Join(tmpDir, tmpFileName) tmpFile, err := os.OpenFile(tmpFileName, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0600) if err != nil { return diff --git a/components/engine/builder/dockerfile/copy_test.go b/components/engine/builder/dockerfile/copy_test.go index ed384fd20b..86f4dcff30 100644 --- a/components/engine/builder/dockerfile/copy_test.go +++ b/components/engine/builder/dockerfile/copy_test.go @@ -1,6 +1,7 @@ package dockerfile import ( + "net/http" "testing" "github.com/gotestyourself/gotestyourself/fs" @@ -43,3 +44,103 @@ func TestIsExistingDirectory(t *testing.T) { assert.Equal(t, testcase.expected, result, testcase.doc) } } + +func TestGetFilenameForDownload(t *testing.T) { + var testcases = []struct { + path string + disposition string + expected string + }{ + { + path: "http://www.example.com/", + expected: "", + }, + { + path: "http://www.example.com/xyz", + expected: "xyz", + }, + { + path: "http://www.example.com/xyz.html", + expected: "xyz.html", + }, + { + path: "http://www.example.com/xyz/", + expected: "", + }, + { + path: "http://www.example.com/xyz/uvw", + expected: "uvw", + }, + { + path: "http://www.example.com/xyz/uvw.html", + expected: "uvw.html", + }, + { + path: "http://www.example.com/xyz/uvw/", + expected: "", + }, + { + path: "/", + expected: "", + }, + { + path: "/xyz", + expected: "xyz", + }, + { + path: "/xyz.html", + expected: "xyz.html", + }, + { + path: "/xyz/", + expected: "", + }, + { + path: "/xyz/", + disposition: "attachment; filename=xyz.html", + expected: "xyz.html", + }, + { + disposition: "", + expected: "", + }, + { + disposition: "attachment; filename=xyz", + expected: "xyz", + }, + { + disposition: "attachment; filename=xyz.html", + expected: "xyz.html", + }, + { + disposition: "attachment; filename=\"xyz\"", + expected: "xyz", + }, + { + disposition: "attachment; filename=\"xyz.html\"", + expected: "xyz.html", + }, + { + disposition: "attachment; filename=\"/xyz.html\"", + expected: "xyz.html", + }, + { + disposition: "attachment; filename=\"/xyz/uvw\"", + expected: "uvw", + }, + { + disposition: "attachment; filename=\"Naïve file.txt\"", + expected: "Naïve file.txt", + }, + } + for _, testcase := range testcases { + resp := http.Response{ + Header: make(map[string][]string), + } + if testcase.disposition != "" { + resp.Header.Add("Content-Disposition", testcase.disposition) + } + filename := getFilenameForDownload(testcase.path, &resp) + assert.Equal(t, testcase.expected, filename) + } +}