From e7bbbfb09834789121e2a96a8fd889b8030eed3b Mon Sep 17 00:00:00 2001 From: Emil Hessman Date: Tue, 3 Feb 2015 20:44:34 +0100 Subject: [PATCH] helper/url: add Windows 'safe' URL Parse wrapper Pull out the urlParse function, which was introduced in config/module, into a helper package. --- config/module/detect.go | 6 ++- config/module/detect_file.go | 15 ++++++ config/module/detect_file_test.go | 6 +-- config/module/get.go | 4 +- config/module/get_hg.go | 4 +- config/module/module_test.go | 3 +- config/module/url_helper.go | 63 ---------------------- helper/url/url.go | 14 +++++ helper/url/url_test.go | 88 +++++++++++++++++++++++++++++++ helper/url/url_unix.go | 11 ++++ helper/url/url_windows.go | 40 ++++++++++++++ 11 files changed, 183 insertions(+), 71 deletions(-) delete mode 100644 config/module/url_helper.go create mode 100644 helper/url/url.go create mode 100644 helper/url/url_test.go create mode 100644 helper/url/url_unix.go create mode 100644 helper/url/url_windows.go diff --git a/config/module/detect.go b/config/module/detect.go index 84e1a1d79..51e07f725 100644 --- a/config/module/detect.go +++ b/config/module/detect.go @@ -3,6 +3,8 @@ package module import ( "fmt" "path/filepath" + + "github.com/hashicorp/terraform/helper/url" ) // Detector defines the interface that an invalid URL or a URL with a blank @@ -37,7 +39,7 @@ func Detect(src string, pwd string) (string, error) { // Separate out the subdir if there is one, we don't pass that to detect getSrc, subDir := getDirSubdir(getSrc) - u, err := urlParse(getSrc) + u, err := url.Parse(getSrc) if err == nil && u.Scheme != "" { // Valid URL return src, nil @@ -66,7 +68,7 @@ func Detect(src string, pwd string) (string, error) { } } if subDir != "" { - u, err := urlParse(result) + u, err := url.Parse(result) if err != nil { return "", fmt.Errorf("Error parsing URL: %s", err) } diff --git a/config/module/detect_file.go b/config/module/detect_file.go index 4aa21ffa2..2b8dbacbe 100644 --- a/config/module/detect_file.go +++ b/config/module/detect_file.go @@ -3,6 +3,7 @@ package module import ( "fmt" "path/filepath" + "runtime" ) // FileDetector implements Detector to detect file paths. @@ -23,3 +24,17 @@ func (d *FileDetector) Detect(src, pwd string) (string, bool, error) { } return fmtFileURL(src), true, nil } + +func fmtFileURL(path string) string { + if runtime.GOOS == "windows" { + // Make sure we're using "/" on Windows. URLs are "/"-based. + path = filepath.ToSlash(path) + return fmt.Sprintf("file://%s", path) + } + + // Make sure that we don't start with "/" since we add that below. + if path[0] == '/' { + path = path[1:] + } + return fmt.Sprintf("file:///%s", path) +} diff --git a/config/module/detect_file_test.go b/config/module/detect_file_test.go index d20271bd6..4c75ce83d 100644 --- a/config/module/detect_file_test.go +++ b/config/module/detect_file_test.go @@ -23,8 +23,8 @@ var unixFileTests = []fileTest{ var winFileTests = []fileTest{ {"/foo", "/pwd", "file:///pwd/foo", false}, - {`C:\`, `/pwd`, `file:///C:/`, false}, - {`C:\?bar=baz`, `/pwd`, `file:///C:/?bar=baz`, false}, + {`C:\`, `/pwd`, `file://C:/`, false}, + {`C:\?bar=baz`, `/pwd`, `file://C:/?bar=baz`, false}, } func TestFileDetector(t *testing.T) { @@ -61,7 +61,7 @@ var noPwdUnixFileTests = []fileTest{ var noPwdWinFileTests = []fileTest{ {in: "/foo", pwd: "", out: "", err: true}, - {in: `C:\`, pwd: ``, out: `file:///C:/`, err: false}, + {in: `C:\`, pwd: ``, out: `file://C:/`, err: false}, } func TestFileDetector_noPwd(t *testing.T) { diff --git a/config/module/get.go b/config/module/get.go index abc724b58..627d395a9 100644 --- a/config/module/get.go +++ b/config/module/get.go @@ -11,6 +11,8 @@ import ( "regexp" "strings" "syscall" + + urlhelper "github.com/hashicorp/terraform/helper/url" ) // Getter defines the interface that schemes must implement to download @@ -72,7 +74,7 @@ func Get(dst, src string) error { dst = tmpDir } - u, err := urlParse(src) + u, err := urlhelper.Parse(src) if err != nil { return err } diff --git a/config/module/get_hg.go b/config/module/get_hg.go index 666762080..f74c14093 100644 --- a/config/module/get_hg.go +++ b/config/module/get_hg.go @@ -6,6 +6,8 @@ import ( "os" "os/exec" "runtime" + + urlhelper "github.com/hashicorp/terraform/helper/url" ) // HgGetter is a Getter implementation that will download a module from @@ -17,7 +19,7 @@ func (g *HgGetter) Get(dst string, u *url.URL) error { return fmt.Errorf("hg must be available and on the PATH") } - newURL, err := urlParse(u.String()) + newURL, err := urlhelper.Parse(u.String()) if err != nil { return err } diff --git a/config/module/module_test.go b/config/module/module_test.go index 88a595cd3..f1517e480 100644 --- a/config/module/module_test.go +++ b/config/module/module_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/hashicorp/terraform/config" + urlhelper "github.com/hashicorp/terraform/helper/url" ) const fixtureDir = "./test-fixtures" @@ -43,7 +44,7 @@ func testModule(n string) string { } func testModuleURL(n string) *url.URL { - u, err := urlParse(testModule(n)) + u, err := urlhelper.Parse(testModule(n)) if err != nil { panic(err) } diff --git a/config/module/url_helper.go b/config/module/url_helper.go deleted file mode 100644 index 792761927..000000000 --- a/config/module/url_helper.go +++ /dev/null @@ -1,63 +0,0 @@ -package module - -import ( - "fmt" - "net/url" - "path/filepath" - "runtime" - "strings" -) - -func urlParse(rawURL string) (*url.URL, error) { - if runtime.GOOS == "windows" { - // Make sure we're using "/" on Windows. URLs are "/"-based. - rawURL = filepath.ToSlash(rawURL) - } - u, err := url.Parse(rawURL) - if err != nil { - return nil, err - } - - if runtime.GOOS != "windows" { - return u, err - } - - if len(rawURL) > 1 && rawURL[1] == ':' { - // Assume we're dealing with a drive letter file path on Windows. - // We need to adjust the URL Path for drive letter file paths - // because url.Parse("c:/users/user") yields URL Scheme = "c" - // and URL path = "/users/user". - u.Path = fmt.Sprintf("%s:%s", u.Scheme, u.Path) - u.Scheme = "" - } - - if len(u.Host) > 1 && u.Host[1] == ':' && strings.HasPrefix(rawURL, "file://") { - // Assume we're dealing with a drive letter file path on Windows - // where the drive letter has been parsed into the URL Host. - u.Path = fmt.Sprintf("%s%s", u.Host, u.Path) - u.Host = "" - } - - // Remove leading slash for absolute file paths on Windows. - // For example, url.Parse yields u.Path = "/C:/Users/user" for - // rawURL = "file:///C:/Users/user", which is an incorrect syntax. - if len(u.Path) > 2 && u.Path[0] == '/' && u.Path[2] == ':' { - u.Path = u.Path[1:] - } - - return u, err -} - -func fmtFileURL(path string) string { - if runtime.GOOS == "windows" { - // Make sure we're using "/" on Windows. URLs are "/"-based. - path = filepath.ToSlash(path) - } - - // Make sure that we don't start with "/" since we add that below. - if path[0] == '/' { - path = path[1:] - } - - return fmt.Sprintf("file:///%s", path) -} diff --git a/helper/url/url.go b/helper/url/url.go new file mode 100644 index 000000000..02497c254 --- /dev/null +++ b/helper/url/url.go @@ -0,0 +1,14 @@ +package url + +import ( + "net/url" +) + +// Parse parses rawURL into a URL structure. +// The rawURL may be relative or absolute. +// +// Parse is a wrapper for the Go stdlib net/url Parse function, but returns +// Windows "safe" URLs on Windows platforms. +func Parse(rawURL string) (*url.URL, error) { + return parse(rawURL) +} diff --git a/helper/url/url_test.go b/helper/url/url_test.go new file mode 100644 index 000000000..ce3226b4b --- /dev/null +++ b/helper/url/url_test.go @@ -0,0 +1,88 @@ +package url + +import ( + "runtime" + "testing" +) + +type parseTest struct { + rawURL string + scheme string + host string + path string + str string + err bool +} + +var parseTests = []parseTest{ + { + rawURL: "/foo/bar", + scheme: "", + host: "", + path: "/foo/bar", + str: "/foo/bar", + err: false, + }, + { + rawURL: "file:///dir/", + scheme: "file", + host: "", + path: "/dir/", + str: "file:///dir/", + err: false, + }, +} + +var winParseTests = []parseTest{ + { + rawURL: `C:\`, + scheme: ``, + host: ``, + path: `C:/`, + str: `C:/`, + err: false, + }, + { + rawURL: `file://C:\`, + scheme: `file`, + host: ``, + path: `C:/`, + str: `file://C:/`, + err: false, + }, + { + rawURL: `file:///C:\`, + scheme: `file`, + host: ``, + path: `C:/`, + str: `file://C:/`, + err: false, + }, +} + +func TestParse(t *testing.T) { + if runtime.GOOS == "windows" { + parseTests = append(parseTests, winParseTests...) + } + for i, pt := range parseTests { + url, err := Parse(pt.rawURL) + if err != nil && !pt.err { + t.Errorf("test %d: unexpected error: %s", i, err) + } + if err == nil && pt.err { + t.Errorf("test %d: expected an error", i) + } + if url.Scheme != pt.scheme { + t.Errorf("test %d: expected Scheme = %q, got %q", i, pt.scheme, url.Scheme) + } + if url.Host != pt.host { + t.Errorf("test %d: expected Host = %q, got %q", i, pt.host, url.Host) + } + if url.Path != pt.path { + t.Errorf("test %d: expected Path = %q, got %q", i, pt.path, url.Path) + } + if url.String() != pt.str { + t.Errorf("test %d: expected url.String() = %q, got %q", i, pt.str, url.String()) + } + } +} diff --git a/helper/url/url_unix.go b/helper/url/url_unix.go new file mode 100644 index 000000000..ed1352a91 --- /dev/null +++ b/helper/url/url_unix.go @@ -0,0 +1,11 @@ +// +build !windows + +package url + +import ( + "net/url" +) + +func parse(rawURL string) (*url.URL, error) { + return url.Parse(rawURL) +} diff --git a/helper/url/url_windows.go b/helper/url/url_windows.go new file mode 100644 index 000000000..4655226f6 --- /dev/null +++ b/helper/url/url_windows.go @@ -0,0 +1,40 @@ +package url + +import ( + "fmt" + "net/url" + "path/filepath" + "strings" +) + +func parse(rawURL string) (*url.URL, error) { + // Make sure we're using "/" since URLs are "/"-based. + rawURL = filepath.ToSlash(rawURL) + + u, err := url.Parse(rawURL) + if err != nil { + return nil, err + } + + if len(rawURL) > 1 && rawURL[1] == ':' { + // Assume we're dealing with a drive letter file path where the drive + // letter has been parsed into the URL Scheme, and the rest of the path + // has been parsed into the URL Path without the leading ':' character. + u.Path = fmt.Sprintf("%s:%s", string(rawURL[0]), u.Path) + u.Scheme = "" + } + + if len(u.Host) > 1 && u.Host[1] == ':' && strings.HasPrefix(rawURL, "file://") { + // Assume we're dealing with a drive letter file path where the drive + // letter has been parsed into the URL Host. + u.Path = fmt.Sprintf("%s%s", u.Host, u.Path) + u.Host = "" + } + + // Remove leading slash for absolute file paths. + if len(u.Path) > 2 && u.Path[0] == '/' && u.Path[2] == ':' { + u.Path = u.Path[1:] + } + + return u, err +}