Skip to content

Commit 2260456

Browse files
committed
OCM-18900 | fix: Improve proxy validation error messages for special characters in passwords
1 parent b18b1c0 commit 2260456

File tree

3 files changed

+78
-10
lines changed

3 files changed

+78
-10
lines changed

cmd/list/dnsdomains/cmd.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,18 +71,19 @@ func run(_ *cobra.Command, _ []string) {
7171
defer r.Cleanup()
7272

7373
r.Reporter.Debugf("Loading dns domains for current org id")
74-
orgID, _, err := r.OCMClient.GetCurrentOrganization()
74+
organizationID, _, err := r.OCMClient.GetCurrentOrganization()
7575
if err != nil {
76-
_ = r.Reporter.Errorf("Failed to get current organization: %s", err)
76+
_ = r.Reporter.Errorf("Failed to get current organization: %v", err)
7777
os.Exit(1)
7878
}
79-
search := fmt.Sprintf("user_defined='true' AND organization.id='%s'", orgID)
79+
80+
search := fmt.Sprintf("user_defined='true' and organization.id='%s'", organizationID)
8081
if args.all {
81-
search = ""
82+
search = fmt.Sprintf("organization.id='%s'", organizationID)
8283
}
8384
dnsDomains, err := r.OCMClient.ListDNSDomains(search)
8485
if err != nil {
85-
r.Reporter.Errorf("Failed to list DNS Domains: %v", err)
86+
_ = r.Reporter.Errorf("Failed to list DNS Domains: %v", err)
8687
os.Exit(1)
8788
}
8889

@@ -93,7 +94,7 @@ func run(_ *cobra.Command, _ []string) {
9394
if output.HasFlag() {
9495
err = output.Print(dnsDomains)
9596
if err != nil {
96-
r.Reporter.Errorf("%s", err)
97+
_ = r.Reporter.Errorf("%s", err)
9798
os.Exit(1)
9899
}
99100
os.Exit(0)

pkg/ocm/helpers.go

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,30 @@ func ValidateHTTPProxy(val interface{}) error {
139139
if httpProxy == "" {
140140
return nil
141141
}
142-
url, err := url.ParseRequestURI(httpProxy)
142+
143+
// Encode password in place
144+
if schemeIdx := strings.Index(httpProxy, "://"); schemeIdx >= 0 {
145+
afterScheme := httpProxy[schemeIdx+3:]
146+
if atIdx := strings.LastIndex(afterScheme, "@"); atIdx >= 0 {
147+
credentials := afterScheme[:atIdx]
148+
if colonIdx := strings.Index(credentials, ":"); colonIdx >= 0 {
149+
username := credentials[:colonIdx]
150+
password := credentials[colonIdx+1:]
151+
httpProxy = httpProxy[:schemeIdx+3] + username + ":" + url.QueryEscape(password) + "@" + afterScheme[atIdx+1:]
152+
}
153+
}
154+
}
155+
156+
parsedURL, err := url.ParseRequestURI(httpProxy)
143157
if err != nil {
144-
return fmt.Errorf("Invalid http-proxy value '%s'", httpProxy)
158+
return fmt.Errorf("Invalid http-proxy value '%s': %s", httpProxy, err)
145159
}
146-
if url.Scheme != "http" {
147-
return errors.Errorf("%s", "Expected http-proxy to have an http:// scheme")
160+
161+
if parsedURL.Scheme != "http" {
162+
if parsedURL.Scheme == "https" {
163+
return errors.Errorf("Invalid http-proxy value '%s': use --https-proxy for HTTPS proxies", httpProxy)
164+
}
165+
return errors.Errorf("Expected http-proxy to have an http:// scheme")
148166
}
149167
return nil
150168
}

pkg/ocm/helpers_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,3 +695,52 @@ var _ = Describe("GetAutoNodeRoleArn", func() {
695695
Expect(exists).To(BeTrue())
696696
})
697697
})
698+
699+
var _ = Describe("ValidateHTTPProxy", func() {
700+
It("accepts valid proxy URL with authentication", func() {
701+
err := ValidateHTTPProxy("http://user:[email protected]:8080")
702+
Expect(err).NotTo(HaveOccurred())
703+
})
704+
705+
It("accepts valid proxy URL without authentication", func() {
706+
err := ValidateHTTPProxy("http://proxy.example.com:8080")
707+
Expect(err).NotTo(HaveOccurred())
708+
})
709+
710+
It("accepts empty string", func() {
711+
err := ValidateHTTPProxy("")
712+
Expect(err).NotTo(HaveOccurred())
713+
})
714+
715+
It("auto-encodes password with forward slash", func() {
716+
err := ValidateHTTPProxy("http://proxyuser:QvoZjyy/[email protected]:8080")
717+
Expect(err).NotTo(HaveOccurred())
718+
})
719+
720+
It("auto-encodes password with @ character", func() {
721+
err := ValidateHTTPProxy("http://user:p@[email protected]:8080")
722+
Expect(err).NotTo(HaveOccurred())
723+
})
724+
725+
It("auto-encodes password with : character", func() {
726+
err := ValidateHTTPProxy("http://user:pa:[email protected]:8080")
727+
Expect(err).NotTo(HaveOccurred())
728+
})
729+
730+
It("accepts password with URL-encoded forward slash", func() {
731+
err := ValidateHTTPProxy("http://proxyuser:QvoZjyy%[email protected]:8080")
732+
Expect(err).NotTo(HaveOccurred())
733+
})
734+
735+
It("rejects HTTPS scheme and suggests using --https-proxy", func() {
736+
err := ValidateHTTPProxy("https://proxy.example.com:8080")
737+
Expect(err).To(HaveOccurred())
738+
Expect(err.Error()).To(ContainSubstring("use --https-proxy"))
739+
})
740+
741+
It("rejects missing protocol scheme", func() {
742+
err := ValidateHTTPProxy("proxy.example.com:8080")
743+
Expect(err).To(HaveOccurred())
744+
Expect(err.Error()).To(ContainSubstring("http:// scheme"))
745+
})
746+
})

0 commit comments

Comments
 (0)