From 160e4f926e8656737f99927c41b106040e01d24b Mon Sep 17 00:00:00 2001 From: Emil Hessman Date: Tue, 27 Jan 2015 07:52:48 +0100 Subject: [PATCH 1/4] config/module: fix panic on Windows when running tests On Windows, Go returns paths with the `\` path separator. Make sure we are using `/` even on Windows since URLs are `/`-based. --- config/module/module_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/module/module_test.go b/config/module/module_test.go index bd72f08629..c63fa02012 100644 --- a/config/module/module_test.go +++ b/config/module/module_test.go @@ -42,7 +42,7 @@ func testModule(n string) string { var url url.URL url.Scheme = "file" - url.Path = p + url.Path = filepath.ToSlash(p) return url.String() } From 74cf8fcabd075553600e132f67aef8f9f1271677 Mon Sep 17 00:00:00 2001 From: Emil Hessman Date: Tue, 27 Jan 2015 08:03:12 +0100 Subject: [PATCH 2/4] config/module: adjust FileDetector tests for Windows "/foo" is not an absolute path on Windows. Adjust the FileDetector tests to take that into account when verifying the results. Fixes FileDetector test failures on Windows. --- config/module/detect_file.go | 4 +- config/module/detect_file_test.go | 82 ++++++++++++++++++++----------- 2 files changed, 56 insertions(+), 30 deletions(-) diff --git a/config/module/detect_file.go b/config/module/detect_file.go index ac9ad8e678..68ff439133 100644 --- a/config/module/detect_file.go +++ b/config/module/detect_file.go @@ -13,8 +13,6 @@ func (d *FileDetector) Detect(src, pwd string) (string, bool, error) { return "", false, nil } - // Make sure we're using "/" even on Windows. URLs are "/"-based. - src = filepath.ToSlash(src) if !filepath.IsAbs(src) { if pwd == "" { return "", true, fmt.Errorf( @@ -23,6 +21,8 @@ func (d *FileDetector) Detect(src, pwd string) (string, bool, error) { src = filepath.Join(pwd, src) } + // Make sure we're using "/" even on Windows. URLs are "/"-based. + src = filepath.ToSlash(src) // Make sure that we don't start with "/" since we add that below if src[0] == '/' { diff --git a/config/module/detect_file_test.go b/config/module/detect_file_test.go index 02a6ccec28..d20271bd63 100644 --- a/config/module/detect_file_test.go +++ b/config/module/detect_file_test.go @@ -1,25 +1,42 @@ package module import ( + "runtime" "testing" ) +type fileTest struct { + in, pwd, out string + err bool +} + +var fileTests = []fileTest{ + {"./foo", "/pwd", "file:///pwd/foo", false}, + {"./foo?foo=bar", "/pwd", "file:///pwd/foo?foo=bar", false}, + {"foo", "/pwd", "file:///pwd/foo", false}, +} + +var unixFileTests = []fileTest{ + {"/foo", "/pwd", "file:///foo", false}, + {"/foo?bar=baz", "/pwd", "file:///foo?bar=baz", false}, +} + +var winFileTests = []fileTest{ + {"/foo", "/pwd", "file:///pwd/foo", false}, + {`C:\`, `/pwd`, `file:///C:/`, false}, + {`C:\?bar=baz`, `/pwd`, `file:///C:/?bar=baz`, false}, +} + func TestFileDetector(t *testing.T) { - cases := []struct { - Input string - Output string - }{ - {"./foo", "file:///pwd/foo"}, - {"./foo?foo=bar", "file:///pwd/foo?foo=bar"}, - {"foo", "file:///pwd/foo"}, - {"/foo", "file:///foo"}, - {"/foo?bar=baz", "file:///foo?bar=baz"}, + if runtime.GOOS == "windows" { + fileTests = append(fileTests, winFileTests...) + } else { + fileTests = append(fileTests, unixFileTests...) } - pwd := "/pwd" f := new(FileDetector) - for i, tc := range cases { - output, ok, err := f.Detect(tc.Input, pwd) + for i, tc := range fileTests { + out, ok, err := f.Detect(tc.in, tc.pwd) if err != nil { t.Fatalf("err: %s", err) } @@ -27,36 +44,45 @@ func TestFileDetector(t *testing.T) { t.Fatal("not ok") } - if output != tc.Output { - t.Fatalf("%d: bad: %#v", i, output) + if out != tc.out { + t.Fatalf("%d: bad: %#v", i, out) } } } +var noPwdFileTests = []fileTest{ + {in: "./foo", pwd: "", out: "", err: true}, + {in: "foo", pwd: "", out: "", err: true}, +} + +var noPwdUnixFileTests = []fileTest{ + {in: "/foo", pwd: "", out: "file:///foo", err: false}, +} + +var noPwdWinFileTests = []fileTest{ + {in: "/foo", pwd: "", out: "", err: true}, + {in: `C:\`, pwd: ``, out: `file:///C:/`, err: false}, +} + func TestFileDetector_noPwd(t *testing.T) { - cases := []struct { - Input string - Output string - Err bool - }{ - {"./foo", "", true}, - {"foo", "", true}, - {"/foo", "file:///foo", false}, + if runtime.GOOS == "windows" { + noPwdFileTests = append(noPwdFileTests, noPwdWinFileTests...) + } else { + noPwdFileTests = append(noPwdFileTests, noPwdUnixFileTests...) } - pwd := "" f := new(FileDetector) - for i, tc := range cases { - output, ok, err := f.Detect(tc.Input, pwd) - if (err != nil) != tc.Err { + for i, tc := range noPwdFileTests { + out, ok, err := f.Detect(tc.in, tc.pwd) + if (err != nil) != tc.err { t.Fatalf("%d: err: %s", i, err) } if !ok { t.Fatal("not ok") } - if output != tc.Output { - t.Fatalf("%d: bad: %#v", i, output) + if out != tc.out { + t.Fatalf("%d: bad: %#v", i, out) } } } From 78d1fc742f728b4a38e4044a1d1625f63c46cb14 Mon Sep 17 00:00:00 2001 From: Emil Hessman Date: Tue, 27 Jan 2015 15:12:37 +0100 Subject: [PATCH 3/4] config/module: adjust HttpGetter test to fix Windows test failure Specify laddr on the form host:port in the call to net.Listen as documented for net.Dial, see godoc.org/net#Dial Fixes the following test failures on Windows: > go test -run=TestHttpGetter --- FAIL: TestHttpGetter_header (0.00s) get_http_test.go:31: err: Get http://[::]:52101/header?terraform-get=1: dial tcp [::]:52101: ConnectEx tcp: The requested address is not valid in its context. --- FAIL: TestHttpGetter_meta (0.00s) get_http_test.go:55: err: Get http://[::]:52103/meta?terraform-get=1: dial tcp [::]:52103: ConnectEx tcp: The requested address is not valid in its context. --- FAIL: TestHttpGetter_metaSubdir (0.00s) get_http_test.go:79: err: Get http://[::]:52105/meta-subdir?terraform-get=1: dial tcp [::]:52105: ConnectEx tcp: The requested address is not valid in its context. FAIL exit status 1 FAIL github.com/hashicorp/terraform/config/module 0.054s --- config/module/get_http_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/module/get_http_test.go b/config/module/get_http_test.go index 5eb83a6192..5f2590f481 100644 --- a/config/module/get_http_test.go +++ b/config/module/get_http_test.go @@ -105,7 +105,7 @@ func TestHttpGetter_none(t *testing.T) { } func testHttpServer(t *testing.T) net.Listener { - ln, err := net.Listen("tcp", ":0") + ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { t.Fatalf("err: %s", err) } From d5a49363d73b57a2cea81302cf4b008ba5eabc95 Mon Sep 17 00:00:00 2001 From: Emil Hessman Date: Tue, 27 Jan 2015 22:19:49 +0100 Subject: [PATCH 4/4] config/module: handle absolute file paths on Windows Using url.Parse to parse an absolute file path on Windows yields a URL type where the Path element is prefixed by a slash. For example, parsing "file:///C:/Users/user" gives a URL type with Path:"/C:/Users/user". According to golang.org/issue/6027, the parsing is correct as is. The leading slash on the Path must be eliminated before any file operations. This commit introduces a urlParse function which wraps the url.Parse functionality and removes the leading slash in Path for absolute file paths on Windows. Fixes config/module test failures on Windows. --- config/module/detect_file.go | 10 +------- config/module/get.go | 2 +- config/module/module_test.go | 8 ++---- config/module/url_helper.go | 50 ++++++++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 16 deletions(-) create mode 100644 config/module/url_helper.go diff --git a/config/module/detect_file.go b/config/module/detect_file.go index 68ff439133..4aa21ffa29 100644 --- a/config/module/detect_file.go +++ b/config/module/detect_file.go @@ -21,13 +21,5 @@ func (d *FileDetector) Detect(src, pwd string) (string, bool, error) { src = filepath.Join(pwd, src) } - // Make sure we're using "/" even on Windows. URLs are "/"-based. - src = filepath.ToSlash(src) - - // Make sure that we don't start with "/" since we add that below - if src[0] == '/' { - src = src[1:] - } - - return fmt.Sprintf("file:///%s", src), true, nil + return fmtFileURL(src), true, nil } diff --git a/config/module/get.go b/config/module/get.go index bf7cf1accc..abc724b581 100644 --- a/config/module/get.go +++ b/config/module/get.go @@ -72,7 +72,7 @@ func Get(dst, src string) error { dst = tmpDir } - u, err := url.Parse(src) + u, err := urlParse(src) if err != nil { return err } diff --git a/config/module/module_test.go b/config/module/module_test.go index c63fa02012..88a595cd39 100644 --- a/config/module/module_test.go +++ b/config/module/module_test.go @@ -39,15 +39,11 @@ func testModule(n string) string { if err != nil { panic(err) } - - var url url.URL - url.Scheme = "file" - url.Path = filepath.ToSlash(p) - return url.String() + return fmtFileURL(p) } func testModuleURL(n string) *url.URL { - u, err := url.Parse(testModule(n)) + u, err := urlParse(testModule(n)) if err != nil { panic(err) } diff --git a/config/module/url_helper.go b/config/module/url_helper.go new file mode 100644 index 0000000000..061496270a --- /dev/null +++ b/config/module/url_helper.go @@ -0,0 +1,50 @@ +package module + +import ( + "fmt" + "net/url" + "path/filepath" + "runtime" +) + +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 u.Scheme != "file" { + return u, err + } + + // 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) +}