util: add parse fallback helpers (#20022)

util/def: add def.Bool and def.Duration default parse helpers

Replace multiple instances of def.Bool and def.Duration with a new util/def
package.

Updates #20018

Co-authored-by: Bobby <boby@codelabs.co.id>
Co-authored-by: Simon Law <sfllaw@tailscale.com>
Signed-off-by: Bobby <boby@codelabs.co.id>
Signed-off-by: Simon Law <sfllaw@tailscale.com>

cmd/k8s-operator/depaware.txt

@@ -871,6 +871,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/
   LW    tailscale.com/util/cmpver                                    from tailscale.com/net/dns+
         tailscale.com/util/ctxkey                                    from tailscale.com/client/tailscale/apitype+
      💣 tailscale.com/util/deephash                                  from tailscale.com/util/syspolicy/setting
+        tailscale.com/util/def                                       from tailscale.com/ipn/localapi
    L 💣 tailscale.com/util/dirwalk                                   from tailscale.com/metrics
         tailscale.com/util/dnsname                                   from tailscale.com/appc+
         tailscale.com/util/eventbus                                  from tailscale.com/tsd+

cmd/tailscaled/depaware-min.txt

@@ -159,6 +159,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/util/cloudenv                                  from tailscale.com/hostinfo+
         tailscale.com/util/cloudinfo                                 from tailscale.com/wgengine/magicsock
         tailscale.com/util/ctxkey                                    from tailscale.com/client/tailscale/apitype+
+        tailscale.com/util/def                                       from tailscale.com/ipn/localapi
         tailscale.com/util/dnsname                                   from tailscale.com/appc+
         tailscale.com/util/eventbus                                  from tailscale.com/control/controlclient+
         tailscale.com/util/execqueue                                 from tailscale.com/appc+

cmd/tailscaled/depaware-minbox.txt

@@ -178,6 +178,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/util/cloudenv                                  from tailscale.com/hostinfo+
         tailscale.com/util/cloudinfo                                 from tailscale.com/wgengine/magicsock
         tailscale.com/util/ctxkey                                    from tailscale.com/client/tailscale/apitype+
+        tailscale.com/util/def                                       from tailscale.com/ipn/localapi
         tailscale.com/util/dnsname                                   from tailscale.com/appc+
         tailscale.com/util/eventbus                                  from tailscale.com/client/local+
         tailscale.com/util/execqueue                                 from tailscale.com/appc+

cmd/tailscaled/depaware.txt

@@ -448,6 +448,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/util/cmpver                                    from tailscale.com/net/dns+
         tailscale.com/util/ctxkey                                    from tailscale.com/ipn/ipnlocal+
      💣 tailscale.com/util/deephash                                  from tailscale.com/util/syspolicy/setting
+        tailscale.com/util/def                                       from tailscale.com/feature/debugportmapper+
    L 💣 tailscale.com/util/dirwalk                                   from tailscale.com/metrics+
         tailscale.com/util/dnsname                                   from tailscale.com/appc+
         tailscale.com/util/eventbus                                  from tailscale.com/tsd+

cmd/tsidp/depaware.txt

@@ -269,6 +269,7 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar
   LW    tailscale.com/util/cmpver                                    from tailscale.com/net/dns+
         tailscale.com/util/ctxkey                                    from tailscale.com/client/tailscale/apitype+
      💣 tailscale.com/util/deephash                                  from tailscale.com/util/syspolicy/setting
+        tailscale.com/util/def                                       from tailscale.com/ipn/localapi
    L 💣 tailscale.com/util/dirwalk                                   from tailscale.com/metrics
         tailscale.com/util/dnsname                                   from tailscale.com/appc+
         tailscale.com/util/eventbus                                  from tailscale.com/client/local+

feature/debugportmapper/debugportmapper.go

@@ -11,7 +11,6 @@ import (
 	"net"
 	"net/http"
 	"net/netip"
-	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -20,6 +19,7 @@ import (
 	"tailscale.com/net/netmon"
 	"tailscale.com/net/portmapper"
 	"tailscale.com/types/logger"
+	"tailscale.com/util/def"
 	"tailscale.com/util/eventbus"
 )
 
@@ -66,7 +66,7 @@ func serveDebugPortmap(h *localapi.Handler, w http.ResponseWriter, r *http.Reque
 		}
 	}
 
-	if defBool(r.FormValue("log_http"), false) {
+	if def.Bool(r.FormValue("log_http"), false) {
 		debugKnobs.LogHTTP = true
 	}
 
@@ -191,14 +191,3 @@ func serveDebugPortmap(h *localapi.Handler, w http.ResponseWriter, r *http.Reque
 		}
 	}
 }
-
-func defBool(a string, def bool) bool {
-	if a == "" {
-		return def
-	}
-	v, err := strconv.ParseBool(a)
-	if err != nil {
-		return def
-	}
-	return v
-}

