fix: error if protocol isn't specified in --access-url

f0ssel · f0ssel · commit 97f37f14a612 · 2022-11-01T15:03:01.000Z
diff --git a/cli/server.go b/cli/server.go
@@ -225,7 +225,7 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
 				cfg.AccessURL.Value = tunnel.URL
 
 				if cfg.WildcardAccessURL.Value == "" {
-					u, err := parseURL(ctx, tunnel.URL)
+					u, err := parseURL(tunnel.URL)
 					if err != nil {
 						return xerrors.Errorf("parse tunnel url: %w", err)
 					}
@@ -235,7 +235,7 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
 				}
 			}
 
-			accessURLParsed, err := parseURL(ctx, cfg.AccessURL.Value)
+			accessURLParsed, err := parseURL(cfg.AccessURL.Value)
 			if err != nil {
 				return xerrors.Errorf("parse URL: %w", err)
 			}
@@ -469,7 +469,7 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
 			}
 
 			// Parse the raw telemetry URL!
-			telemetryURL, err := parseURL(ctx, cfg.Telemetry.URL.Value)
+			telemetryURL, err := parseURL(cfg.Telemetry.URL.Value)
 			if err != nil {
 				return xerrors.Errorf("parse telemetry url: %w", err)
 			}
@@ -779,34 +779,21 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
 	return root
 }
 
-// parseURL parses a string into a URL. It works around some technically correct
-// but undesired behavior of url.Parse by prepending a scheme if one does not
-// exist so that the URL does not get parsed improperly.
-func parseURL(ctx context.Context, u string) (*url.URL, error) {
+// parseURL parses a string into a URL.
+func parseURL(u string) (*url.URL, error) {
 	var (
 		hasScheme = strings.HasPrefix(u, "http:") || strings.HasPrefix(u, "https:")
 	)
 
 	if !hasScheme {
-		// Append a scheme if it doesn't have one. Otherwise the hostname
-		// will likely get parsed as the scheme and cause methods like Hostname()
-		// to return an empty string, largely obviating the purpose of this
-		// function.
-		u = "https://" + u
+		return nil, xerrors.Errorf("URL %q must have a scheme of either http or https", u)
 	}
 
 	parsed, err := url.Parse(u)
 	if err != nil {
 		return nil, err
 	}
 
-	// If the specified url is a loopback device and no scheme has been
-	// specified, prefer http over https. It's unlikely anyone intends to use
-	// https on a loopback and if they do they can specify a scheme.
-	if local, _ := isLocalURL(ctx, parsed); local && !hasScheme {
-		parsed.Scheme = "http"
-	}
-
 	return parsed, nil
 }
 

Original file line number	Diff line number	Diff line change
`@@ -225,7 +225,7 @@ func Server(vip viper.Viper, newAPI func(context.Context, coderd.Options) (*co`
`225`	`225`	`cfg.AccessURL.Value = tunnel.URL`
`226`	`226`
`227`	`227`	`if cfg.WildcardAccessURL.Value == "" {`
`228`		`- u, err := parseURL(ctx, tunnel.URL)`
	`228`	`+ u, err := parseURL(tunnel.URL)`
`229`	`229`	`if err != nil {`
`230`	`230`	`return xerrors.Errorf("parse tunnel url: %w", err)`
`231`	`231`	`}`
`@@ -235,7 +235,7 @@ func Server(vip viper.Viper, newAPI func(context.Context, coderd.Options) (*co`
`235`	`235`	`}`
`236`	`236`	`}`
`237`	`237`
`238`		`- accessURLParsed, err := parseURL(ctx, cfg.AccessURL.Value)`
	`238`	`+ accessURLParsed, err := parseURL(cfg.AccessURL.Value)`
`239`	`239`	`if err != nil {`
`240`	`240`	`return xerrors.Errorf("parse URL: %w", err)`
`241`	`241`	`}`
`@@ -469,7 +469,7 @@ func Server(vip viper.Viper, newAPI func(context.Context, coderd.Options) (*co`
`469`	`469`	`}`
`470`	`470`
`471`	`471`	`// Parse the raw telemetry URL!`
`472`		`- telemetryURL, err := parseURL(ctx, cfg.Telemetry.URL.Value)`
	`472`	`+ telemetryURL, err := parseURL(cfg.Telemetry.URL.Value)`
`473`	`473`	`if err != nil {`
`474`	`474`	`return xerrors.Errorf("parse telemetry url: %w", err)`
`475`	`475`	`}`
`@@ -779,34 +779,21 @@ func Server(vip viper.Viper, newAPI func(context.Context, coderd.Options) (*co`
`779`	`779`	`return root`
`780`	`780`	`}`
`781`	`781`
`782`		`-// parseURL parses a string into a URL. It works around some technically correct`
`783`		`-// but undesired behavior of url.Parse by prepending a scheme if one does not`
`784`		`-// exist so that the URL does not get parsed improperly.`
`785`		`-func parseURL(ctx context.Context, u string) (*url.URL, error) {`
	`782`	`+// parseURL parses a string into a URL.`
	`783`	`+func parseURL(u string) (*url.URL, error) {`
`786`	`784`	`var (`
`787`	`785`	`hasScheme = strings.HasPrefix(u, "http:") \|\| strings.HasPrefix(u, "https:")`
`788`	`786`	`)`
`789`	`787`
`790`	`788`	`if !hasScheme {`
`791`		`- // Append a scheme if it doesn't have one. Otherwise the hostname`
`792`		`- // will likely get parsed as the scheme and cause methods like Hostname()`
`793`		`- // to return an empty string, largely obviating the purpose of this`
`794`		`- // function.`
`795`		`- u = "https://" + u`
	`789`	`+ return nil, xerrors.Errorf("URL %q must have a scheme of either http or https", u)`
`796`	`790`	`}`
`797`	`791`
`798`	`792`	`parsed, err := url.Parse(u)`
`799`	`793`	`if err != nil {`
`800`	`794`	`return nil, err`
`801`	`795`	`}`
`802`	`796`
`803`		`- // If the specified url is a loopback device and no scheme has been`
`804`		`- // specified, prefer http over https. It's unlikely anyone intends to use`
`805`		`- // https on a loopback and if they do they can specify a scheme.`
`806`		`- if local, _ := isLocalURL(ctx, parsed); local && !hasScheme {`
`807`		`- parsed.Scheme = "http"`
`808`		`- }`
`809`		`-`
`810`	`797`	`return parsed, nil`
`811`	`798`	`}`
`812`	`799`