From 5f3cdaf2833cd3ed8325eb90f7a45e2360952487 Mon Sep 17 00:00:00 2001 From: Marwan Sulaiman Date: Tue, 24 Oct 2023 18:18:47 -0400 Subject: [PATCH] cmd/tailscale/cli: chage port flags to uint for serve and funnel This PR changes the -https, -http, -tcp, and -tls-terminated-tcp flags from string to int and also updates the validation to ensure they fit the uint16 size as the flag library does not have a Uint16Var method. Updates #8489 Signed-off-by: Marwan Sulaiman --- cmd/tailscale/cli/serve_legacy.go | 8 ++++---- cmd/tailscale/cli/serve_v2.go | 32 ++++++++++++++---------------- cmd/tailscale/cli/serve_v2_test.go | 17 +++++++++++----- 3 files changed, 31 insertions(+), 26 deletions(-) diff --git a/cmd/tailscale/cli/serve_legacy.go b/cmd/tailscale/cli/serve_legacy.go index de990c3b2..a1803c6dd 100644 --- a/cmd/tailscale/cli/serve_legacy.go +++ b/cmd/tailscale/cli/serve_legacy.go @@ -159,10 +159,10 @@ type serveEnv struct { // v2 specific flags bg bool // background mode setPath string // serve path - https string // HTTP port - http string // HTTP port - tcp string // TCP port - tlsTerminatedTCP string // a TLS terminated TCP port + https uint // HTTP port + http uint // HTTP port + tcp uint // TCP port + tlsTerminatedTCP uint // a TLS terminated TCP port subcmd serveMode // subcommand yes bool // update without prompt diff --git a/cmd/tailscale/cli/serve_v2.go b/cmd/tailscale/cli/serve_v2.go index 7c79defd8..8d2b43d8c 100644 --- a/cmd/tailscale/cli/serve_v2.go +++ b/cmd/tailscale/cli/serve_v2.go @@ -11,6 +11,7 @@ "fmt" "io" "log" + "math" "net" "net/url" "os" @@ -126,15 +127,15 @@ func newServeV2Command(e *serveEnv, subcmd serveMode) *ffcli.Command { Exec: e.runServeCombined(subcmd), FlagSet: e.newFlags("serve-set", func(fs *flag.FlagSet) { - fs.BoolVar(&e.bg, "bg", false, "Run the command as a background process") + fs.BoolVar(&e.bg, "bg", false, "Run the command as a background process (default false)") fs.StringVar(&e.setPath, "set-path", "", "Appends the specified path to the base URL for accessing the underlying service") - fs.StringVar(&e.https, "https", "", "Expose an HTTPS server at the specified port (default") - fs.StringVar(&e.http, "http", "", "Expose an HTTP server at the specified port") - fs.StringVar(&e.tcp, "tcp", "", "Expose a TCP forwarder to forward raw TCP packets at the specified port") - fs.StringVar(&e.tlsTerminatedTCP, "tls-terminated-tcp", "", "Expose a TCP forwarder to forward TLS-terminated TCP packets at the specified port") - fs.BoolVar(&e.yes, "yes", false, "Update without interactive prompts") + fs.UintVar(&e.https, "https", 0, "Expose an HTTPS server at the specified port (default mode)") + fs.UintVar(&e.http, "http", 0, "Expose an HTTP server at the specified port") + fs.UintVar(&e.tcp, "tcp", 0, "Expose a TCP forwarder to forward raw TCP packets at the specified port") + fs.UintVar(&e.tlsTerminatedTCP, "tls-terminated-tcp", 0, "Expose a TCP forwarder to forward TLS-terminated TCP packets at the specified port") + fs.BoolVar(&e.yes, "yes", false, "Update without interactive prompts (default false)") }), - UsageFunc: usageFunc, + UsageFunc: usageFuncNoDefaultValues, Subcommands: []*ffcli.Command{ { Name: "status", @@ -649,7 +650,7 @@ func (e *serveEnv) unsetServe(sc *ipn.ServeConfig, dnsName string, srvType serve } func srvTypeAndPortFromFlags(e *serveEnv) (srvType serveType, srvPort uint16, err error) { - sourceMap := map[serveType]string{ + sourceMap := map[serveType]uint{ serveTypeHTTP: e.http, serveTypeHTTPS: e.https, serveTypeTCP: e.tcp, @@ -657,13 +658,15 @@ func srvTypeAndPortFromFlags(e *serveEnv) (srvType serveType, srvPort uint16, er } var srcTypeCount int - var srcValue string for k, v := range sourceMap { - if v != "" { + if v != 0 { + if v > math.MaxUint16 { + return 0, 0, fmt.Errorf("port number %d is too high for %s flag", v, srvType) + } srcTypeCount++ srvType = k - srcValue = v + srvPort = uint16(v) } } @@ -671,12 +674,7 @@ func srvTypeAndPortFromFlags(e *serveEnv) (srvType serveType, srvPort uint16, er return 0, 0, fmt.Errorf("cannot serve multiple types for a single mount point") } else if srcTypeCount == 0 { srvType = serveTypeHTTPS - srcValue = "443" - } - - srvPort, err = parseServePort(srcValue) - if err != nil { - return 0, 0, fmt.Errorf("invalid port %q: %w", srcValue, err) + srvPort = 443 } return srvType, srvPort, nil diff --git a/cmd/tailscale/cli/serve_v2_test.go b/cmd/tailscale/cli/serve_v2_test.go index a663548d0..d829e431e 100644 --- a/cmd/tailscale/cli/serve_v2_test.go +++ b/cmd/tailscale/cli/serve_v2_test.go @@ -207,6 +207,13 @@ type group struct { wantErr: anyErr(), }}, }, + { + name: "invalid_mount_port_too_high", + steps: []step{{ + command: cmd("serve --https=65536 --bg http://localhost:3000"), // invalid port, too high + wantErr: anyErr(), + }}, + }, { name: "invalid_host", steps: []step{{ @@ -948,28 +955,28 @@ func TestSrcTypeFromFlags(t *testing.T) { }{ { name: "only http set", - env: &serveEnv{http: "80"}, + env: &serveEnv{http: 80}, expectedType: serveTypeHTTP, expectedPort: 80, expectedErr: false, }, { name: "only https set", - env: &serveEnv{https: "10000"}, + env: &serveEnv{https: 10000}, expectedType: serveTypeHTTPS, expectedPort: 10000, expectedErr: false, }, { name: "only tcp set", - env: &serveEnv{tcp: "8000"}, + env: &serveEnv{tcp: 8000}, expectedType: serveTypeTCP, expectedPort: 8000, expectedErr: false, }, { name: "only tls-terminated-tcp set", - env: &serveEnv{tlsTerminatedTCP: "8080"}, + env: &serveEnv{tlsTerminatedTCP: 8080}, expectedType: serveTypeTLSTerminatedTCP, expectedPort: 8080, expectedErr: false, @@ -983,7 +990,7 @@ func TestSrcTypeFromFlags(t *testing.T) { }, { name: "multiple types set", - env: &serveEnv{http: "80", https: "443"}, + env: &serveEnv{http: 80, https: 443}, expectedPort: 0, expectedErr: true, },