feature/routecheck/localapi.go

@@ -5,7 +5,6 @@ package routecheck
 
 import (
 	"net/http"
-	"strconv"
 	"time"
 
 	jsonv2 "github.com/go-json-experiment/json"
@@ -13,6 +12,7 @@ import (
 
 	"tailscale.com/ipn/localapi"
 	"tailscale.com/net/routecheck"
+	"tailscale.com/util/def"
 	"tailscale.com/util/httpm"
 )
 
@@ -39,8 +39,8 @@ func serveRouteCheck(h *localapi.Handler, w http.ResponseWriter, r *http.Request
 
 	var err error
 	var report *routecheck.Report
-	if defBool(r.FormValue("probe"), false) {
+	if def.Bool(r.FormValue("probe"), false) {
-		timeout := defDuration(r.FormValue("timeout"), routecheck.DefaultTimeout)
+		timeout := def.Duration(r.FormValue("timeout"), routecheck.DefaultTimeout)
 		timeout = min(max(0, timeout), 60*time.Second) // clamp to [0s, 60s]
 		report, err = rc.Refresh(r.Context(), timeout)
 	} else {
@@ -61,25 +61,3 @@ func serveRouteCheck(h *localapi.Handler, w http.ResponseWriter, r *http.Request
 	// with its default options, marshal with DefaultOptionsV1.
 	jsonv2.MarshalWrite(w, report, jsonv1.DefaultOptionsV1())
 }
-
-func defBool(a string, def bool) bool {
-	if a == "" {
-		return def
-	}
-	v, err := strconv.ParseBool(a)
-	if err != nil {
-		return def
-	}
-	return v
-}
-
-func defDuration(a string, def time.Duration) time.Duration {
-	if a == "" {
-		return def
-	}
-	v, err := time.ParseDuration(a)
-	if err != nil {
-		return def
-	}
-	return v
-}

ipn/localapi/localapi.go

@@ -44,6 +44,7 @@ import (
 	"tailscale.com/types/logger"
 	"tailscale.com/types/logid"
 	"tailscale.com/util/clientmetric"
+	"tailscale.com/util/def"
 	"tailscale.com/util/eventbus"
 	"tailscale.com/util/httpm"
 	"tailscale.com/util/mak"
@@ -448,7 +449,7 @@ func (h *Handler) serveBugReport(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	if defBool(r.URL.Query().Get("diagnose"), false) {
+	if def.Bool(r.URL.Query().Get("diagnose"), false) {
 		if f, ok := ipnlocal.HookDoctor.GetOk(); ok {
 			f(r.Context(), h.b, logger.WithPrefix(h.logf, "diag: "))
 		}
@@ -458,7 +459,7 @@ func (h *Handler) serveBugReport(w http.ResponseWriter, r *http.Request) {
 
 	// Nothing else to do if we're not in record mode; we wrote the marker
 	// above, so we can just finish our response now.
-	if !defBool(r.URL.Query().Get("record"), false) {
+	if !def.Bool(r.URL.Query().Get("record"), false) {
 		return
 	}
 
@@ -846,7 +847,7 @@ func (h *Handler) serveStatus(w http.ResponseWriter, r *http.Request) {
 	}
 	w.Header().Set("Content-Type", "application/json")
 	var st *ipnstate.Status
-	if defBool(r.FormValue("peers"), true) {
+	if def.Bool(r.FormValue("peers"), true) {
 		st = h.b.Status()
 	} else {
 		st = h.b.StatusWithoutPeers()
@@ -1666,17 +1667,6 @@ func (h *Handler) serveQueryFeature(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-func defBool(a string, def bool) bool {
-	if a == "" {
-		return def
-	}
-	v, err := strconv.ParseBool(a)
-	if err != nil {
-		return def
-	}
-	return v
-}
-
 // serveUpdateCheck returns the ClientVersion from Status, which contains
 // information on whether an update is available, and if so, what version,
 // *if* we support auto-updates on this platform. If we don't, this endpoint

tsnet/depaware.txt

@@ -264,6 +264,7 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware)
   LW    tailscale.com/util/cmpver                                    from tailscale.com/net/dns+
         tailscale.com/util/ctxkey                                    from tailscale.com/client/tailscale/apitype+
      💣 tailscale.com/util/deephash                                  from tailscale.com/util/syspolicy/setting
+        tailscale.com/util/def                                       from tailscale.com/ipn/localapi
   LA 💣 tailscale.com/util/dirwalk                                   from tailscale.com/metrics
         tailscale.com/util/dnsname                                   from tailscale.com/appc+
         tailscale.com/util/eventbus                                  from tailscale.com/client/local+

util/def/def.go

@@ -0,0 +1,34 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+// Package def parses strings with fallback default values.
+package def
+
+import (
+	"strconv"
+	"time"
+)
+
+// Bool parses s as a bool, returning def when s is empty or invalid.
+func Bool(s string, def bool) bool {
+	if s == "" {
+		return def
+	}
+	v, err := strconv.ParseBool(s)
+	if err != nil {
+		return def
+	}
+	return v
+}
+
+// Duration parses s as a time.Duration, returning def when s is empty or invalid.
+func Duration(s string, def time.Duration) time.Duration {
+	if s == "" {
+		return def
+	}
+	v, err := time.ParseDuration(s)
+	if err != nil {
+		return def
+	}
+	return v
+}

util/def/def_test.go

@@ -0,0 +1,121 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+package def_test
+
+import (
+	"strconv"
+	"testing"
+	"time"
+
+	"tailscale.com/util/def"
+)
+
+func TestBool(t *testing.T) {
+	tests := []struct {
+		name string
+		in   string
+		def  bool
+		want bool
+	}{
+		{name: "empty_true", in: "", def: true, want: true},
+		{name: "empty_false", in: "", def: false, want: false},
+		{name: "valid_1", in: "1", def: false, want: true},
+		{name: "valid_t", in: "t", def: false, want: true},
+		{name: "valid_T", in: "T", def: false, want: true},
+		{name: "valid_TRUE", in: "TRUE", def: false, want: true},
+		{name: "valid_true", in: "true", def: false, want: true},
+		{name: "valid_True", in: "True", def: false, want: true},
+		{name: "valid_true_default_true", in: "true", def: true, want: true},
+		{name: "valid_0", in: "0", def: true, want: false},
+		{name: "valid_f", in: "f", def: true, want: false},
+		{name: "valid_F", in: "F", def: true, want: false},
+		{name: "valid_FALSE", in: "FALSE", def: true, want: false},
+		{name: "valid_false", in: "false", def: true, want: false},
+		{name: "valid_False", in: "False", def: true, want: false},
+		{name: "valid_false_default_false", in: "false", def: false, want: false},
+		{name: "invalid_true", in: "sure", def: true, want: true},
+		{name: "invalid_false", in: "sure", def: false, want: false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := def.Bool(tt.in, tt.def); got != tt.want {
+				t.Errorf("Bool(%q, %v) = %v; want %v", tt.in, tt.def, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestDuration(t *testing.T) {
+	tests := []struct {
+		name string
+		in   string
+		def  time.Duration
+		want time.Duration
+	}{
+		{name: "empty_second", in: "", def: time.Second, want: time.Second},
+		{name: "empty_zero", in: "", def: 0, want: 0},
+		{name: "valid", in: "2m30s", def: time.Second, want: 2*time.Minute + 30*time.Second},
+		{name: "valid_zero", in: "0s", def: time.Second, want: 0},
+		{name: "invalid_second", in: "soon", def: time.Second, want: time.Second},
+		{name: "invalid_minute", in: "soon", def: time.Minute, want: time.Minute},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := def.Duration(tt.in, tt.def); got != tt.want {
+				t.Errorf("Duration(%q, %v) = %v; want %v", tt.in, tt.def, got, tt.want)
+			}
+		})
+	}
+}
+
+func FuzzBool(f *testing.F) {
+	for _, tc := range []struct {
+		in  string
+		def bool
+	}{
+		{in: "", def: true},
+		{in: "", def: false},
+		{in: "true", def: false},
+		{in: "false", def: true},
+		{in: "sure", def: true},
+		{in: "sure", def: false},
+	} {
+		f.Add(tc.in, tc.def)
+	}
+	f.Fuzz(func(t *testing.T, in string, fallback bool) {
+		got := def.Bool(in, fallback)
+		want, err := strconv.ParseBool(in)
+		if in == "" || err != nil {
+			want = fallback
+		}
+		if got != want {
+			t.Fatalf("Bool(%q, %v) = %v; want %v", in, fallback, got, want)
+		}
+	})
+}
+
+func FuzzDuration(f *testing.F) {
+	for _, tc := range []struct {
+		in  string
+		def time.Duration
+	}{
+		{in: "", def: time.Second},
+		{in: "", def: 0},
+		{in: "2m30s", def: time.Second},
+		{in: "soon", def: time.Second},
+	} {
+		f.Add(tc.in, int64(tc.def))
+	}
+	f.Fuzz(func(t *testing.T, in string, fallbackN int64) {
+		fallback := time.Duration(fallbackN)
+		got := def.Duration(in, fallback)
+		want, err := time.ParseDuration(in)
+		if in == "" || err != nil {
+			want = fallback
+		}
+		if got != want {
+			t.Fatalf("Duration(%q, %v) = %v; want %v", in, fallback, got, want)
+		}
+	})
+}