pr comments

coder · f0ssel · Apr 26, 2024 · Apr 11, 2024 · Apr 11, 2024 · Apr 11, 2024
commit bbbd3c2e76a3e17438cb001184d8322e07f1b538
diff --git a/coderd/tailnet.go b/coderd/tailnet.go
@@ -4,6 +4,7 @@ import (
 	"bufio"
 	"context"
 	"crypto/tls"
+	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -362,23 +363,27 @@ func (s *ServerTailnet) ReverseProxy(targetURL, dashboardURL *url.URL, agentID u
 			switchTarget = ""
 		)
 
-		if strings.Contains(theErr.Error(), "tls:") {
+		var tlsError tls.RecordHeaderError
+		if (errors.As(theErr, &tlsError) && tlsError.Msg == "first record does not look like a TLS handshake") ||
+			errors.Is(theErr, http.ErrSchemeMismatch) {
 			// If the error is due to an HTTP/HTTPS mismatch, we can provide a
 			// more helpful error message with redirect buttons.
 			switchURL := url.URL{
 				Scheme: dashboardURL.Scheme,
 			}
-			if app.IsPort() {
-				if app.PortProtocol() == "https" {
-					app.ChangePortProtocol("http")
-				} else {
-					app.ChangePortProtocol("https")
+			_, protocol, isPort := app.PortInfo()
+			if isPort {
+				if protocol == "https" {
+					app = app.ChangePortProtocol("http")
+				}
+				if protocol == "http" {
+					app = app.ChangePortProtocol("https")
 				}
 
 				switchURL.Host = fmt.Sprintf("%s%s", app.String(), strings.TrimPrefix(wildcardHostname, "*"))
 				switchLink = switchURL.String()
-				switchTarget = app.PortProtocol()
-				additional += fmt.Sprintf("This error seems to be due to an app protocol mismatch, try switching to %s.", strings.ToUpper(app.PortProtocol()))
+				switchTarget = protocol
+				additional += fmt.Sprintf("This error seems to be due to an app protocol mismatch, try switching to %s.", strings.ToUpper(protocol))
 			}
 		}
 
@@ -388,9 +393,9 @@ func (s *ServerTailnet) ReverseProxy(targetURL, dashboardURL *url.URL, agentID u
 			Description:          desc,
 			RetryEnabled:         true,
 			DashboardURL:         dashboardURL.String(),
-			SwitchProtocolLink:   switchLink,
-			SwitchProtocolTarget: strings.ToUpper(switchTarget),
 			AdditionalInfo:       additional,
+			AdditionalButtonLink: switchLink,
+			AdditionalButtonText: fmt.Sprintf("Switch to %s", strings.ToUpper(switchTarget)),
 		})
 	}
 	proxy.Director = s.director(agentID, proxy.Director)

diff --git a/coderd/workspaceapps/appurl/appurl.go b/coderd/workspaceapps/appurl/appurl.go
@@ -84,62 +84,87 @@ func (a ApplicationURL) Path() string {
 	return fmt.Sprintf("/@%s/%s.%s/apps/%s", a.Username, a.WorkspaceName, a.AgentName, a.AppSlugOrPort)
 }
 
-func (a ApplicationURL) IsPort() bool {
-	// check if https port
-	if strings.HasSuffix(a.AppSlugOrPort, "s") {
-		trimmed := strings.TrimSuffix(a.AppSlugOrPort, "s")
-		_, err := strconv.ParseInt(trimmed, 10, 64)
-		//nolint:gosimple
-		if err != nil {
-			return false
-		}
-
-		return true
-	}
-
-	// check if port at all
-	_, err := strconv.ParseInt(a.AppSlugOrPort, 10, 64)
-	//nolint:gosimple
-	if err != nil {
-		return false
-	}
-
-	return true
-}
+// func (a ApplicationURL) IsPort() bool {
+// 	// check if https port
+// 	if strings.HasSuffix(a.AppSlugOrPort, "s") {
+// 		trimmed := strings.TrimSuffix(a.AppSlugOrPort, "s")
+// 		_, err := strconv.ParseInt(trimmed, 10, 64)
+// 		//nolint:gosimple
+// 		if err != nil {
+// 			return false
+// 		}
+
+// 		return true
+// 	}
+
+// 	// check if port at all
+// 	_, err := strconv.ParseInt(a.AppSlugOrPort, 10, 64)
+// 	//nolint:gosimple
+// 	if err != nil {
+// 		return false
+// 	}
+
+// 	return true
+// }
+
+// func (a ApplicationURL) PortProtocol() string {
+// 	if strings.HasSuffix(a.AppSlugOrPort, "s") {
+// 		trimmed := strings.TrimSuffix(a.AppSlugOrPort, "s")
+// 		_, err := strconv.ParseInt(trimmed, 10, 64)
+// 		if err == nil {
+// 			return "https"
+// 		}
+// 	}
+
+// 	return "http"
+// }
+
+func (a ApplicationURL) PortInfo() (uint, string, bool) {
+	var (
+		port     uint64
+		protocol string
+		isPort   bool
+		err      error
+	)
 
-func (a ApplicationURL) PortProtocol() string {
 	if strings.HasSuffix(a.AppSlugOrPort, "s") {
 		trimmed := strings.TrimSuffix(a.AppSlugOrPort, "s")
-		_, err := strconv.ParseInt(trimmed, 10, 64)
+		port, err = strconv.ParseUint(trimmed, 10, 16)
+		if err == nil {
+			protocol = "https"
+			isPort = true
+		}
+	} else {
+		port, err = strconv.ParseUint(a.AppSlugOrPort, 10, 16)
 		if err == nil {
-			return "https"
+			protocol = "http"
+			isPort = true
 		}
 	}
 
-	return "http"
+	return uint(port), protocol, isPort
 }
 
-func (a *ApplicationURL) ChangePortProtocol(target string) {
-	if target == "http" {
-		if a.PortProtocol() == "http" {
-			return
-		}
-		trimmed := strings.TrimSuffix(a.AppSlugOrPort, "s")
-		_, err := strconv.ParseInt(trimmed, 10, 64)
-		if err == nil {
-			a.AppSlugOrPort = trimmed
-		}
+func (a *ApplicationURL) ChangePortProtocol(target string) ApplicationURL {
+	newAppURL := *a
+	port, protocol, isPort := a.PortInfo()
+	if !isPort {
+		return newAppURL
+	}
+
+	if target == protocol {
+		return newAppURL
 	}
 
 	if target == "https" {
-		if a.PortProtocol() == "https" {
-			return
-		}
-		_, err := strconv.ParseInt(a.AppSlugOrPort, 10, 64)
-		if err == nil {
-			a.AppSlugOrPort = fmt.Sprintf("%s%s", a.AppSlugOrPort, "s")
-		}
+		newAppURL.AppSlugOrPort = fmt.Sprintf("%ds", port)
 	}
+
+	if target == "http" {
+		newAppURL.AppSlugOrPort = fmt.Sprintf("%d", port)
+	}
+
+	return newAppURL
 }
 
 // ParseSubdomainAppURL parses an ApplicationURL from the given subdomain. If

diff --git a/coderd/workspaceapps/proxy.go b/coderd/workspaceapps/proxy.go
@@ -545,8 +545,9 @@ func (s *Server) proxyWorkspaceApp(rw http.ResponseWriter, r *http.Request, appT
 
 	r.URL.Path = path
 	appURL.RawQuery = ""
-	if app.IsPort() {
-		appURL.Scheme = app.PortProtocol()
+	_, protocol, isPort := app.PortInfo()
+	if isPort {
+		appURL.Scheme = protocol
 	}
 
 	proxy := s.AgentProvider.ReverseProxy(appURL, s.DashboardURL, appToken.AgentID, app, s.Hostname)

@@ -792,9 +792,9 @@ type ErrorPageData struct {
 	RetryEnabled         bool
 	DashboardURL         string
 	Warnings             []string
-	SwitchProtocolLink   string
-	SwitchProtocolTarget string
 	AdditionalInfo       string
+	AdditionalButtonLink string
+	AdditionalButtonText string
 
 	RenderDescriptionMarkdown bool
 }

@@ -198,12 +198,12 @@ <h3>Warnings</h3>
       </div>
       {{ end }}
       <div class="button-group">
-        {{- if .Error.RetryEnabled }}
-        <button onclick="window.location.reload()">Retry</button>
-        {{ end }} {{- if .Error.SwitchProtocolLink }}
-        <a href="{{ .Error.SwitchProtocolLink }}"
-          >Switch to {{ .Error.SwitchProtocolTarget }}</a
+        {{- if and .Error.AdditionalButtonText .Error.AdditionButtonLink }}
+        <a href="{{ .Error.AdditionButtonLink }}"
+          >Switch to {{ .Error.AdditionalButtonText }}</a
         >
+        {{ end }} {{- if .Error.RetryEnabled }}
+        <button onclick="window.location.reload()">Retry</button>
         {{ end }}
         <a href="{{ .Error.DashboardURL }}">Back to site</a>
       </div>