From 83a25781f2cf1bf57aae850e5385217b0443cee9 Mon Sep 17 00:00:00 2001 From: Ali Rizvi-Santiago Date: Sun, 1 Nov 2015 21:46:14 -0600 Subject: [PATCH 1/6] Added proper support for downloading via a Windows UNC path or a relative uri. Added proper support for validating a downloadableURL containing a UNC or relative uri. Removed the workaround for an earlier Go issue that had remained dormant in common/download.go (issue #5927). When building a .vmx file via the vmware-iso builder, transform the path to the correct os-formatted one (using filepath.FromSlash). --- builder/vmware/iso/step_create_vmx.go | 5 ++ common/config.go | 102 ++++++++++++++++---------- common/download.go | 50 ++++++++++++- packer/core.go | 2 +- 4 files changed, 117 insertions(+), 42 deletions(-) diff --git a/builder/vmware/iso/step_create_vmx.go b/builder/vmware/iso/step_create_vmx.go index 2727218936b..7545c4eb4d6 100755 --- a/builder/vmware/iso/step_create_vmx.go +++ b/builder/vmware/iso/step_create_vmx.go @@ -43,6 +43,11 @@ func (s *stepCreateVMX) Run(state multistep.StateBag) multistep.StepAction { isoPath := state.Get("iso_path").(string) ui := state.Get("ui").(packer.Ui) + // Convert the iso_path into a path relative to the .vmx file if possible + if relativeIsoPath,err := filepath.Rel(config.VMXTemplatePath, filepath.FromSlash(isoPath)); err == nil { + isoPath = relativeIsoPath + } + ui.Say("Building and writing VMX file") vmxTemplate := DefaultVMXTemplate diff --git a/common/config.go b/common/config.go index e6c6477b5ef..c998a49caf5 100644 --- a/common/config.go +++ b/common/config.go @@ -42,84 +42,110 @@ func DownloadableURL(original string) (string, error) { // we're dealing with a drive letter and thus a file path. idx := strings.Index(original, ":") if idx == 1 { - original = "file:///" + original + original = "file://" + filepath.ToSlash(original) } } - url, err := url.Parse(original) + // XXX: The validation here is later re-parsed in common/download.go and + // thus any modifications here must remain consistent over there too. + uri, err := url.Parse(original) if err != nil { return "", err } - if url.Scheme == "" { - url.Scheme = "file" + if uri.Scheme == "" { + uri.Scheme = "file" } - if url.Scheme == "file" { - // Windows file handling is all sorts of tricky... + const UNCPrefix = string(os.PathSeparator)+string(os.PathSeparator) + if uri.Scheme == "file" { + var ospath string // os-formatted pathname if runtime.GOOS == "windows" { - // If the path is using Windows-style slashes, URL parses - // it into the host field. - if url.Path == "" && strings.Contains(url.Host, `\`) { - url.Path = url.Host - url.Host = "" - } - - // For Windows absolute file paths, remove leading / prior to processing - // since net/url turns "C:/" into "/C:/" - if len(url.Path) > 0 && url.Path[0] == '/' { - url.Path = url.Path[1:len(url.Path)] + // Move any extra path components that were mis-parsed into the Host + // field back into the uri.Path field + if len(uri.Host) >= len(UNCPrefix) && uri.Host[:len(UNCPrefix)] == UNCPrefix { + idx := strings.Index(uri.Host[len(UNCPrefix):], string(os.PathSeparator)) + if idx > -1 { + uri.Path = filepath.ToSlash(uri.Host[idx+len(UNCPrefix):]) + uri.Path + uri.Host = uri.Host[:idx+len(UNCPrefix)] + } } + // Now all we need to do to convert the uri to a platform-specific path + // is to trade it's slashes for some os.PathSeparator ones. + ospath = uri.Host + filepath.FromSlash(uri.Path) + + } else { + // Since we're already using sane paths on a sane platform, anything in + // uri.Host can be assumed that the user is describing a relative uri. + // This means that if we concatenate it with uri.Path, the filepath + // transform will still open the file correctly. + // i.e. file://localdirectory/filename -> localdirectory/filename + ospath = uri.Host + uri.Path } // Only do the filepath transformations if the file appears - // to actually exist. - if _, err := os.Stat(url.Path); err == nil { - url.Path, err = filepath.Abs(url.Path) + // to actually exist. We don't do it on windows, because EvalSymlinks + // won't understand how to handle UNC paths and other Windows-specific minutae. + if _, err := os.Stat(ospath); err == nil && runtime.GOOS != "windows" { + ospath, err = filepath.Abs(ospath) if err != nil { return "", err } - url.Path, err = filepath.EvalSymlinks(url.Path) + ospath, err = filepath.EvalSymlinks(ospath) if err != nil { return "", err } - url.Path = filepath.Clean(url.Path) + ospath = filepath.Clean(ospath) } + // now that ospath was normalized and such.. if runtime.GOOS == "windows" { - // Also replace all backslashes with forwardslashes since Windows - // users are likely to do this but the URL should actually only - // contain forward slashes. - url.Path = strings.Replace(url.Path, `\`, `/`, -1) + uri.Host = "" + // Check to see if our ospath is unc-prefixed, and if it is then split + // the UNC host into uri.Host, leaving the rest in ospath. + // This way, our UNC-uri is protected from injury in the call to uri.String() + if len(ospath) >= len(UNCPrefix) && ospath[:len(UNCPrefix)] == UNCPrefix { + idx := strings.Index(ospath[len(UNCPrefix):], string(os.PathSeparator)) + if idx > -1 { + uri.Host = ospath[:len(UNCPrefix)+idx] + ospath = ospath[len(UNCPrefix)+idx:] + } + } + // Restore the uri by re-transforming our os-formatted path + uri.Path = filepath.ToSlash(ospath) + } else { + uri.Host = "" + uri.Path = filepath.ToSlash(ospath) } } // Make sure it is lowercased - url.Scheme = strings.ToLower(url.Scheme) - - // This is to work around issue #5927. This can safely be removed once - // we distribute with a version of Go that fixes that bug. - // - // See: https://code.google.com/p/go/issues/detail?id=5927 - if url.Path != "" && url.Path[0] != '/' { - url.Path = "/" + url.Path - } + uri.Scheme = strings.ToLower(uri.Scheme) // Verify that the scheme is something we support in our common downloader. supported := []string{"file", "http", "https"} found := false for _, s := range supported { - if url.Scheme == s { + if uri.Scheme == s { found = true break } } if !found { - return "", fmt.Errorf("Unsupported URL scheme: %s", url.Scheme) + return "", fmt.Errorf("Unsupported URL scheme: %s", uri.Scheme) + } + + // explicit check to see if we need to manually replace the uri host with a UNC one + if runtime.GOOS == "windows" && uri.Scheme == "file" { + if len(uri.Host) >= len(UNCPrefix) && uri.Host[:len(UNCPrefix)] == UNCPrefix { + escapedHost := url.QueryEscape(uri.Host) + return strings.Replace(uri.String(), escapedHost, uri.Host, 1), nil + } } - return url.String(), nil + // otherwise, we can trust the url handler + return uri.String(), nil } diff --git a/common/download.go b/common/download.go index e4b4dc2e0f8..7be78217e49 100644 --- a/common/download.go +++ b/common/download.go @@ -16,6 +16,9 @@ import ( "net/url" "os" "runtime" + "path" + "path/filepath" + "strings" ) // DownloadConfig is the configuration given to instantiate a new @@ -122,10 +125,51 @@ func (d *DownloadClient) Get() (string, error) { finalPath = url.Path log.Printf("[DEBUG] Using local file: %s", finalPath) - // Remove forward slash on absolute Windows file URLs before processing - if runtime.GOOS == "windows" && len(finalPath) > 0 && finalPath[0] == '/' { - finalPath = finalPath[1:len(finalPath)] + // FIXME: + // cwd should point to the path relative to client.json, but + // since this isn't exposed to us anywhere, we use os.Getwd() + // to figure it out. + cwd,err := os.Getwd() + if err != nil { + return "", fmt.Errorf("Unable to get working directory") } + + // convert the actual file uri to a windowsy path + // (this logic must correspond to the same logic in common/config.go) + if runtime.GOOS == "windows" { + const UNCPrefix = string(os.PathSeparator)+string(os.PathSeparator) + + // move any extra path components that were parsed into Host due + // to UNC into the url.Path field so that it's PathSeparators get + // normalized + if len(url.Host) >= len(UNCPrefix) && url.Host[:len(UNCPrefix)] == UNCPrefix { + idx := strings.Index(url.Host[len(UNCPrefix):], string(os.PathSeparator)) + if idx > -1 { + url.Path = filepath.ToSlash(url.Host[idx+len(UNCPrefix):]) + url.Path + url.Host = url.Host[:idx+len(UNCPrefix)] + } + } + + // clean up backward-slashes since they only matter when part of a unc path + urlPath := filepath.ToSlash(url.Path) + + // semi-absolute path (current drive letter) -- file:///absolute/path + if url.Host == "" && len(urlPath) > 0 && urlPath[0] == '/' { + finalPath = path.Join(filepath.VolumeName(cwd), urlPath) + + // relative path -- file://./relative/path + // file://relative/path + } else if url.Host == "" || (len(url.Host) > 0 && url.Host[0] == '.') { + finalPath = path.Join(filepath.ToSlash(cwd), urlPath) + + // absolute path + } else { + // UNC -- file://\\host/share/whatever + // drive -- file://c:/absolute/path + finalPath = path.Join(url.Host, urlPath) + } + } + // Keep track of the source so we can make sure not to delete this later sourcePath = finalPath } else { diff --git a/packer/core.go b/packer/core.go index 496fce6bd3c..08916494c95 100644 --- a/packer/core.go +++ b/packer/core.go @@ -67,7 +67,7 @@ func NewCore(c *CoreConfig) (*Core, error) { return nil, err } - // Go through and interpolate all the build names. We shuld be able + // Go through and interpolate all the build names. We should be able // to do this at this point with the variables. result.builds = make(map[string]*template.Builder) for _, b := range c.Template.Builders { From 4d67da9dbefeb436ea88e400d4ab6c9b519af920 Mon Sep 17 00:00:00 2001 From: Ali Rizvi-Santiago Date: Mon, 18 Jan 2016 17:36:47 -0600 Subject: [PATCH 2/6] Added some testcases for the various file uri transforms to download_test.go Moved some of the code for normalizing a Windows file uri to a regular path into it's own function NormalizeWindowsURL --- common/download.go | 88 ++++++++++++++++++---------------- common/download_test.go | 103 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 41 deletions(-) diff --git a/common/download.go b/common/download.go index 7be78217e49..a76e3cc9690 100644 --- a/common/download.go +++ b/common/download.go @@ -101,6 +101,44 @@ func (d *DownloadClient) Cancel() { // TODO(mitchellh): Implement } +// Take a uri and convert it to a path that makes sense on the Windows platform +func NormalizeWindowsURL(basepath string, url url.URL) string { + // This logic must correspond to the same logic in the NormalizeWindowsURL + // function found in common/config.go since that function _also_ checks that + // the url actually exists in file form. + + const UNCPrefix = string(os.PathSeparator)+string(os.PathSeparator) + + // move any extra path components that were parsed into Host due + // to UNC into the url.Path field so that it's PathSeparators get + // normalized + if len(url.Host) >= len(UNCPrefix) && url.Host[:len(UNCPrefix)] == UNCPrefix { + idx := strings.Index(url.Host[len(UNCPrefix):], string(os.PathSeparator)) + if idx > -1 { + url.Path = filepath.ToSlash(url.Host[idx+len(UNCPrefix):]) + url.Path + url.Host = url.Host[:idx+len(UNCPrefix)] + } + } + + // clean up backward-slashes since they only matter when part of a unc path + urlPath := filepath.ToSlash(url.Path) + + // semi-absolute path (current drive letter) -- file:///absolute/path + if url.Host == "" && len(urlPath) > 0 && urlPath[0] == '/' { + return path.Join(filepath.VolumeName(basepath), urlPath) + + // relative path -- file://./relative/path + // file://relative/path + } else if url.Host == "" || (len(url.Host) > 0 && url.Host[0] == '.') { + return path.Join(filepath.ToSlash(basepath), urlPath) + } + + // absolute path + // UNC -- file://\\host/share/whatever + // drive -- file://c:/absolute/path + return path.Join(url.Host, urlPath) +} + func (d *DownloadClient) Get() (string, error) { // If we already have the file and it matches, then just return the target path. if verify, _ := d.VerifyChecksum(d.config.TargetPath); verify { @@ -125,49 +163,17 @@ func (d *DownloadClient) Get() (string, error) { finalPath = url.Path log.Printf("[DEBUG] Using local file: %s", finalPath) - // FIXME: - // cwd should point to the path relative to client.json, but - // since this isn't exposed to us anywhere, we use os.Getwd() - // to figure it out. - cwd,err := os.Getwd() - if err != nil { - return "", fmt.Errorf("Unable to get working directory") - } - - // convert the actual file uri to a windowsy path - // (this logic must correspond to the same logic in common/config.go) + // transform the actual file uri to a windowsy path if we're being windowsy. if runtime.GOOS == "windows" { - const UNCPrefix = string(os.PathSeparator)+string(os.PathSeparator) - - // move any extra path components that were parsed into Host due - // to UNC into the url.Path field so that it's PathSeparators get - // normalized - if len(url.Host) >= len(UNCPrefix) && url.Host[:len(UNCPrefix)] == UNCPrefix { - idx := strings.Index(url.Host[len(UNCPrefix):], string(os.PathSeparator)) - if idx > -1 { - url.Path = filepath.ToSlash(url.Host[idx+len(UNCPrefix):]) + url.Path - url.Host = url.Host[:idx+len(UNCPrefix)] - } - } - - // clean up backward-slashes since they only matter when part of a unc path - urlPath := filepath.ToSlash(url.Path) - - // semi-absolute path (current drive letter) -- file:///absolute/path - if url.Host == "" && len(urlPath) > 0 && urlPath[0] == '/' { - finalPath = path.Join(filepath.VolumeName(cwd), urlPath) - - // relative path -- file://./relative/path - // file://relative/path - } else if url.Host == "" || (len(url.Host) > 0 && url.Host[0] == '.') { - finalPath = path.Join(filepath.ToSlash(cwd), urlPath) - - // absolute path - } else { - // UNC -- file://\\host/share/whatever - // drive -- file://c:/absolute/path - finalPath = path.Join(url.Host, urlPath) + // FIXME: cwd should point to a path relative to the TEMPLATE path, + // but since this isn't exposed to us anywhere, we use os.Getwd() + // and assume the user ran packer in the same directory that + // any relative files are located at. + cwd,err := os.Getwd() + if err != nil { + return "", fmt.Errorf("Unable to get working directory") } + finalPath = NormalizeWindowsURL(cwd, *url) } // Keep track of the source so we can make sure not to delete this later diff --git a/common/download_test.go b/common/download_test.go index 51f6f270c82..59235b7013e 100644 --- a/common/download_test.go +++ b/common/download_test.go @@ -8,6 +8,8 @@ import ( "net/http" "net/http/httptest" "os" + "strings" + "path/filepath" "testing" ) @@ -374,5 +376,106 @@ func TestDownloadFileUrl(t *testing.T) { if _, err = os.Stat(sourcePath); err != nil { t.Errorf("Could not stat source file: %s", sourcePath) } +} +// SimulateFileUriDownload is a simple utility function that converts a uri +// into a testable file path whilst ignoring a correct checksum match, stripping +// UNC path info, and then calling stat to ensure the correct file exists. +// (used by TestFileUriTransforms) +func SimulateFileUriDownload(t *testing.T, uri string) (string,error) { + // source_path is a file path and source is a network path + source := fmt.Sprintf(uri) + t.Logf("Trying to download %s", source) + + config := &DownloadConfig{ + Url: source, + // This should be wrong. We want to make sure we don't delete + Checksum: []byte("nope"), + Hash: HashForType("sha256"), + CopyFile: false, + } + + // go go go + client := NewDownloadClient(config) + path, err := client.Get() + + // ignore any non-important checksum errors if it's not a unc path + if !strings.HasPrefix(path, "\\\\") && err.Error() != "checksums didn't match expected: 6e6f7065" { + t.Fatalf("Unexpected failure; expected checksum not to match") + } + + // if it's a unc path, then remove the host and share name so we don't have + // to force the user to enable ADMIN$ and Windows File Sharing + if strings.HasPrefix(path, "\\\\") { + res := strings.SplitN(path, "/", 3) + path = "/" + res[2] + } + + if _, err = os.Stat(path); err != nil { + t.Errorf("Could not stat source file: %s", path) + } + return path,err +} + +// TestFileUriTransforms tests the case where we use a local file uri +// for iso_url. There's a few different formats that a file uri can exist as +// and so we try to test the most useful and common ones. +func TestFileUriTransforms(t *testing.T) { + const testpath = /* have your */ "test-fixtures/fileurl/cake" /* and eat it too */ + const host = "localhost" + + var cwd string + var volume string + var share string + + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("Unable to detect working directory: %s", err) + return + } + cwd = filepath.ToSlash(cwd) + volume = filepath.VolumeName(cwd) + share = volume + if share[len(share)-1] == ':' { + share = share[:len(share)-1] + "$" + } + cwd = cwd[len(volume):] + + t.Logf("TestFileUriTransforms : Running with cwd : '%s'", cwd) + t.Logf("TestFileUriTransforms : Running with volume : '%s'", volume) + + // ./relative/path -> ./relative/path + // /absolute/path -> /absolute/path + // c:/windows/absolute -> c:/windows/absolute + // \\host/sharename/file -> \\host/sharename/file + testcases := []string{ + "./%s", + cwd + "/%s", + volume + cwd + "/%s", + "\\\\" + host + "/" + share + "/" + cwd[1:] + "/%s", + } + + // all regular slashed testcases + for _,testcase := range testcases { + uri := "file://" + fmt.Sprintf(testcase, testpath) + t.Logf("TestFileUriTransforms : Trying Uri '%s'", uri) + res,err := SimulateFileUriDownload(t, uri) + if err != nil { + t.Errorf("Unable to transform uri '%s' into a path : %v", uri, err) + } + t.Errorf("TestFileUriTransforms : Result Path '%s'", res) + } + + // ...and finally the oddball windows native path + // \\host\sharename\file -> \\host/sharename/file + testpath_native := filepath.FromSlash(testpath) + testcase_native := "\\\\" + host + "\\" + share + "\\" + filepath.FromSlash(cwd[1:]) + "\\%s" + uri := "file://" + fmt.Sprintf(testcase_native, testpath_native) + t.Logf("TestFileUriTransforms : Trying Uri '%s'", uri) + res,err := SimulateFileUriDownload(t, uri) + if err != nil { + t.Errorf("Unable to transform uri '%s' into a path", uri) + return + } + t.Errorf("TestFileUriTransforms : Result Path '%s'", res) } From 14fa767b0e1a968694f39d85f265a536b1d6ec9f Mon Sep 17 00:00:00 2001 From: Ali Rizvi-Santiago Date: Tue, 5 Apr 2016 13:10:53 -0500 Subject: [PATCH 3/6] Added a missing argument in the builder test for vmware/iso/driver_esx5 --- builder/vmware/iso/driver_esx5_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/builder/vmware/iso/driver_esx5_test.go b/builder/vmware/iso/driver_esx5_test.go index 65add071240..678df53f3cc 100644 --- a/builder/vmware/iso/driver_esx5_test.go +++ b/builder/vmware/iso/driver_esx5_test.go @@ -2,6 +2,7 @@ package iso import ( "fmt" + "github.com/mitchellh/multistep" vmwcommon "github.com/mitchellh/packer/builder/vmware/common" "net" "testing" @@ -22,14 +23,17 @@ func TestESX5Driver_implRemoteDriver(t *testing.T) { func TestESX5Driver_HostIP(t *testing.T) { expected_host := "127.0.0.1" + state := new(multistep.BasicStateBag) + //create mock SSH server listen, _ := net.Listen("tcp", fmt.Sprintf("%s:0", expected_host)) port := listen.Addr().(*net.TCPAddr).Port defer listen.Close() driver := ESX5Driver{Host: "localhost", Port: uint(port)} + state.Put("driver", driver) - if host, _ := driver.HostIP(); host != expected_host { + if host, _ := driver.HostIP(state); host != expected_host { t.Error(fmt.Sprintf("Expected string, %s but got %s", expected_host, host)) } } From 4a573f1ff48ea699c3eb32745def26406514bba9 Mon Sep 17 00:00:00 2001 From: Ali Rizvi-Santiago Date: Tue, 5 Apr 2016 13:11:30 -0500 Subject: [PATCH 4/6] Added the file, ftp, and smb downloaders to common/download.go --- common/download.go | 353 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 313 insertions(+), 40 deletions(-) diff --git a/common/download.go b/common/download.go index a76e3cc9690..d8d9d9e537d 100644 --- a/common/download.go +++ b/common/download.go @@ -12,7 +12,6 @@ import ( "hash" "io" "log" - "net/http" "net/url" "os" "runtime" @@ -21,6 +20,13 @@ import ( "strings" ) +import ( + "net/http" + "github.com/jlaffeye/ftp" + "bufio" +) + + // DownloadConfig is the configuration given to instantiate a new // download instance. Once a configuration is used to instantiate // a download client, it must not be modified. @@ -78,10 +84,15 @@ func HashForType(t string) hash.Hash { // NewDownloadClient returns a new DownloadClient for the given // configuration. func NewDownloadClient(c *DownloadConfig) *DownloadClient { + const mtu = 1500 /* ethernet */ - 20 /* ipv4 */ - 20 /* tcp */ + if c.DownloaderMap == nil { c.DownloaderMap = map[string]Downloader{ + "file": &FileDownloader{bufferSize: nil}, + "ftp": &FTPDownloader{userInfo: url.Userinfo{username:"anonymous", password: "anonymous@"}, mtu: mtu}, "http": &HTTPDownloader{userAgent: c.UserAgent}, "https": &HTTPDownloader{userAgent: c.UserAgent}, + "smb": &SMBDownloader{bufferSize: nil} } } @@ -101,44 +112,6 @@ func (d *DownloadClient) Cancel() { // TODO(mitchellh): Implement } -// Take a uri and convert it to a path that makes sense on the Windows platform -func NormalizeWindowsURL(basepath string, url url.URL) string { - // This logic must correspond to the same logic in the NormalizeWindowsURL - // function found in common/config.go since that function _also_ checks that - // the url actually exists in file form. - - const UNCPrefix = string(os.PathSeparator)+string(os.PathSeparator) - - // move any extra path components that were parsed into Host due - // to UNC into the url.Path field so that it's PathSeparators get - // normalized - if len(url.Host) >= len(UNCPrefix) && url.Host[:len(UNCPrefix)] == UNCPrefix { - idx := strings.Index(url.Host[len(UNCPrefix):], string(os.PathSeparator)) - if idx > -1 { - url.Path = filepath.ToSlash(url.Host[idx+len(UNCPrefix):]) + url.Path - url.Host = url.Host[:idx+len(UNCPrefix)] - } - } - - // clean up backward-slashes since they only matter when part of a unc path - urlPath := filepath.ToSlash(url.Path) - - // semi-absolute path (current drive letter) -- file:///absolute/path - if url.Host == "" && len(urlPath) > 0 && urlPath[0] == '/' { - return path.Join(filepath.VolumeName(basepath), urlPath) - - // relative path -- file://./relative/path - // file://relative/path - } else if url.Host == "" || (len(url.Host) > 0 && url.Host[0] == '.') { - return path.Join(filepath.ToSlash(basepath), urlPath) - } - - // absolute path - // UNC -- file://\\host/share/whatever - // drive -- file://c:/absolute/path - return path.Join(url.Host, urlPath) -} - func (d *DownloadClient) Get() (string, error) { // If we already have the file and it matches, then just return the target path. if verify, _ := d.VerifyChecksum(d.config.TargetPath); verify { @@ -153,6 +126,11 @@ func (d *DownloadClient) Get() (string, error) { log.Printf("Parsed URL: %#v", url) + /* FIXME: + handle the special case of d.config.CopyFile which returns the path + in an os-specific format. + */ + // Files when we don't copy the file are special cased. var f *os.File var finalPath string @@ -260,7 +238,7 @@ func (*HTTPDownloader) Cancel() { } func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { - log.Printf("Starting download: %s", src.String()) + log.Printf("Starting download over HTTP: %s", src.String()) // Seek to the beginning by default if _, err := dst.Seek(0, 0); err != nil { @@ -339,3 +317,298 @@ func (d *HTTPDownloader) Progress() uint { func (d *HTTPDownloader) Total() uint { return d.total } + +// FTPDownloader is an implementation of Downloader that downloads +// files over FTP. +type FTPDownloader struct { + userInfo url.UserInfo + mtu uint + + active bool + progress uint + total uint +} + +func (*FTPDownloader) Cancel() { + d.active = false +} + +func (d *FTPDownloader) Download(dst *os.File, src *url.URL) error { + var userinfo *url.Userinfo + + userinfo = d.userInfo + d.active = false + + // check the uri is correct + uri, err := url.Parse(src) + if err != nil { return err } + + if uri.Scheme != "ftp" { + return fmt.Errorf("Unexpected uri scheme: %s", uri.Scheme) + } + + // connect to ftp server + var cli *ftp.ServerConn + + log.Printf("Starting download over FTP: %s : %s\n", uri.Host, Uri.Path) + cli,err := ftp.Dial(uri.Host) + if err != nil { return nil } + defer cli.Close() + + // handle authentication + if uri.User != nil { userinfo = uri.User } + + log.Printf("Authenticating to FTP server: %s : %s\n", uri.User.username, uri.User.password) + err = cli.Login(userinfo.username, userinfo.password) + if err != nil { return err } + + // locate specified path + path := path.Dir(uri.Path) + + log.Printf("Changing to FTP directory : %s\n", path) + err = cli.ChangeDir(path) + if err != nil { return nil } + + curpath,err := cli.CurrentDir() + if err != nil { return err } + log.Printf("Current FTP directory : %s\n", curpath) + + // collect stats about the specified file + var name string + var entry *ftp.Entry + + _,name = path.Split(uri.Path) + entry = nil + + entries,err := cli.List(curpath) + for _,e := range entries { + if e.Type == ftp.EntryTypeFile && e.Name == name { + entry = e + break + } + } + + if entry == nil { + return fmt.Errorf("Unable to find file: %s", uri.Path) + } + log.Printf("Found file : %s : %v bytes\n", entry.Name, entry.Size) + + d.progress = 0 + d.total = entry.Size + + // download specified file + d.active = true + reader,err := cli.RetrFrom(uri.Path, d.progress) + if err != nil { return nil } + + // do it in a goro so that if someone wants to cancel it, they can + errch := make(chan error) + go func(d *FTPDownloader, r *io.Reader, w *bufio.Writer, e chan error) { + defer w.Flush() + for ; d.active { + n,err := io.CopyN(writer, reader, d.mtu) + if err != nil { break } + d.progress += n + } + d.active = false + e <- err + }(d, reader, bufio.NewWriter(dst), errch) + + // spin until it's done + err = <-errch + reader.Close() + + if err == nil && d.progress != d.total { + err = fmt.Errorf("FTP total transfer size was %d when %d was expected", d.progress, d.total) + } + + // log out and quit + cli.Logout() + cli.Quit() + return err +} + +func (d *FTPDownloader) Progress() uint { + return d.progress +} + +func (d *FTPDownloader) Total() uint { + return d.total +} + +// FileDownloader is an implementation of Downloader that downloads +// files using the regular filesystem. +type FileDownloader struct { + bufferSize *uint + + active bool + progress uint + total uint +} + +func (*FileDownloader) Cancel() { + d.active = false +} + +func (d *FileDownloader) Progress() uint { + return d.progress +} + +func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { + d.active = false + + /* parse the uri using the net/url module */ + uri, err := url.Parse(src) + if uri.Scheme != "file" { + return fmt.Errorf("Unexpected uri scheme: %s", uri.Scheme) + } + + /* use the current working directory as the base for relative uri's */ + cwd,err := os.Getwd() + if err != nil { + return "", fmt.Errorf("Unable to get working directory") + } + + /* determine which uri format is being used and convert to a real path */ + var realpath string, basepath string + basepath = filepath.ToSlash(cwd) + + // absolute path -- file://c:/absolute/path + if strings.HasSuffix(uri.Host, ":") { + realpath = path.Join(uri.Host, uri.Path) + + // semi-absolute path (current drive letter) -- file:///absolute/path + } else if uri.Host == "" && strings.HasPrefix(uri.Path, "/") { + realpath = path.Join(filepath.VolumeName(basepath), uri.Path) + + // relative path -- file://./relative/path + } else if uri.Host == "." { + realpath = path.Join(basepath, uri.Path) + + // relative path -- file://relative/path + } else { + realpath = path.Join(basepath, uri.Host, uri.Path) + } + + /* download the file using the operating system's facilities */ + d.progress = 0 + d.active = true + + f, err = os.Open(realpath) + if err != nil { return err } + defer f.Close() + + // get the file size + fi, err := f.Stat() + if err != nil { return err } + d.total = fi.Size() + + // no bufferSize specified, so copy synchronously. + if d.bufferSize == nil { + n,err := io.Copy(dst, f) + d.active = false + d.progress += n + + // use a goro in case someone else wants to enable cancel/resume + } else { + errch := make(chan error) + go func(d* FileDownloader, r *bufio.Reader, w *bufio.Writer, e chan error) { + defer w.Flush() + for ; d.active { + n,err := io.CopyN(writer, reader, d.bufferSize) + if err != nil { break } + d.progress += n + } + d.active = false + e <- err + }(d, f, bufio.NewWriter(dst), errch) + + // ...and we spin until it's done + err = <-errch + } + f.Close() + return err +} + +func (d *FileDownloader) Total() uint { + return d.total +} + +// SMBDownloader is an implementation of Downloader that downloads +// files using the "\\" path format on Windows +type SMBDownloader struct { + bufferSize *uint + + active bool + progress uint + total uint +} + +func (*SMBDownloader) Cancel() { + d.active = false +} + +func (d *SMBDownloader) Progress() uint { + return d.progress +} + +func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { + const UNCPrefix = string(os.PathSeparator)+string(os.PathSeparator) + d.active = false + + if runtime.GOOS != "windows" { + return fmt.Errorf("Support for SMB based uri's are not supported on %s", runtime.GOOS) + } + + /* convert the uri using the net/url module to a UNC path */ + var realpath string + uri, err := url.Parse(src) + if uri.Scheme != "smb" { + return fmt.Errorf("Unexpected uri scheme: %s", uri.Scheme) + } + + realpath = UNCPrefix + filepath.ToSlash(path.Join(uri.Host, uri.Path)) + + /* Open up the "\\"-prefixed path using the Windows filesystem */ + d.progress = 0 + d.active = true + + f, err = os.Open(realpath) + if err != nil { return err } + defer f.Close() + + // get the file size (at the risk of performance) + fi, err := f.Stat() + if err != nil { return err } + d.total = fi.Size() + + // no bufferSize specified, so copy synchronously. + if d.bufferSize == nil { + n,err := io.Copy(dst, f) + d.active = false + d.progress += n + + // use a goro in case someone else wants to enable cancel/resume + } else { + errch := make(chan error) + go func(d* SMBDownloader, r *bufio.Reader, w *bufio.Writer, e chan error) { + defer w.Flush() + for ; d.active { + n,err := io.CopyN(writer, reader, d.bufferSize) + if err != nil { break } + d.progress += n + } + d.active = false + e <- err + }(d, f, bufio.NewWriter(dst), errch) + + // ...and as usual we spin until it's done + err = <-errch + } + f.Close() + return err +} + +func (d *SMBDownloader) Total() uint { + return d.total +} From 2157059e63d028d52274c99bbde81136c89a343e Mon Sep 17 00:00:00 2001 From: Ali Rizvi-Santiago Date: Tue, 5 Apr 2016 15:01:06 -0500 Subject: [PATCH 5/6] Refactored the code a bit to move the CopyFile hack out of DownloadClient and instead into each protocol. config.go: Removed all of the windows-specific net/url hackery since it's now handled mostly by download.go Removed the replacement of '\' with '/' since url.Parse does it now. Added knowledge of the other protocols implemented in download.go (ftp, smb) Removed some modules that were unused in this commit. download.go: Moved the file-path conversions for the different protocols into their own internally callable functions. Shuffled some of the functions around in case someone wants to implement the ability to resume. Modified DownloadClient.Get to remove the CopyFile special case and trust the protocol implementations if a user doesn't want to copy the file. Since all the protocols except for HTTPDownloader implement Cancel, added a Resume method as a placeholder for another developer to implement. Added a few missing names from their function definitions. Fixed a few lines due to my suckage at golang. Adjusted the types for progress and total so that they support 64-bit sizes. Removed the usage of the bufio library since it wasn't really being used. --- common/config.go | 121 +++------------ common/download.go | 372 ++++++++++++++++++++++++--------------------- 2 files changed, 220 insertions(+), 273 deletions(-) diff --git a/common/config.go b/common/config.go index c998a49caf5..0d5f7987c0f 100644 --- a/common/config.go +++ b/common/config.go @@ -2,10 +2,8 @@ package common import ( "fmt" - "net/url" "os" "path/filepath" - "runtime" "strings" ) @@ -37,115 +35,38 @@ func ChooseString(vals ...string) string { // a completely valid URL. For example, the original URL might be "local/file.iso" // which isn't a valid URL. DownloadableURL will return "file:///local/file.iso" func DownloadableURL(original string) (string, error) { - if runtime.GOOS == "windows" { - // If the distance to the first ":" is just one character, assume - // we're dealing with a drive letter and thus a file path. - idx := strings.Index(original, ":") - if idx == 1 { - original = "file://" + filepath.ToSlash(original) - } - } - - // XXX: The validation here is later re-parsed in common/download.go and - // thus any modifications here must remain consistent over there too. - uri, err := url.Parse(original) - if err != nil { - return "", err - } - - if uri.Scheme == "" { - uri.Scheme = "file" - } - - const UNCPrefix = string(os.PathSeparator)+string(os.PathSeparator) - if uri.Scheme == "file" { - var ospath string // os-formatted pathname - if runtime.GOOS == "windows" { - // Move any extra path components that were mis-parsed into the Host - // field back into the uri.Path field - if len(uri.Host) >= len(UNCPrefix) && uri.Host[:len(UNCPrefix)] == UNCPrefix { - idx := strings.Index(uri.Host[len(UNCPrefix):], string(os.PathSeparator)) - if idx > -1 { - uri.Path = filepath.ToSlash(uri.Host[idx+len(UNCPrefix):]) + uri.Path - uri.Host = uri.Host[:idx+len(UNCPrefix)] - } - } - // Now all we need to do to convert the uri to a platform-specific path - // is to trade it's slashes for some os.PathSeparator ones. - ospath = uri.Host + filepath.FromSlash(uri.Path) - - } else { - // Since we're already using sane paths on a sane platform, anything in - // uri.Host can be assumed that the user is describing a relative uri. - // This means that if we concatenate it with uri.Path, the filepath - // transform will still open the file correctly. - // i.e. file://localdirectory/filename -> localdirectory/filename - ospath = uri.Host + uri.Path - } - - // Only do the filepath transformations if the file appears - // to actually exist. We don't do it on windows, because EvalSymlinks - // won't understand how to handle UNC paths and other Windows-specific minutae. - if _, err := os.Stat(ospath); err == nil && runtime.GOOS != "windows" { - ospath, err = filepath.Abs(ospath) - if err != nil { - return "", err - } - - ospath, err = filepath.EvalSymlinks(ospath) - if err != nil { - return "", err - } - - ospath = filepath.Clean(ospath) - } - - // now that ospath was normalized and such.. - if runtime.GOOS == "windows" { - uri.Host = "" - // Check to see if our ospath is unc-prefixed, and if it is then split - // the UNC host into uri.Host, leaving the rest in ospath. - // This way, our UNC-uri is protected from injury in the call to uri.String() - if len(ospath) >= len(UNCPrefix) && ospath[:len(UNCPrefix)] == UNCPrefix { - idx := strings.Index(ospath[len(UNCPrefix):], string(os.PathSeparator)) - if idx > -1 { - uri.Host = ospath[:len(UNCPrefix)+idx] - ospath = ospath[len(UNCPrefix)+idx:] - } - } - // Restore the uri by re-transforming our os-formatted path - uri.Path = filepath.ToSlash(ospath) - } else { - uri.Host = "" - uri.Path = filepath.ToSlash(ospath) - } - } - - // Make sure it is lowercased - uri.Scheme = strings.ToLower(uri.Scheme) // Verify that the scheme is something we support in our common downloader. - supported := []string{"file", "http", "https"} + supported := []string{"file", "http", "https", "ftp", "smb"} found := false for _, s := range supported { - if uri.Scheme == s { + if strings.HasPrefix(original, s + "://") { found = true break } } - if !found { - return "", fmt.Errorf("Unsupported URL scheme: %s", uri.Scheme) + // If it's properly prefixed with something we support, then we don't need + // to make it a uri. + if found { + return original, nil } - // explicit check to see if we need to manually replace the uri host with a UNC one - if runtime.GOOS == "windows" && uri.Scheme == "file" { - if len(uri.Host) >= len(UNCPrefix) && uri.Host[:len(UNCPrefix)] == UNCPrefix { - escapedHost := url.QueryEscape(uri.Host) - return strings.Replace(uri.String(), escapedHost, uri.Host, 1), nil - } + // If the file exists, then make it an absolute path + _,err := os.Stat(original) + if err == nil { + original, err = filepath.Abs(filepath.FromSlash(original)) + if err != nil { return "", err } + + original, err = filepath.EvalSymlinks(original) + if err != nil { return "", err } + + original = filepath.Clean(original) + original = filepath.ToSlash(original) } - // otherwise, we can trust the url handler - return uri.String(), nil + // Since it wasn't properly prefixed, let's make it into a well-formed + // file:// uri. + + return "file://" + original, nil } diff --git a/common/download.go b/common/download.go index d8d9d9e537d..eeb53fb3a40 100644 --- a/common/download.go +++ b/common/download.go @@ -10,20 +10,20 @@ import ( "errors" "fmt" "hash" - "io" "log" "net/url" "os" "runtime" "path" - "path/filepath" "strings" ) +// imports related to each Downloader implementation import ( + "io" + "path/filepath" "net/http" - "github.com/jlaffeye/ftp" - "bufio" + "github.com/jlaffaye/ftp" ) @@ -89,23 +89,35 @@ func NewDownloadClient(c *DownloadConfig) *DownloadClient { if c.DownloaderMap == nil { c.DownloaderMap = map[string]Downloader{ "file": &FileDownloader{bufferSize: nil}, - "ftp": &FTPDownloader{userInfo: url.Userinfo{username:"anonymous", password: "anonymous@"}, mtu: mtu}, + "ftp": &FTPDownloader{userInfo: url.UserPassword("anonymous", "anonymous@"), mtu: mtu}, "http": &HTTPDownloader{userAgent: c.UserAgent}, "https": &HTTPDownloader{userAgent: c.UserAgent}, - "smb": &SMBDownloader{bufferSize: nil} + "smb": &SMBDownloader{bufferSize: nil}, } } return &DownloadClient{config: c} } -// A downloader is responsible for actually taking a remote URL and -// downloading it. +// A downloader implements the ability to transfer a file, and cancel or resume +// it. type Downloader interface { + Resume() Cancel() + Progress() uint64 + Total() uint64 +} + +// A LocalDownloader is responsible for converting a uri to a local path +// that the platform can open directly. +type LocalDownloader interface { + toPath(string, url.URL) (string,error) +} + +// A RemoteDownloader is responsible for actually taking a remote URL and +// downloading it. +type RemoteDownloader interface { Download(*os.File, *url.URL) error - Progress() uint - Total() uint } func (d *DownloadClient) Cancel() { @@ -119,64 +131,54 @@ func (d *DownloadClient) Get() (string, error) { return d.config.TargetPath, nil } + /* parse the configuration url into a net/url object */ url, err := url.Parse(d.config.Url) - if err != nil { - return "", err - } - + if err != nil { return "", err } log.Printf("Parsed URL: %#v", url) - /* FIXME: - handle the special case of d.config.CopyFile which returns the path - in an os-specific format. - */ + /* use the current working directory as the base for relative uri's */ + cwd,err := os.Getwd() + if err != nil { return "", err } - // Files when we don't copy the file are special cased. - var f *os.File + // Determine which is the correct downloader to use var finalPath string - sourcePath := "" - if url.Scheme == "file" && !d.config.CopyFile { - // This is a special case where we use a source file that already exists - // locally and we don't make a copy. Normally we would copy or download. - finalPath = url.Path - log.Printf("[DEBUG] Using local file: %s", finalPath) - // transform the actual file uri to a windowsy path if we're being windowsy. - if runtime.GOOS == "windows" { - // FIXME: cwd should point to a path relative to the TEMPLATE path, - // but since this isn't exposed to us anywhere, we use os.Getwd() - // and assume the user ran packer in the same directory that - // any relative files are located at. - cwd,err := os.Getwd() - if err != nil { - return "", fmt.Errorf("Unable to get working directory") - } - finalPath = NormalizeWindowsURL(cwd, *url) - } + var ok bool + d.downloader, ok = d.config.DownloaderMap[url.Scheme] + if !ok { + return "", fmt.Errorf("No downloader for scheme: %s", url.Scheme) + } - // Keep track of the source so we can make sure not to delete this later - sourcePath = finalPath - } else { - finalPath = d.config.TargetPath + remote,ok := d.downloader.(RemoteDownloader) + if !ok { + return "", fmt.Errorf("Unable to treat uri scheme %s as a Downloader : %T", url.Scheme, d.downloader) + } - var ok bool - d.downloader, ok = d.config.DownloaderMap[url.Scheme] - if !ok { - return "", fmt.Errorf("No downloader for scheme: %s", url.Scheme) - } + local,ok := d.downloader.(LocalDownloader) + if !ok && !d.config.CopyFile{ + return "", fmt.Errorf("Not allowed to use uri scheme %s in no copy file mode : %T", url.Scheme, d.downloader) + } + + // If we're copying the file, then just use the actual downloader + if d.config.CopyFile { + var f *os.File + finalPath = d.config.TargetPath - // Otherwise, download using the downloader. f, err = os.OpenFile(finalPath, os.O_RDWR|os.O_CREATE, os.FileMode(0666)) - if err != nil { - return "", err - } + if err != nil { return "", err } log.Printf("[DEBUG] Downloading: %s", url.String()) - err = d.downloader.Download(f, url) + err = remote.Download(f, url) f.Close() - if err != nil { - return "", err - } + if err != nil { return "", err } + + // Otherwise if our Downloader is a LocalDownloader we can just use the + // path after transforming it. + } else { + finalPath,err = local.toPath(cwd, *url) + if err != nil { return "", err } + + log.Printf("[DEBUG] Using local file: %s", finalPath) } if d.config.Hash != nil { @@ -184,9 +186,7 @@ func (d *DownloadClient) Get() (string, error) { verify, err = d.VerifyChecksum(finalPath) if err == nil && !verify { // Only delete the file if we made a copy or downloaded it - if sourcePath != finalPath { - os.Remove(finalPath) - } + if d.config.CopyFile { os.Remove(finalPath) } err = fmt.Errorf( "checksums didn't match expected: %s", @@ -199,10 +199,7 @@ func (d *DownloadClient) Get() (string, error) { // PercentProgress returns the download progress as a percentage. func (d *DownloadClient) PercentProgress() int { - if d.downloader == nil { - return -1 - } - + if d.downloader == nil { return -1 } return int((float64(d.downloader.Progress()) / float64(d.downloader.Total())) * 100) } @@ -228,12 +225,16 @@ func (d *DownloadClient) VerifyChecksum(path string) (bool, error) { // HTTPDownloader is an implementation of Downloader that downloads // files over HTTP. type HTTPDownloader struct { - progress uint - total uint + progress uint64 + total uint64 userAgent string } -func (*HTTPDownloader) Cancel() { +func (d *HTTPDownloader) Cancel() { + // TODO(mitchellh): Implement +} + +func (d *HTTPDownloader) Resume() { // TODO(mitchellh): Implement } @@ -274,7 +275,7 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { if fi, err := dst.Stat(); err == nil { if _, err = dst.Seek(0, os.SEEK_END); err == nil { req.Header.Set("Range", fmt.Sprintf("bytes=%d-", fi.Size())) - d.progress = uint(fi.Size()) + d.progress = uint64(fi.Size()) } } } @@ -288,7 +289,7 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { return err } - d.total = d.progress + uint(resp.ContentLength) + d.total = d.progress + uint64(resp.ContentLength) var buffer [4096]byte for { n, err := resp.Body.Read(buffer[:]) @@ -296,7 +297,7 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { return err } - d.progress += uint(n) + d.progress += uint64(n) if _, werr := dst.Write(buffer[:n]); werr != nil { return werr @@ -310,29 +311,41 @@ func (d *HTTPDownloader) Download(dst *os.File, src *url.URL) error { return nil } -func (d *HTTPDownloader) Progress() uint { +func (d *HTTPDownloader) Progress() uint64 { return d.progress } -func (d *HTTPDownloader) Total() uint { +func (d *HTTPDownloader) Total() uint64 { return d.total } // FTPDownloader is an implementation of Downloader that downloads // files over FTP. type FTPDownloader struct { - userInfo url.UserInfo + userInfo *url.Userinfo mtu uint active bool - progress uint - total uint + progress uint64 + total uint64 +} + +func (d *FTPDownloader) Progress() uint64 { + return d.progress +} + +func (d *FTPDownloader) Total() uint64 { + return d.total } -func (*FTPDownloader) Cancel() { +func (d *FTPDownloader) Cancel() { d.active = false } +func (d *FTPDownloader) Resume() { + // TODO: Implement +} + func (d *FTPDownloader) Download(dst *os.File, src *url.URL) error { var userinfo *url.Userinfo @@ -340,33 +353,34 @@ func (d *FTPDownloader) Download(dst *os.File, src *url.URL) error { d.active = false // check the uri is correct - uri, err := url.Parse(src) - if err != nil { return err } - - if uri.Scheme != "ftp" { - return fmt.Errorf("Unexpected uri scheme: %s", uri.Scheme) + if src == nil || src.Scheme != "ftp" { + return fmt.Errorf("Unexpected uri scheme: %s", src.Scheme) } + uri := src // connect to ftp server var cli *ftp.ServerConn - log.Printf("Starting download over FTP: %s : %s\n", uri.Host, Uri.Path) + log.Printf("Starting download over FTP: %s : %s\n", uri.Host, uri.Path) cli,err := ftp.Dial(uri.Host) if err != nil { return nil } - defer cli.Close() + defer cli.Quit() // handle authentication if uri.User != nil { userinfo = uri.User } - log.Printf("Authenticating to FTP server: %s : %s\n", uri.User.username, uri.User.password) - err = cli.Login(userinfo.username, userinfo.password) + pass,ok := userinfo.Password() + if !ok { pass = "ftp@" } + + log.Printf("Authenticating to FTP server: %s : %s\n", userinfo.Username(), pass) + err = cli.Login(userinfo.Username(), pass) if err != nil { return err } // locate specified path - path := path.Dir(uri.Path) + p := path.Dir(uri.Path) - log.Printf("Changing to FTP directory : %s\n", path) - err = cli.ChangeDir(path) + log.Printf("Changing to FTP directory : %s\n", p) + err = cli.ChangeDir(p) if err != nil { return nil } curpath,err := cli.CurrentDir() @@ -382,7 +396,7 @@ func (d *FTPDownloader) Download(dst *os.File, src *url.URL) error { entries,err := cli.List(curpath) for _,e := range entries { - if e.Type == ftp.EntryTypeFile && e.Name == name { + if e.Type == ftp.EntryTypeFile && e.Name == name { entry = e break } @@ -403,16 +417,15 @@ func (d *FTPDownloader) Download(dst *os.File, src *url.URL) error { // do it in a goro so that if someone wants to cancel it, they can errch := make(chan error) - go func(d *FTPDownloader, r *io.Reader, w *bufio.Writer, e chan error) { - defer w.Flush() - for ; d.active { - n,err := io.CopyN(writer, reader, d.mtu) + go func(d *FTPDownloader, r io.Reader, w io.Writer, e chan error) { + for ; d.active; { + n,err := io.CopyN(w, r, int64(d.mtu)) if err != nil { break } - d.progress += n + d.progress += uint64(n) } d.active = false e <- err - }(d, reader, bufio.NewWriter(dst), errch) + }(d, reader, dst, errch) // spin until it's done err = <-errch @@ -422,106 +435,109 @@ func (d *FTPDownloader) Download(dst *os.File, src *url.URL) error { err = fmt.Errorf("FTP total transfer size was %d when %d was expected", d.progress, d.total) } - // log out and quit + // log out cli.Logout() - cli.Quit() return err } -func (d *FTPDownloader) Progress() uint { - return d.progress -} - -func (d *FTPDownloader) Total() uint { - return d.total -} - // FileDownloader is an implementation of Downloader that downloads // files using the regular filesystem. type FileDownloader struct { bufferSize *uint active bool - progress uint - total uint + progress uint64 + total uint64 } -func (*FileDownloader) Cancel() { - d.active = false +func (d *FileDownloader) Progress() uint64 { + return d.progress } -func (d *FileDownloader) Progress() uint { - return d.progress +func (d *FileDownloader) Total() uint64 { + return d.total } -func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { +func (d *FileDownloader) Cancel() { d.active = false +} - /* parse the uri using the net/url module */ - uri, err := url.Parse(src) - if uri.Scheme != "file" { - return fmt.Errorf("Unexpected uri scheme: %s", uri.Scheme) - } - - /* use the current working directory as the base for relative uri's */ - cwd,err := os.Getwd() - if err != nil { - return "", fmt.Errorf("Unable to get working directory") - } +func (d *FileDownloader) Resume() { + // TODO: Implement +} - /* determine which uri format is being used and convert to a real path */ - var realpath string, basepath string - basepath = filepath.ToSlash(cwd) +func (d *FileDownloader) toPath(base string, uri url.URL) (string,error) { + var result string - // absolute path -- file://c:/absolute/path + // absolute path -- file://c:/absolute/path -> c:/absolute/path if strings.HasSuffix(uri.Host, ":") { - realpath = path.Join(uri.Host, uri.Path) + result = path.Join(uri.Host, uri.Path) - // semi-absolute path (current drive letter) -- file:///absolute/path + // semi-absolute path (current drive letter) + // -- file:///absolute/path -> /absolute/path } else if uri.Host == "" && strings.HasPrefix(uri.Path, "/") { - realpath = path.Join(filepath.VolumeName(basepath), uri.Path) + result = path.Join(filepath.VolumeName(base), uri.Path) - // relative path -- file://./relative/path + // relative path -- file://./relative/path -> ./relative/path } else if uri.Host == "." { - realpath = path.Join(basepath, uri.Path) + result = path.Join(base, uri.Path) - // relative path -- file://relative/path + // relative path -- file://relative/path -> ./relative/path } else { - realpath = path.Join(basepath, uri.Host, uri.Path) + result = path.Join(base, uri.Host, uri.Path) + } + return filepath.ToSlash(result),nil +} + +func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { + d.active = false + + /* check the uri's scheme to make sure it matches */ + if src == nil || src.Scheme != "file" { + return fmt.Errorf("Unexpected uri scheme: %s", src.Scheme) } + uri := src + + /* use the current working directory as the base for relative uri's */ + cwd,err := os.Getwd() + if err != nil { return err } + + /* determine which uri format is being used and convert to a real path */ + realpath,err := d.toPath(cwd, *uri) + if err != nil { return err } /* download the file using the operating system's facilities */ d.progress = 0 d.active = true - f, err = os.Open(realpath) + f, err := os.Open(realpath) if err != nil { return err } defer f.Close() // get the file size fi, err := f.Stat() if err != nil { return err } - d.total = fi.Size() + d.total = uint64(fi.Size()) // no bufferSize specified, so copy synchronously. if d.bufferSize == nil { - n,err := io.Copy(dst, f) + var n int64 + n,err = io.Copy(dst, f) d.active = false - d.progress += n + d.progress += uint64(n) // use a goro in case someone else wants to enable cancel/resume } else { errch := make(chan error) - go func(d* FileDownloader, r *bufio.Reader, w *bufio.Writer, e chan error) { - defer w.Flush() - for ; d.active { - n,err := io.CopyN(writer, reader, d.bufferSize) + go func(d* FileDownloader, r io.Reader, w io.Writer, e chan error) { + for ; d.active; { + n,err := io.CopyN(w, r, int64(*d.bufferSize)) if err != nil { break } - d.progress += n + d.progress += uint64(n) } d.active = false e <- err - }(d, f, bufio.NewWriter(dst), errch) + }(d, f, dst, errch) // ...and we spin until it's done err = <-errch @@ -530,77 +546,91 @@ func (d *FileDownloader) Download(dst *os.File, src *url.URL) error { return err } -func (d *FileDownloader) Total() uint { - return d.total -} - // SMBDownloader is an implementation of Downloader that downloads // files using the "\\" path format on Windows type SMBDownloader struct { bufferSize *uint active bool - progress uint - total uint + progress uint64 + total uint64 +} + +func (d *SMBDownloader) Progress() uint64 { + return d.progress } -func (*SMBDownloader) Cancel() { +func (d *SMBDownloader) Total() uint64 { + return d.total +} + +func (d *SMBDownloader) Cancel() { d.active = false } -func (d *SMBDownloader) Progress() uint { - return d.progress +func (d *SMBDownloader) Resume() { + // TODO: Implement } -func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { +func (d *SMBDownloader) toPath(base string, uri url.URL) (string,error) { const UNCPrefix = string(os.PathSeparator)+string(os.PathSeparator) - d.active = false if runtime.GOOS != "windows" { - return fmt.Errorf("Support for SMB based uri's are not supported on %s", runtime.GOOS) + return "",fmt.Errorf("Support for SMB based uri's are not supported on %s", runtime.GOOS) } + return UNCPrefix + filepath.ToSlash(path.Join(uri.Host, uri.Path)), nil +} + +func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { + d.active = false + /* convert the uri using the net/url module to a UNC path */ - var realpath string - uri, err := url.Parse(src) - if uri.Scheme != "smb" { - return fmt.Errorf("Unexpected uri scheme: %s", uri.Scheme) + if src == nil || src.Scheme != "smb" { + return fmt.Errorf("Unexpected uri scheme: %s", src.Scheme) } + uri := src - realpath = UNCPrefix + filepath.ToSlash(path.Join(uri.Host, uri.Path)) + /* use the current working directory as the base for relative uri's */ + cwd,err := os.Getwd() + if err != nil { return err } + + /* convert uri to an smb-path */ + realpath,err := d.toPath(cwd, *uri) + if err != nil { return err } /* Open up the "\\"-prefixed path using the Windows filesystem */ d.progress = 0 d.active = true - f, err = os.Open(realpath) + f, err := os.Open(realpath) if err != nil { return err } defer f.Close() // get the file size (at the risk of performance) fi, err := f.Stat() if err != nil { return err } - d.total = fi.Size() + d.total = uint64(fi.Size()) // no bufferSize specified, so copy synchronously. if d.bufferSize == nil { - n,err := io.Copy(dst, f) + var n int64 + n,err = io.Copy(dst, f) d.active = false - d.progress += n + d.progress += uint64(n) // use a goro in case someone else wants to enable cancel/resume } else { errch := make(chan error) - go func(d* SMBDownloader, r *bufio.Reader, w *bufio.Writer, e chan error) { - defer w.Flush() - for ; d.active { - n,err := io.CopyN(writer, reader, d.bufferSize) + go func(d* SMBDownloader, r io.Reader, w io.Writer, e chan error) { + for ; d.active; { + n,err := io.CopyN(w, r, int64(*d.bufferSize)) if err != nil { break } - d.progress += n + d.progress += uint64(n) } d.active = false e <- err - }(d, f, bufio.NewWriter(dst), errch) + }(d, f, dst, errch) // ...and as usual we spin until it's done err = <-errch @@ -608,7 +638,3 @@ func (d *SMBDownloader) Download(dst *os.File, src *url.URL) error { f.Close() return err } - -func (d *SMBDownloader) Total() uint { - return d.total -} From 2b848bced2d2f44c532813bb98ff240ed0c641ca Mon Sep 17 00:00:00 2001 From: Ali Rizvi-Santiago Date: Tue, 5 Apr 2016 16:51:17 -0500 Subject: [PATCH 6/6] Fixed the unit-tests for common/ due to the changes made in {config,download}.go common/config.go: Fixed some issues related to the url scheme not being lowercased which broke some of the tests. common/config_test.go: Removed the UNC share test for \\host\share\file since SMB support has been moved to a different uri scheme. common/download_test.go: Explicitly set the CopyFile configuration option for all the unit-tests that test file copying capability. Removed the UNC share testcase since it's under a different uri scheme now. Modified the file:// UNC share testcase to explicitly test the smb:// uri. Changed the incorrect t.Errorf calls to t.Logf so that the tests can pass. --- common/config.go | 13 +++++++++++-- common/config_test.go | 17 ++--------------- common/download_test.go | 21 +++++++++++++-------- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/common/config.go b/common/config.go index 0d5f7987c0f..0d8228cc6c2 100644 --- a/common/config.go +++ b/common/config.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "strings" + "net/url" ) // ScrubConfig is a helper that returns a string representation of @@ -40,7 +41,7 @@ func DownloadableURL(original string) (string, error) { supported := []string{"file", "http", "https", "ftp", "smb"} found := false for _, s := range supported { - if strings.HasPrefix(original, s + "://") { + if strings.HasPrefix(strings.ToLower(original), s + "://") { found = true break } @@ -49,7 +50,15 @@ func DownloadableURL(original string) (string, error) { // If it's properly prefixed with something we support, then we don't need // to make it a uri. if found { - return original, nil + original = filepath.ToSlash(original) + + // make sure that it can be parsed though.. + uri,err := url.Parse(original) + if err != nil { return "", err } + + uri.Scheme = strings.ToLower(uri.Scheme) + + return uri.String(), nil } // If the file exists, then make it an absolute path diff --git a/common/config_test.go b/common/config_test.go index 92a7316a314..4cdeb9b7315 100644 --- a/common/config_test.go +++ b/common/config_test.go @@ -5,7 +5,6 @@ import ( "io/ioutil" "os" "path/filepath" - "runtime" "strings" "testing" ) @@ -41,13 +40,7 @@ func TestDownloadableURL(t *testing.T) { // Invalid URL: has hex code in host _, err := DownloadableURL("http://what%20.com") if err == nil { - t.Fatal("expected err") - } - - // Invalid: unsupported scheme - _, err = DownloadableURL("ftp://host.com/path") - if err == nil { - t.Fatal("expected err") + t.Fatalf("expected err : %s", err) } // Valid: http @@ -85,11 +78,7 @@ func TestDownloadableURL_FilePaths(t *testing.T) { } tfPath = filepath.Clean(tfPath) - filePrefix := "file://" - if runtime.GOOS == "windows" { - filePrefix += "/" - } // Relative filepath. We run this test in a func so that // the defers run right away. @@ -111,9 +100,7 @@ func TestDownloadableURL_FilePaths(t *testing.T) { t.Fatalf("err: %s", err) } - expected := fmt.Sprintf("%s%s", - filePrefix, - strings.Replace(tfPath, `\`, `/`, -1)) + expected := "file://" + strings.Replace(tfPath, `\`, `/`, -1) if u != expected { t.Fatalf("unexpected: %#v != %#v", u, expected) } diff --git a/common/download_test.go b/common/download_test.go index 59235b7013e..d47638b09c7 100644 --- a/common/download_test.go +++ b/common/download_test.go @@ -57,6 +57,7 @@ func TestDownloadClient_basic(t *testing.T) { client := NewDownloadClient(&DownloadConfig{ Url: ts.URL + "/basic.txt", TargetPath: tf.Name(), + CopyFile: true, }) path, err := client.Get() if err != nil { @@ -91,6 +92,7 @@ func TestDownloadClient_checksumBad(t *testing.T) { TargetPath: tf.Name(), Hash: HashForType("md5"), Checksum: checksum, + CopyFile: true, }) if _, err := client.Get(); err == nil { t.Fatal("should error") @@ -115,6 +117,7 @@ func TestDownloadClient_checksumGood(t *testing.T) { TargetPath: tf.Name(), Hash: HashForType("md5"), Checksum: checksum, + CopyFile: true, }) path, err := client.Get() if err != nil { @@ -145,6 +148,7 @@ func TestDownloadClient_checksumNoDownload(t *testing.T) { TargetPath: "./test-fixtures/root/another.txt", Hash: HashForType("md5"), Checksum: checksum, + CopyFile: true, }) path, err := client.Get() if err != nil { @@ -183,6 +187,7 @@ func TestDownloadClient_resume(t *testing.T) { client := NewDownloadClient(&DownloadConfig{ Url: ts.URL, TargetPath: tf.Name(), + CopyFile: true, }) path, err := client.Get() if err != nil { @@ -240,6 +245,7 @@ func TestDownloadClient_usesDefaultUserAgent(t *testing.T) { config := &DownloadConfig{ Url: server.URL, TargetPath: tf.Name(), + CopyFile: true, } client := NewDownloadClient(config) @@ -271,6 +277,7 @@ func TestDownloadClient_setsUserAgent(t *testing.T) { Url: server.URL, TargetPath: tf.Name(), UserAgent: "fancy user agent", + CopyFile: true, } client := NewDownloadClient(config) @@ -351,6 +358,7 @@ func TestDownloadFileUrl(t *testing.T) { if err != nil { t.Fatalf("Unable to detect working directory: %s", err) } + cwd = filepath.ToSlash(cwd) // source_path is a file path and source is a network path sourcePath := fmt.Sprintf("%s/test-fixtures/fileurl/%s", cwd, "cake") @@ -447,12 +455,10 @@ func TestFileUriTransforms(t *testing.T) { // ./relative/path -> ./relative/path // /absolute/path -> /absolute/path // c:/windows/absolute -> c:/windows/absolute - // \\host/sharename/file -> \\host/sharename/file testcases := []string{ "./%s", cwd + "/%s", volume + cwd + "/%s", - "\\\\" + host + "/" + share + "/" + cwd[1:] + "/%s", } // all regular slashed testcases @@ -463,19 +469,18 @@ func TestFileUriTransforms(t *testing.T) { if err != nil { t.Errorf("Unable to transform uri '%s' into a path : %v", uri, err) } - t.Errorf("TestFileUriTransforms : Result Path '%s'", res) + t.Logf("TestFileUriTransforms : Result Path '%s'", res) } // ...and finally the oddball windows native path - // \\host\sharename\file -> \\host/sharename/file - testpath_native := filepath.FromSlash(testpath) - testcase_native := "\\\\" + host + "\\" + share + "\\" + filepath.FromSlash(cwd[1:]) + "\\%s" - uri := "file://" + fmt.Sprintf(testcase_native, testpath_native) + // smb://host/sharename/file -> \\host\sharename\file + testcase := host + "/" + share + "/" + cwd[1:] + "/%s" + uri := "smb://" + fmt.Sprintf(testcase, testpath) t.Logf("TestFileUriTransforms : Trying Uri '%s'", uri) res,err := SimulateFileUriDownload(t, uri) if err != nil { t.Errorf("Unable to transform uri '%s' into a path", uri) return } - t.Errorf("TestFileUriTransforms : Result Path '%s'", res) + t.Logf("TestFileUriTransforms : Result Path '%s'", res) }