From 393b2d16c469dff09a3e010e5899ba6da2b18878 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 15 Jul 2020 12:10:09 -0700 Subject: [PATCH] switch to using net/url.URL struct for encoding rather than the incorrect QueryEscape for the whole URL. --- post-processor/vsphere/post-processor.go | 50 ++++++++-------- post-processor/vsphere/post-processor_test.go | 59 ++++++------------- 2 files changed, 44 insertions(+), 65 deletions(-) diff --git a/post-processor/vsphere/post-processor.go b/post-processor/vsphere/post-processor.go index 7b45c6a9e..35722bfa9 100644 --- a/post-processor/vsphere/post-processor.go +++ b/post-processor/vsphere/post-processor.go @@ -115,11 +115,9 @@ func (p *PostProcessor) Configure(raws ...interface{}) error { return nil } -func (p *PostProcessor) generateURI() string { - password := escapeWithSpaces(p.config.Password) - ovftool_uri := fmt.Sprintf("vi://%s:%s@%s/%s/host/%s", - escapeWithSpaces(p.config.Username), - password, +func (p *PostProcessor) generateURI() (*url.URL, error) { + // use net/url lib to encode and escape url elements + ovftool_uri := fmt.Sprintf("vi://%s/%s/host/%s", p.config.Host, p.config.Datacenter, p.config.Cluster) @@ -128,14 +126,20 @@ func (p *PostProcessor) generateURI() string { ovftool_uri += "/Resources/" + p.config.ResourcePool } + u, err := url.Parse(ovftool_uri) + if err != nil { + return nil, fmt.Errorf("Couldn't generate uri for ovftool: %s", err) + } + u.User = url.UserPassword(p.config.Username, p.config.Password) + if p.config.ESXiHost != "" { if ipv4Regex.MatchString(p.config.ESXiHost) { - ovftool_uri += "/?ip=" + p.config.ESXiHost + u.RawQuery = "ip=" + url.QueryEscape(p.config.ESXiHost) } else if hostnameRegex.MatchString(p.config.ESXiHost) { - ovftool_uri += "/?dns=" + p.config.ESXiHost + u.RawQuery = "dns=" + url.QueryEscape(p.config.ESXiHost) } } - return ovftool_uri + return u, nil } func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact packer.Artifact) (packer.Artifact, bool, bool, error) { @@ -151,9 +155,12 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact return nil, false, false, fmt.Errorf("VMX, OVF or OVA file not found") } - ovftool_uri := p.generateURI() + ovftool_uri, err := p.generateURI() + if err != nil { + return nil, false, false, err + } - args, err := p.BuildArgs(source, ovftool_uri) + args, err := p.BuildArgs(source, ovftool_uri.String()) if err != nil { ui.Message(fmt.Sprintf("Failed: %s\n", err)) } @@ -161,7 +168,7 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact ui.Message(fmt.Sprintf("Uploading %s to vSphere", source)) log.Printf("Starting ovftool with parameters: %s", - p.filterLog(strings.Join(args, " "))) + filterLog(strings.Join(args, " "), ovftool_uri)) var errWriter io.Writer var errOut bytes.Buffer @@ -171,20 +178,24 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact cmd.Stderr = errWriter if err := cmd.Run(); err != nil { - err := fmt.Errorf("Error uploading virtual machine: %s\n%s\n", err, p.filterLog(errOut.String())) + err := fmt.Errorf("Error uploading virtual machine: %s\n%s\n", err, filterLog(errOut.String(), ovftool_uri)) return nil, false, false, err } - ui.Message(p.filterLog(errOut.String())) + ui.Message(filterLog(errOut.String(), ovftool_uri)) artifact = NewArtifact(p.config.Datastore, p.config.VMFolder, p.config.VMName, artifact.Files()) return artifact, false, false, nil } -func (p *PostProcessor) filterLog(s string) string { - password := escapeWithSpaces(p.config.Password) - return strings.Replace(s, password, "", -1) +func filterLog(s string, u *url.URL) string { + password, passwordSet := u.User.Password() + if passwordSet { + return strings.Replace(s, password, "", -1) + } + + return s } func (p *PostProcessor) BuildArgs(source, ovftool_uri string) ([]string, error) { @@ -223,10 +234,3 @@ func (p *PostProcessor) BuildArgs(source, ovftool_uri string) ([]string, error) return args, nil } - -// Encode everything except for + signs -func escapeWithSpaces(stringToEscape string) string { - escapedString := url.QueryEscape(stringToEscape) - escapedString = strings.Replace(escapedString, "+", `%20`, -1) - return escapedString -} diff --git a/post-processor/vsphere/post-processor_test.go b/post-processor/vsphere/post-processor_test.go index 7b9351ec6..8a14357d3 100644 --- a/post-processor/vsphere/post-processor_test.go +++ b/post-processor/vsphere/post-processor_test.go @@ -52,9 +52,12 @@ func TestGenerateURI_Basic(t *testing.T) { p.config = getTestConfig() - uri := p.generateURI() + uri, err := p.generateURI() + if err != nil { + t.Fatalf("had error: %s", err) + } expected_uri := "vi://me:notpassword@myhost/mydc/host/mycluster" - if uri != expected_uri { + if uri.String() != expected_uri { t.Fatalf("URI did not match. Recieved: %s. Expected: %s", uri, expected_uri) } } @@ -68,17 +71,17 @@ func TestGenerateURI_PasswordEscapes(t *testing.T) { cases := []escapeCases{ {`this has spaces`, `this%20has%20spaces`}, {`exclaimation_!`, `exclaimation_%21`}, - {`hash_#_dollar_$`, `hash_%23_dollar_%24`}, - {`ampersand_&awesome`, `ampersand_%26awesome`}, + {`hash_#_dollar_$`, `hash_%23_dollar_$`}, + {`ampersand_&awesome`, `ampersand_&awesome`}, {`single_quote_'_and_another_'`, `single_quote_%27_and_another_%27`}, {`open_paren_(_close_paren_)`, `open_paren_%28_close_paren_%29`}, - {`asterisk_*_plus_+`, `asterisk_%2A_plus_%2B`}, - {`comma_,slash_/`, `comma_%2Cslash_%2F`}, - {`colon_:semicolon_;`, `colon_%3Asemicolon_%3B`}, - {`equal_=question_?`, `equal_%3Dquestion_%3F`}, + {`asterisk_*_plus_+`, `asterisk_%2A_plus_+`}, + {`comma_,slash_/`, `comma_,slash_%2F`}, + {`colon_:semicolon_;`, `colon_%3Asemicolon_;`}, + {`equal_=question_?`, `equal_=question_%3F`}, {`at_@`, `at_%40`}, {`open_bracket_[closed_bracket]`, `open_bracket_%5Bclosed_bracket%5D`}, - {`user:password with $paces@host/name.foo`, `user%3Apassword%20with%20%24paces%40host%2Fname.foo`}, + {`user:password with $paces@host/name.foo`, `user%3Apassword%20with%20$paces%40host%2Fname.foo`}, } for _, escapeCase := range cases { @@ -87,42 +90,14 @@ func TestGenerateURI_PasswordEscapes(t *testing.T) { p.config = getTestConfig() p.config.Password = escapeCase.Input - uri := p.generateURI() + uri, err := p.generateURI() + if err != nil { + t.Fatalf("had error: %s", err) + } expected_uri := fmt.Sprintf("vi://me:%s@myhost/mydc/host/mycluster", escapeCase.Expected) - if uri != expected_uri { + if uri.String() != expected_uri { t.Fatalf("URI did not match. Recieved: %s. Expected: %s", uri, expected_uri) } } } - -func TestEscaping(t *testing.T) { - type escapeCases struct { - Input string - Expected string - } - - cases := []escapeCases{ - {`this has spaces`, `this%20has%20spaces`}, - {`exclaimation_!`, `exclaimation_%21`}, - {`hash_#_dollar_$`, `hash_%23_dollar_%24`}, - {`ampersand_&awesome`, `ampersand_%26awesome`}, - {`single_quote_'_and_another_'`, `single_quote_%27_and_another_%27`}, - {`open_paren_(_close_paren_)`, `open_paren_%28_close_paren_%29`}, - {`asterisk_*_plus_+`, `asterisk_%2A_plus_%2B`}, - {`comma_,slash_/`, `comma_%2Cslash_%2F`}, - {`colon_:semicolon_;`, `colon_%3Asemicolon_%3B`}, - {`equal_=question_?`, `equal_%3Dquestion_%3F`}, - {`at_@`, `at_%40`}, - {`open_bracket_[closed_bracket]`, `open_bracket_%5Bclosed_bracket%5D`}, - {`user:password with $paces@host/name.foo`, `user%3Apassword%20with%20%24paces%40host%2Fname.foo`}, - } - for _, escapeCase := range cases { - received := escapeWithSpaces(escapeCase.Input) - - if escapeCase.Expected != received { - t.Errorf("Error escaping URL; expected %s got %s", escapeCase.Expected, received) - } - } - -}