From a54c54ef24e4ba345e8f479d7f793493aa144563 Mon Sep 17 00:00:00 2001 From: Matt Hook Date: Fri, 26 Aug 2022 11:55:55 +1200 Subject: [PATCH] fix(swarm): fixed issue parsing url with no scheme [EE-4017] (#7502) --- api/agent/version.go | 11 ++---- api/http/handler/endpoints/endpoint_create.go | 3 -- api/http/handler/endpoints/endpoint_update.go | 7 +--- api/http/handler/endpoints/utils.go | 12 ------- api/http/proxy/factory/agent.go | 17 ++------- api/http/proxy/factory/docker.go | 6 ++-- api/internal/url/url.go | 19 ++++++++++ .../environment.service/create.ts | 2 +- .../views/endpoints/edit/endpoint.html | 2 ++ .../shared/AgentForm/AgentForm.validation.tsx | 36 ++++++++++++++++++- .../shared/AgentForm/EnvironmentUrlField.tsx | 2 +- 11 files changed, 67 insertions(+), 50 deletions(-) create mode 100644 api/internal/url/url.go diff --git a/api/agent/version.go b/api/agent/version.go index 0c42022ad..bdb7ebce1 100644 --- a/api/agent/version.go +++ b/api/agent/version.go @@ -5,18 +5,17 @@ import ( "errors" "fmt" "net/http" - netUrl "net/url" "strconv" - "strings" "time" portainer "github.com/portainer/portainer/api" + "github.com/portainer/portainer/api/internal/url" ) // GetAgentVersionAndPlatform returns the agent version and platform // // it sends a ping to the agent and parses the version and platform from the headers -func GetAgentVersionAndPlatform(url string, tlsConfig *tls.Config) (portainer.AgentPlatform, string, error) { +func GetAgentVersionAndPlatform(endpointUrl string, tlsConfig *tls.Config) (portainer.AgentPlatform, string, error) { httpCli := &http.Client{ Timeout: 3 * time.Second, } @@ -27,11 +26,7 @@ func GetAgentVersionAndPlatform(url string, tlsConfig *tls.Config) (portainer.Ag } } - if !strings.Contains(url, "://") { - url = "https://" + url - } - - parsedURL, err := netUrl.Parse(fmt.Sprintf("%s/ping", url)) + parsedURL, err := url.ParseURL(endpointUrl + "/ping") if err != nil { return 0, "", err } diff --git a/api/http/handler/endpoints/endpoint_create.go b/api/http/handler/endpoints/endpoint_create.go index 5b1dde10d..8fe0db2a4 100644 --- a/api/http/handler/endpoints/endpoint_create.go +++ b/api/http/handler/endpoints/endpoint_create.go @@ -259,9 +259,6 @@ func (handler *Handler) createEndpoint(payload *endpointCreatePayload) (*portain endpointType := portainer.DockerEnvironment var agentVersion string if payload.EndpointCreationType == agentEnvironment { - - payload.URL = "tcp://" + normalizeAgentAddress(payload.URL) - var tlsConfig *tls.Config if payload.TLS { tlsConfig, err = crypto.CreateTLSConfigurationFromBytes(payload.TLSCACertFile, payload.TLSCertFile, payload.TLSKeyFile, payload.TLSSkipVerify, payload.TLSSkipClientVerify) diff --git a/api/http/handler/endpoints/endpoint_update.go b/api/http/handler/endpoints/endpoint_update.go index 7e9c71fee..2a52f6f35 100644 --- a/api/http/handler/endpoints/endpoint_update.go +++ b/api/http/handler/endpoints/endpoint_update.go @@ -105,12 +105,7 @@ func (handler *Handler) endpointUpdate(w http.ResponseWriter, r *http.Request) * } if payload.URL != nil { - if endpoint.Type == portainer.AgentOnDockerEnvironment || - endpoint.Type == portainer.AgentOnKubernetesEnvironment { - endpoint.URL = normalizeAgentAddress(*payload.URL) - } else { - endpoint.URL = *payload.URL - } + endpoint.URL = *payload.URL } if payload.PublicURL != nil { diff --git a/api/http/handler/endpoints/utils.go b/api/http/handler/endpoints/utils.go index e991fb8f7..a00f36358 100644 --- a/api/http/handler/endpoints/utils.go +++ b/api/http/handler/endpoints/utils.go @@ -1,18 +1,6 @@ package endpoints -import "strings" - func BoolAddr(b bool) *bool { boolVar := b return &boolVar } - -func normalizeAgentAddress(url string) string { - // Case insensitive strip http or https scheme if URL entered - index := strings.Index(url, "://") - if index >= 0 { - return url[index+3:] - } - - return url -} diff --git a/api/http/proxy/factory/agent.go b/api/http/proxy/factory/agent.go index 74b01d084..630c880fc 100644 --- a/api/http/proxy/factory/agent.go +++ b/api/http/proxy/factory/agent.go @@ -5,14 +5,13 @@ import ( "log" "net" "net/http" - "net/url" - "strings" "github.com/pkg/errors" portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/crypto" "github.com/portainer/portainer/api/http/proxy/factory/agent" "github.com/portainer/portainer/api/internal/endpointutils" + "github.com/portainer/portainer/api/internal/url" ) // ProxyServer provide an extended proxy with a local server to forward requests @@ -34,7 +33,7 @@ func (factory *ProxyFactory) NewAgentProxy(endpoint *portainer.Endpoint) (*Proxy urlString = fmt.Sprintf("http://127.0.0.1:%d", tunnel.Port) } - endpointURL, err := parseURL(urlString) + endpointURL, err := url.ParseURL(urlString) if err != nil { return nil, errors.Wrapf(err, "failed parsing url %s", endpoint.URL) } @@ -99,15 +98,3 @@ func (proxy *ProxyServer) Close() { proxy.server.Close() } } - -// parseURL parses the endpointURL using url.Parse. -// -// to prevent an error when url has port but no protocol prefix -// we add `//` prefix if needed -func parseURL(endpointURL string) (*url.URL, error) { - if !strings.HasPrefix(endpointURL, "http") && !strings.HasPrefix(endpointURL, "tcp") && !strings.HasPrefix(endpointURL, "//") { - endpointURL = fmt.Sprintf("//%s", endpointURL) - } - - return url.Parse(endpointURL) -} diff --git a/api/http/proxy/factory/docker.go b/api/http/proxy/factory/docker.go index 7b2bdb5f3..aca8264c8 100644 --- a/api/http/proxy/factory/docker.go +++ b/api/http/proxy/factory/docker.go @@ -5,13 +5,13 @@ import ( "io" "log" "net/http" - "net/url" "strings" httperror "github.com/portainer/libhttp/error" portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/crypto" "github.com/portainer/portainer/api/http/proxy/factory/docker" + "github.com/portainer/portainer/api/internal/url" ) func (factory *ProxyFactory) newDockerProxy(endpoint *portainer.Endpoint) (http.Handler, error) { @@ -23,7 +23,7 @@ func (factory *ProxyFactory) newDockerProxy(endpoint *portainer.Endpoint) (http. } func (factory *ProxyFactory) newDockerLocalProxy(endpoint *portainer.Endpoint) (http.Handler, error) { - endpointURL, err := url.Parse(endpoint.URL) + endpointURL, err := url.ParseURL(endpoint.URL) if err != nil { return nil, err } @@ -38,7 +38,7 @@ func (factory *ProxyFactory) newDockerHTTPProxy(endpoint *portainer.Endpoint) (h rawURL = fmt.Sprintf("http://127.0.0.1:%d", tunnel.Port) } - endpointURL, err := url.Parse(rawURL) + endpointURL, err := url.ParseURL(rawURL) if err != nil { return nil, err } diff --git a/api/internal/url/url.go b/api/internal/url/url.go new file mode 100644 index 000000000..8d7acb82f --- /dev/null +++ b/api/internal/url/url.go @@ -0,0 +1,19 @@ +package url + +import ( + "fmt" + "net/url" + "strings" +) + +// ParseURL parses the endpointURL using url.Parse. +// +// to prevent an error when url has port but no protocol prefix +// we add `//` prefix if needed +func ParseURL(endpointURL string) (*url.URL, error) { + if !strings.HasPrefix(endpointURL, "http") && !strings.HasPrefix(endpointURL, "tcp") && !strings.HasPrefix(endpointURL, "//") { + endpointURL = fmt.Sprintf("//%s", endpointURL) + } + + return url.Parse(endpointURL) +} diff --git a/app/portainer/environments/environment.service/create.ts b/app/portainer/environments/environment.service/create.ts index 3d87da63c..599cd95d0 100644 --- a/app/portainer/environments/environment.service/create.ts +++ b/app/portainer/environments/environment.service/create.ts @@ -130,7 +130,7 @@ export async function createRemoteEnvironment({ }: CreateRemoteEnvironment) { return createEnvironment(name, creationType, { ...options, - url: `${url}`, + url: `tcp://${url}`, }); } diff --git a/app/portainer/views/endpoints/edit/endpoint.html b/app/portainer/views/endpoints/edit/endpoint.html index ed3dc7b11..bb740d733 100644 --- a/app/portainer/views/endpoints/edit/endpoint.html +++ b/app/portainer/views/endpoints/edit/endpoint.html @@ -110,9 +110,11 @@ Environment URL Environment address +
{ return object({ name: nameValidation(), - environmentUrl: string().required('This field is required.'), + environmentUrl: environmentValidation(), meta: metadataValidation(), gpus: gpusListValidation(), }); } + +function environmentValidation() { + return string() + .required('This field is required') + .test( + 'address', + 'Environment address must be of the form : or :.', + (environmentUrl) => validateAddress(environmentUrl) + ); +} + +export function validateAddress(address: string | undefined) { + if (typeof address === 'undefined') { + return false; + } + + if (address.indexOf('://') > -1) { + return false; + } + + const [host, port] = address.split(':'); + + if ( + host.length === 0 || + Number.isNaN(parseInt(port, 10)) || + port.match(/^[0-9]+$/) == null || + parseInt(port, 10) < 1 || + parseInt(port, 10) > 65535 + ) { + return false; + } + + return true; +} diff --git a/app/react/portainer/environments/wizard/EnvironmentsCreationView/shared/AgentForm/EnvironmentUrlField.tsx b/app/react/portainer/environments/wizard/EnvironmentsCreationView/shared/AgentForm/EnvironmentUrlField.tsx index 7789520eb..d6e009ccb 100644 --- a/app/react/portainer/environments/wizard/EnvironmentsCreationView/shared/AgentForm/EnvironmentUrlField.tsx +++ b/app/react/portainer/environments/wizard/EnvironmentsCreationView/shared/AgentForm/EnvironmentUrlField.tsx @@ -12,7 +12,7 @@ export function EnvironmentUrlField() { errors={meta.error} required inputId="environment-url-field" - tooltip="A host:port combination. The host can be either an IP address or a host name." + tooltip=": or :" >