Skip to content

Commit aa528ca

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

File tree

3 files changed

+68
-10
lines changed

3 files changed

+68
-10
lines changed

cmd/list/dnsdomains/cmd.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func init() {
5353
"all",
5454
"a",
5555
false,
56-
"List all DNS domains (default lists just user defined).",
56+
"List all DNS domains across organizations (for support).",
5757
)
5858

5959
flags.BoolVar(
@@ -70,14 +70,19 @@ func run(_ *cobra.Command, _ []string) {
7070
r := rosa.NewRuntime().WithOCM()
7171
defer r.Cleanup()
7272

73-
r.Reporter.Debugf("Loading dns domains for current org id")
74-
search := "user_defined='true'"
73+
organizationID, _, err := r.OCMClient.GetCurrentOrganization()
74+
if err != nil {
75+
_ = r.Reporter.Errorf("Failed to get current organization: %v", err)
76+
os.Exit(1)
77+
}
78+
79+
search := fmt.Sprintf("user_defined='true' and organization.id='%s'", organizationID)
7580
if args.all {
76-
search = ""
81+
search = "user_defined='true'"
7782
}
7883
dnsDomains, err := r.OCMClient.ListDNSDomains(search)
7984
if err != nil {
80-
r.Reporter.Errorf("Failed to list DNS Domains: %v", err)
85+
_ = r.Reporter.Errorf("Failed to list DNS Domains: %v", err)
8186
os.Exit(1)
8287
}
8388

@@ -88,7 +93,7 @@ func run(_ *cobra.Command, _ []string) {
8893
if output.HasFlag() {
8994
err = output.Print(dnsDomains)
9095
if err != nil {
91-
r.Reporter.Errorf("%s", err)
96+
_ = r.Reporter.Errorf("%s", err)
9297
os.Exit(1)
9398
}
9499
os.Exit(0)

pkg/ocm/helpers.go

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,25 @@ func ValidateHTTPProxy(val interface{}) error {
139139
if httpProxy == "" {
140140
return nil
141141
}
142-
url, err := url.ParseRequestURI(httpProxy)
142+
parsedURL, err := url.ParseRequestURI(httpProxy)
143143
if err != nil {
144-
return fmt.Errorf("Invalid http-proxy value '%s'", httpProxy)
144+
errMsg := err.Error()
145+
if strings.Contains(errMsg, "invalid port") && strings.Contains(httpProxy, "@") {
146+
return fmt.Errorf("Invalid http-proxy value '%s': password contains special characters that must be URL-encoded (e.g., '/' as '%%2F', '#' as '%%23')",
147+
httpProxy)
148+
}
149+
if !strings.Contains(httpProxy, "://") {
150+
return fmt.Errorf("Invalid http-proxy value '%s': missing protocol scheme. URL must start with 'http://'",
151+
httpProxy)
152+
}
153+
// Show the actual parsing error for other cases
154+
return fmt.Errorf("Invalid http-proxy value '%s': %s", httpProxy, err)
145155
}
146-
if url.Scheme != "http" {
147-
return errors.Errorf("%s", "Expected http-proxy to have an http:// scheme")
156+
if parsedURL.Scheme != "http" {
157+
if parsedURL.Scheme == "https" {
158+
return errors.Errorf("Invalid http-proxy value '%s': use --https-proxy for HTTPS proxies", httpProxy)
159+
}
160+
return errors.Errorf("Expected http-proxy to have an http:// scheme")
148161
}
149162
return nil
150163
}

pkg/ocm/helpers_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,3 +695,43 @@ 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("rejects password with forward slash and provides helpful error", func() {
716+
err := ValidateHTTPProxy("http://proxyuser:QvoZjyy/[email protected]:8080")
717+
Expect(err).To(HaveOccurred())
718+
Expect(err.Error()).To(ContainSubstring("must be URL-encoded"))
719+
})
720+
721+
It("accepts password with URL-encoded forward slash", func() {
722+
err := ValidateHTTPProxy("http://proxyuser:QvoZjyy%[email protected]:8080")
723+
Expect(err).NotTo(HaveOccurred())
724+
})
725+
726+
It("rejects HTTPS scheme and suggests using --https-proxy", func() {
727+
err := ValidateHTTPProxy("https://proxy.example.com:8080")
728+
Expect(err).To(HaveOccurred())
729+
Expect(err.Error()).To(ContainSubstring("use --https-proxy"))
730+
})
731+
732+
It("rejects missing protocol scheme", func() {
733+
err := ValidateHTTPProxy("proxy.example.com:8080")
734+
Expect(err).To(HaveOccurred())
735+
Expect(err.Error()).To(ContainSubstring("http:// scheme"))
736+
})
737+
})

0 commit comments

Comments
 (0)