cmd/tailscale: funnel wip cleanup and additional test coverage (#9316)

General cleanup and additional test coverage of WIP code.

* use enum for serveType
* combine instances of ServeConfig access within unset
* cleanMountPoint rewritten into cleanURLPath as it only handles URL paths
* refactor and test expandProxyTargetDev

> **Note**
> Behind the `TAILSCALE_USE_WIP_CODE` flag

updates #8489

Signed-off-by: Tyler Smalley <tyler@tailscale.com>

cmd/tailscale/cli/serve.go

@@ -167,7 +167,7 @@ type serveEnv struct {
 	https            string    // HTTP port
 	http             string    // HTTP port
 	tcp              string    // TCP port
-	tlsTerminatedTcp string    // a TLS terminated TCP port
+	tlsTerminatedTCP string    // a TLS terminated TCP port
 	subcmd           serveMode // subcommand
 
 	lc localServeClient // localClient interface, specific to serve

cmd/tailscale/cli/serve_dev.go

@@ -13,6 +13,7 @@ import (
 	"net/url"
 	"os"
 	"os/signal"
+	"path"
 	"path/filepath"
 	"slices"
 	"sort"
@@ -57,6 +58,15 @@ const (
 	funnel
 )
 
+type serveType int
+
+const (
+	serveTypeHTTPS serveType = iota
+	serveTypeHTTP
+	serveTypeTCP
+	serveTypeTLSTerminatedTCP
+)
+
 var infoMap = map[serveMode]commandInfo{
 	serve: {
 		Name:      "serve",
@@ -100,7 +110,7 @@ func newServeDevCommand(e *serveEnv, subcmd serveMode) *ffcli.Command {
 			fmt.Sprintf("%s status [--json]", info.Name),
 			fmt.Sprintf("%s reset", info.Name),
 		}, "\n  "),
-		LongHelp: info.LongHelp + fmt.Sprintf(strings.TrimSpace(serveHelpCommon), subcmd, subcmd),
+		LongHelp: info.LongHelp + fmt.Sprintf(strings.TrimSpace(serveHelpCommon), info.Name, info.Name),
 		Exec:     e.runServeCombined(subcmd),
 
 		FlagSet: e.newFlags("serve-set", func(fs *flag.FlagSet) {
@@ -109,7 +119,7 @@ func newServeDevCommand(e *serveEnv, subcmd serveMode) *ffcli.Command {
 			fs.StringVar(&e.https, "https", "", "default; HTTPS listener")
 			fs.StringVar(&e.http, "http", "", "HTTP listener")
 			fs.StringVar(&e.tcp, "tcp", "", "TCP listener")
-			fs.StringVar(&e.tlsTerminatedTcp, "tls-terminated-tcp", "", "TLS terminated TCP listener")
+			fs.StringVar(&e.tlsTerminatedTCP, "tls-terminated-tcp", "", "TLS terminated TCP listener")
 
 		}),
 		UsageFunc: usageFunc,
@@ -134,38 +144,30 @@ func newServeDevCommand(e *serveEnv, subcmd serveMode) *ffcli.Command {
 	}
 }
 
-// runServeCombined is the entry point for the "tailscale {serve,funnel}" commands.
+func validateArgs(subcmd serveMode, args []string) error {
-func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc {
+	switch len(args) {
-	e.subcmd = subcmd
+	case 0:
-
-	return func(ctx context.Context, args []string) error {
-		if len(args) == 0 {
 		return flag.ErrHelp
-		}
+	case 1, 2:
-
+		if isLegacyInvocation(subcmd, args) {
-		funnel := subcmd == funnel
+			fmt.Fprintf(os.Stderr, "error: the CLI for serve and funnel has changed.")
-
+			fmt.Fprintf(os.Stderr, "Please see https://tailscale.com/kb/1242/tailscale-serve for more information.")
-		err := checkLegacyServeInvocation(subcmd, args)
-		if err != nil {
-			fmt.Fprintf(os.Stderr, "error: the CLI for serve and funnel has changed.\n")
-			fmt.Fprintf(os.Stderr, "Please see https://tailscale.com/kb/1242/tailscale-serve for more information.\n\n")
-
 			return errHelp
 		}
-
+	default:
-		if len(args) > 2 {
+		fmt.Fprintf(os.Stderr, "error: invalid number of arguments (%d)", len(args))
-			fmt.Fprintf(os.Stderr, "error: invalid number of arguments (%d)\n\n", len(args))
 		return errHelp
 	}
+	return nil
+}
 
-		turnOff := "off" == args[len(args)-1]
+// runServeCombined is the entry point for the "tailscale {serve,funnel}" commands.
+func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc {
+	e.subcmd = subcmd
 
-		// support passing in a port number as the target
+	return func(ctx context.Context, args []string) error {
-		// TODO(tylersmalley) move to expandProxyTarget when we remove the legacy serve invocation
+		if err := validateArgs(subcmd, args); err != nil {
-		target := args[0]
+			return err
-		port, err := strconv.ParseUint(args[0], 10, 16)
-		if err == nil {
-			target = fmt.Sprintf("http://127.0.0.1:%d", port)
 		}
 
 		ctx, cancel := signal.NotifyContext(ctx, os.Interrupt)
@@ -176,6 +178,7 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc {
 			return fmt.Errorf("getting client status: %w", err)
 		}
 
+		funnel := subcmd == funnel
 		if funnel {
 			// verify node has funnel capabilities
 			if err := e.verifyFunnelEnabled(ctx, st, 443); err != nil {
@@ -183,10 +186,9 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc {
 			}
 		}
 
-		// default mount point to "/"
+		mount, err := cleanURLPath(e.setPath)
-		mount := e.setPath
+		if err != nil {
-		if mount == "" {
+			return fmt.Errorf("failed to clean the mount point: %w", err)
-			mount = "/"
 		}
 
 		if e.setPath != "" {
@@ -220,7 +222,8 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc {
 		// foreground or background.
 		parentSC := sc
 
-		if !turnOff && srvType == "https" {
+		turnOff := "off" == args[len(args)-1]
+		if !turnOff && srvType == serveTypeHTTPS {
 			// Running serve with https requires that the tailnet has enabled
 			// https cert provisioning. Send users through an interactive flow
 			// to enable this if not already done.
@@ -263,7 +266,7 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc {
 		if turnOff {
 			err = e.unsetServe(sc, dnsName, srvType, srvPort, mount)
 		} else {
-			err = e.setServe(sc, st, dnsName, srvType, srvPort, mount, target, funnel)
+			err = e.setServe(sc, st, dnsName, srvType, srvPort, mount, args[0], funnel)
 			msg = e.messageForPort(sc, st, dnsName, srvPort)
 		}
 		if err != nil {
@@ -275,7 +278,9 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc {
 			return err
 		}
 
+		if msg != "" {
 			fmt.Fprintln(os.Stderr, msg)
+		}
 
 		if watcher != nil {
 			for {
@@ -293,20 +298,16 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc {
 	}
 }
 
-func (e *serveEnv) setServe(sc *ipn.ServeConfig, st *ipnstate.Status, dnsName, srvType string, srvPort uint16, mount string, target string, allowFunnel bool) error {
+func (e *serveEnv) setServe(sc *ipn.ServeConfig, st *ipnstate.Status, dnsName string, srvType serveType, srvPort uint16, mount string, target string, allowFunnel bool) error {
 	// update serve config based on the type
 	switch srvType {
-	case "https", "http":
+	case serveTypeHTTPS, serveTypeHTTP:
-		mount, err := cleanMountPoint(mount)
+		useTLS := srvType == serveTypeHTTPS
-		if err != nil {
+		err := e.applyWebServe(sc, dnsName, srvPort, useTLS, mount, target)
-			return fmt.Errorf("failed to clean the mount point: %w", err)
-		}
-		useTLS := srvType == "https"
-		err = e.applyWebServe(sc, dnsName, srvPort, useTLS, mount, target)
 		if err != nil {
 			return fmt.Errorf("failed apply web serve: %w", err)
 		}
-	case "tcp", "tls-terminated-tcp":
+	case serveTypeTCP, serveTypeTLSTerminatedTCP:
 		err := e.applyTCPServe(sc, dnsName, srvType, srvPort, target)
 		if err != nil {
 			return fmt.Errorf("failed to apply TCP serve: %w", err)
@@ -321,6 +322,8 @@ func (e *serveEnv) setServe(sc *ipn.ServeConfig, st *ipnstate.Status, dnsName, s
 	return nil
 }
 
+// messageForPort returns a message for the given port based on the
+// serve config and status.
 func (e *serveEnv) messageForPort(sc *ipn.ServeConfig, st *ipnstate.Status, dnsName string, srvPort uint16) string {
 	var output strings.Builder
 
@@ -345,6 +348,11 @@ func (e *serveEnv) messageForPort(sc *ipn.ServeConfig, st *ipnstate.Status, dnsN
 
 	output.WriteString(fmt.Sprintf("%s://%s%s\n\n", scheme, dnsName, portPart))
 
+	if !e.bg {
+		output.WriteString("Press Ctrl+C to exit.")
+		return output.String()
+	}
+
 	srvTypeAndDesc := func(h *ipn.HTTPHandler) (string, string) {
 		switch {
 		case h.Path != "":
@@ -389,42 +397,28 @@ func (e *serveEnv) messageForPort(sc *ipn.ServeConfig, st *ipnstate.Status, dnsN
 		output.WriteString(fmt.Sprintf("|--> tcp://%s\n", h.TCPForward))
 	}
 
-	if e.bg {
 	output.WriteString("\nServe started and running in the background.\n")
 	output.WriteString(fmt.Sprintf("To disable the proxy, run: tailscale %s off", infoMap[e.subcmd].Name))
-	} else {
-		// TODO(marwan-at-work): give the user more context on their foreground process.
-	}
 
-	return output.String() + "\n"
+	return output.String()
 }
 
 func (e *serveEnv) applyWebServe(sc *ipn.ServeConfig, dnsName string, srvPort uint16, useTLS bool, mount, target string) error {
 	h := new(ipn.HTTPHandler)
 
-	// TODO: use strings.Cut as the prefix OR use strings.HasPrefix
-	ts, _, _ := strings.Cut(target, ":")
 	switch {
-	case ts == "text":
+	case strings.HasPrefix(target, "text:"):
 		text := strings.TrimPrefix(target, "text:")
 		if text == "" {
 			return errors.New("unable to serve; text cannot be an empty string")
 		}
 		h.Text = text
-	case isProxyTarget(target):
+	case filepath.IsAbs(target):
-		t, err := expandProxyTarget(target)
-		if err != nil {
-			return err
-		}
-		h.Proxy = t
-	default: // assume path
 		if version.IsSandboxedMacOS() {
 			// don't allow path serving for now on macOS (2022-11-15)
 			return errors.New("path serving is not supported if sandboxed on macOS")
 		}
-		if !filepath.IsAbs(target) {
+
-			return errors.New("path must be absolute")
-		}
 		target = filepath.Clean(target)
 		fi, err := os.Stat(target)
 		if err != nil {
@@ -438,6 +432,12 @@ func (e *serveEnv) applyWebServe(sc *ipn.ServeConfig, dnsName string, srvPort ui
 			mount += "/"
 		}
 		h.Path = target
+	default:
+		t, err := expandProxyTargetDev(target)
+		if err != nil {
+			return err
+		}
+		h.Proxy = t
 	}
 
 	// TODO: validation needs to check nested foreground configs
@@ -472,12 +472,12 @@ func (e *serveEnv) applyWebServe(sc *ipn.ServeConfig, dnsName string, srvPort ui
 	return nil
 }
 
-func (e *serveEnv) applyTCPServe(sc *ipn.ServeConfig, dnsName string, srcType string, srcPort uint16, target string) error {
+func (e *serveEnv) applyTCPServe(sc *ipn.ServeConfig, dnsName string, srcType serveType, srcPort uint16, target string) error {
 	var terminateTLS bool
 	switch srcType {
-	case "tcp":
+	case serveTypeTCP:
 		terminateTLS = false
-	case "tls-terminated-tcp":
+	case serveTypeTLSTerminatedTCP:
 		terminateTLS = true
 	default:
 		return fmt.Errorf("invalid TCP target %q", target)
@@ -535,35 +535,34 @@ func (e *serveEnv) applyFunnel(sc *ipn.ServeConfig, dnsName string, srvPort uint
 	}
 }
 
-// TODO(tylersmalley) Refactor into setServe so handleWebServeFunnelRemove and handleTCPServeRemove.
+// unsetServe removes the serve config for the given serve port.
-// apply serve config changes and we print a status message.
+func (e *serveEnv) unsetServe(sc *ipn.ServeConfig, dnsName string, srvType serveType, srvPort uint16, mount string) error {
-func (e *serveEnv) unsetServe(sc *ipn.ServeConfig, dnsName string, srvType string, srvPort uint16, mount string) error {
 	switch srvType {
-	case "https", "http":
+	case serveTypeHTTPS, serveTypeHTTP:
-		mount, err := cleanMountPoint(mount)
+		err := e.removeWebServe(sc, dnsName, srvPort, mount)
 		if err != nil {
-			return fmt.Errorf("failed to clean the mount point: %w", err)
+			return fmt.Errorf("failed to remove web serve: %w", err)
 		}
-		err = e.handleWebServeFunnelRemove(sc, dnsName, srvPort, mount)
+	case serveTypeTCP, serveTypeTLSTerminatedTCP:
+		err := e.removeTCPServe(sc, srvPort)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to remove TCP serve: %w", err)
 		}
-
-		return nil
-	case "tcp", "tls-terminated-tcp":
-		// TODO(tylersmalley) should remove funnel
-		return e.removeTCPServe(sc, srvPort)
 	default:
 		return fmt.Errorf("invalid type %q", srvType)
 	}
+
+	// TODO(tylersmalley): remove funnel
+
+	return nil
 }
 
-func srvTypeAndPortFromFlags(e *serveEnv) (srvType string, srvPort uint16, err error) {
+func srvTypeAndPortFromFlags(e *serveEnv) (srvType serveType, srvPort uint16, err error) {
-	sourceMap := map[string]string{
+	sourceMap := map[serveType]string{
-		"http":               e.http,
+		serveTypeHTTP:             e.http,
-		"https":              e.https,
+		serveTypeHTTPS:            e.https,
-		"tcp":                e.tcp,
+		serveTypeTCP:              e.tcp,
-		"tls-terminated-tcp": e.tlsTerminatedTcp,
+		serveTypeTLSTerminatedTCP: e.tlsTerminatedTCP,
 	}
 
 	var srcTypeCount int
@@ -578,60 +577,60 @@ func srvTypeAndPortFromFlags(e *serveEnv) (srvType string, srvPort uint16, err e
 	}
 
 	if srcTypeCount > 1 {
-		return "", 0, fmt.Errorf("cannot serve multiple types for a single mount point")
+		return 0, 0, fmt.Errorf("cannot serve multiple types for a single mount point")
 	} else if srcTypeCount == 0 {
-		srvType = "https"
+		srvType = serveTypeHTTPS
 		srcValue = "443"
 	}
 
 	srvPort, err = parseServePort(srcValue)
 	if err != nil {
-		return "", 0, fmt.Errorf("invalid port %q: %w", srcValue, err)
+		return 0, 0, fmt.Errorf("invalid port %q: %w", srcValue, err)
 	}
 
 	return srvType, srvPort, nil
 }
 
-func checkLegacyServeInvocation(subcmd serveMode, args []string) error {
+func isLegacyInvocation(subcmd serveMode, args []string) bool {
 	if subcmd == serve && len(args) == 2 {
-		prefixes := []string{"http:", "https:", "tls:", "tls-terminated-tcp:"}
+		prefixes := []string{"http", "https", "tcp", "tls-terminated-tcp"}
 
 		for _, prefix := range prefixes {
 			if strings.HasPrefix(args[0], prefix) {
-				return errors.New("invalid invocation")
+				return true
 			}
 		}
 	}
 
-	return nil
+	return false
 }
 
-// handleWebServeFunnelRemove removes a web handler from the serve config
+// removeWebServe removes a web handler from the serve config
 // and removes funnel if no remaining mounts exist for the serve port.
 // The srvPort argument is the serving port and the mount argument is
 // the mount point or registered path to remove.
-// TODO(tylersmalley): fork of handleWebServeRemove, return name once dev work is merged
+func (e *serveEnv) removeWebServe(sc *ipn.ServeConfig, dnsName string, srvPort uint16, mount string) error {
-func (e *serveEnv) handleWebServeFunnelRemove(sc *ipn.ServeConfig, dnsName string, srvPort uint16, mount string) error {
-	if sc == nil {
-		return errors.New("error: serve config does not exist")
-	}
 	if sc.IsTCPForwardingOnPort(srvPort) {
 		return errors.New("cannot remove web handler; currently serving TCP")
 	}
+
 	hp := ipn.HostPort(net.JoinHostPort(dnsName, strconv.Itoa(int(srvPort))))
 	if !sc.WebHandlerExists(hp, mount) {
 		return errors.New("error: handler does not exist")
 	}
+
 	// delete existing handler, then cascade delete if empty
 	delete(sc.Web[hp].Handlers, mount)
 	if len(sc.Web[hp].Handlers) == 0 {
 		delete(sc.Web, hp)
 		delete(sc.TCP, srvPort)
 	}
+
 	// clear empty maps mostly for testing
 	if len(sc.Web) == 0 {
 		sc.Web = nil
 	}
+
 	if len(sc.TCP) == 0 {
 		sc.TCP = nil
 	}
@@ -663,3 +662,94 @@ func (e *serveEnv) removeTCPServe(sc *ipn.ServeConfig, src uint16) error {
 	}
 	return nil
 }
+
+// expandProxyTargetDev expands the supported target values to be proxied
+// allowing for input values to be a port number, a partial URL, or a full URL
+// including a path.
+//
+// examples:
+//   - 3000
+//   - localhost:3000
+//   - http://localhost:3000
+//   - https://localhost:3000
+//   - https-insecure://localhost:3000
+//   - https-insecure://localhost:3000/foo
+func expandProxyTargetDev(target string) (string, error) {
+	var (
+		scheme = "http"
+		host   = "127.0.0.1"
+	)
+
+	// support target being a port number
+	if port, err := strconv.ParseUint(target, 10, 16); err == nil {
+		return fmt.Sprintf("%s://%s:%d", scheme, host, port), nil
+	}
+
+	// prepend scheme if not present
+	if !strings.Contains(target, "://") {
+		target = scheme + "://" + target
+	}
+
+	// make sure we can parse the target
+	u, err := url.ParseRequestURI(target)
+	if err != nil {
+		return "", fmt.Errorf("invalid URL %w", err)
+	}
+
+	// ensure a supported scheme
+	switch u.Scheme {
+	case "http", "https", "https+insecure":
+	default:
+		return "", errors.New("must be a URL starting with http://, https://, or https+insecure://")
+	}
+
+	// validate the port
+	port, err := strconv.ParseUint(u.Port(), 10, 16)
+	if err != nil || port == 0 {
+		return "", fmt.Errorf("invalid port %q", u.Port())
+	}
+
+	// validate the host.
+	switch u.Hostname() {
+	case "localhost", "127.0.0.1":
+		u.Host = fmt.Sprintf("%s:%d", host, port)
+	default:
+		return "", errors.New("only localhost or 127.0.0.1 proxies are currently supported")
+	}
+
+	return u.String(), nil
+}
+
+// cleanURLPath ensures the path is clean and has a leading "/".
+func cleanURLPath(urlPath string) (string, error) {
+	if urlPath == "" {
+		return "/", nil
+	}
+
+	// TODO(tylersmalley) verify still needed with path being a flag
+	urlPath = cleanMinGWPathConversionIfNeeded(urlPath)
+	if !strings.HasPrefix(urlPath, "/") {
+		urlPath = "/" + urlPath
+	}
+
+	c := path.Clean(urlPath)
+	if urlPath == c || urlPath == c+"/" {
+		return urlPath, nil
+	}
+	return "", fmt.Errorf("invalid mount point %q", urlPath)
+}
+
+func (s serveType) String() string {
+	switch s {
+	case serveTypeHTTP:
+		return "httpListener"
+	case serveTypeHTTPS:
+		return "httpsListener"
+	case serveTypeTCP:
+		return "tcpListener"
+	case serveTypeTLSTerminatedTCP:
+		return "tlsTerminatedTCPListener"
+	default:
+		return "unknownServeType"
+	}
+}

cmd/tailscale/cli/serve_dev_test.go

@@ -6,10 +6,12 @@ package cli
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"os"
 	"path/filepath"
 	"reflect"
 	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/peterbourgon/ff/v3/ffcli"
@@ -783,49 +785,48 @@ func TestSrcTypeFromFlags(t *testing.T) {
 	tests := []struct {
 		name         string
 		env          *serveEnv
-		expectedType string
+		expectedType serveType
 		expectedPort uint16
 		expectedErr  bool
 	}{
 		{
 			name:         "only http set",
 			env:          &serveEnv{http: "80"},
-			expectedType: "http",
+			expectedType: serveTypeHTTP,
 			expectedPort: 80,
 			expectedErr:  false,
 		},
 		{
 			name:         "only https set",
 			env:          &serveEnv{https: "10000"},
-			expectedType: "https",
+			expectedType: serveTypeHTTPS,
 			expectedPort: 10000,
 			expectedErr:  false,
 		},
 		{
 			name:         "only tcp set",
 			env:          &serveEnv{tcp: "8000"},
-			expectedType: "tcp",
+			expectedType: serveTypeTCP,
 			expectedPort: 8000,
 			expectedErr:  false,
 		},
 		{
 			name:         "only tls-terminated-tcp set",
-			env:          &serveEnv{tlsTerminatedTcp: "8080"},
+			env:          &serveEnv{tlsTerminatedTCP: "8080"},
-			expectedType: "tls-terminated-tcp",
+			expectedType: serveTypeTLSTerminatedTCP,
 			expectedPort: 8080,
 			expectedErr:  false,
 		},
 		{
 			name:         "defaults to https, port 443",
 			env:          &serveEnv{},
-			expectedType: "https",
+			expectedType: serveTypeHTTPS,
 			expectedPort: 443,
 			expectedErr:  false,
 		},
 		{
 			name:         "multiple types set",
 			env:          &serveEnv{http: "80", https: "443"},
-			expectedType: "",
 			expectedPort: 0,
 			expectedErr:  true,
 		},
@@ -838,7 +839,7 @@ func TestSrcTypeFromFlags(t *testing.T) {
 				t.Errorf("Expected error: %v, got: %v", tt.expectedErr, err)
 			}
 			if srcType != tt.expectedType {
-				t.Errorf("Expected srcType: %s, got: %s", tt.expectedType, srcType)
+				t.Errorf("Expected srcType: %s, got: %s", tt.expectedType.String(), srcType)
 			}
 			if srcPort != tt.expectedPort {
 				t.Errorf("Expected srcPort: %d, got: %d", tt.expectedPort, srcPort)
@@ -846,3 +847,109 @@ func TestSrcTypeFromFlags(t *testing.T) {
 		})
 	}
 }
+
+func TestExpandProxyTargetDev(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected string
+		wantErr  bool
+	}{
+		{input: "8080", expected: "http://127.0.0.1:8080"},
+		{input: "localhost:8080", expected: "http://127.0.0.1:8080"},
+		{input: "http://localhost:8080", expected: "http://127.0.0.1:8080"},
+		{input: "http://127.0.0.1:8080", expected: "http://127.0.0.1:8080"},
+		{input: "http://127.0.0.1:8080/foo", expected: "http://127.0.0.1:8080/foo"},
+		{input: "https://localhost:8080", expected: "https://127.0.0.1:8080"},
+		{input: "https+insecure://localhost:8080", expected: "https+insecure://127.0.0.1:8080"},
+
+		// errors
+		{input: "localhost:9999999", wantErr: true},
+		{input: "ftp://localhost:8080", expected: "", wantErr: true},
+		{input: "https://tailscale.com:8080", expected: "", wantErr: true},
+		{input: "", expected: "", wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			actual, err := expandProxyTargetDev(tt.input)
+
+			if tt.wantErr == true && err == nil {
+				t.Errorf("Expected an error but got none")
+				return
+			}
+
+			if tt.wantErr == false && err != nil {
+				t.Errorf("Got an error, but didn't expect one: %v", err)
+				return
+			}
+
+			if actual != tt.expected {
+				t.Errorf("Got: %q; expected: %q", actual, tt.expected)
+			}
+		})
+	}
+}
+
+func TestCleanURLPath(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected string
+		wantErr  bool
+	}{
+		{input: "", expected: "/"},
+		{input: "/", expected: "/"},
+		{input: "/foo", expected: "/foo"},
+		{input: "/foo/", expected: "/foo/"},
+		{input: "/../bar", wantErr: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			actual, err := cleanURLPath(tt.input)
+
+			if tt.wantErr == true && err == nil {
+				t.Errorf("Expected an error but got none")
+				return
+			}
+
+			if tt.wantErr == false && err != nil {
+				t.Errorf("Got an error, but didn't expect one: %v", err)
+				return
+			}
+
+			if actual != tt.expected {
+				t.Errorf("Got: %q; expected: %q", actual, tt.expected)
+			}
+		})
+	}
+}
+
+func TestIsLegacyInvocation(t *testing.T) {
+	tests := []struct {
+		subcmd   serveMode
+		args     []string
+		expected bool
+	}{
+		{subcmd: serve, args: []string{"https", "localhost:3000"}, expected: true},
+		{subcmd: serve, args: []string{"https:8443", "localhost:3000"}, expected: true},
+		{subcmd: serve, args: []string{"http", "localhost:3000"}, expected: true},
+		{subcmd: serve, args: []string{"http:80", "localhost:3000"}, expected: true},
+		{subcmd: serve, args: []string{"tcp:2222", "tcp://localhost:22"}, expected: true},
+		{subcmd: serve, args: []string{"tls-terminated-tcp:443", "tcp://localhost:80"}, expected: true},
+
+		// false
+		{subcmd: serve, args: []string{"3000"}, expected: false},
+		{subcmd: serve, args: []string{"localhost:3000"}, expected: false},
+	}
+
+	for _, tt := range tests {
+		args := strings.Join(tt.args, " ")
+		t.Run(fmt.Sprintf("%v %s", infoMap[tt.subcmd].Name, args), func(t *testing.T) {
+			actual := isLegacyInvocation(tt.subcmd, tt.args)
+
+			if actual != tt.expected {
+				t.Errorf("Got: %v; expected: %v", actual, tt.expected)
+			}
+		})
+	}
+}

cmd/tailscale/cli/serve_test.go

@@ -9,7 +9,6 @@ import (
 	"errors"
 	"flag"
 	"fmt"
-	"io"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -902,11 +901,6 @@ func (lc *fakeLocalServeClient) IncrementCounter(ctx context.Context, name strin
 	return nil // unused in tests
 }
 
-func (lc *fakeLocalServeClient) StreamServe(ctx context.Context, req ipn.ServeStreamRequest) (io.ReadCloser, error) {
-	// TODO: testing :)
-	return nil, nil
-}
-
 // exactError returns an error checker that wants exactly the provided want error.
 // If optName is non-empty, it's used in the error message.
 func exactErr(want error, optName ...string) func(error) string {

ipn/serve.go

@@ -85,27 +85,6 @@ type FunnelConn struct {
 	Src netip.AddrPort
 }
 
-// ServeStreamRequest defines the JSON request body
-// for the serve stream endpoint
-type ServeStreamRequest struct {
-	// HostPort is the DNS and port of the tailscale
-	// URL.
-	HostPort HostPort `json:",omitempty"`
-
-	// Source is the user's serve source
-	// as defined in the `tailscale serve`
-	// command such as http://127.0.0.1:3000
-	Source string `json:",omitempty"`
-
-	// MountPoint is the path prefix for
-	// the given HostPort.
-	MountPoint string `json:",omitempty"`
-
-	// Funnel indicates whether the request
-	// is a serve request or a funnel one.
-	Funnel bool `json:",omitempty"`
-}
-
 // WebServerConfig describes a web server's configuration.
 type WebServerConfig struct {
 	Handlers map[string]*HTTPHandler // mountPoint => handler