From 82cfea90ca1f3bbfd8f224d88c939f8264eb2ff1 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 19 Jul 2026 14:06:06 +0000 Subject: [PATCH] all: fix JSON serialization under Go 1.27's finalized encoding/json/v2 Go 1.27 enables GOEXPERIMENT=jsonv2 by default: encoding/json is now backed by the json/v2 machinery, and github.com/go-json-experiment/json compiles as a thin alias of the standard library's encoding/json/v2. Several tag options and behaviors we relied on did not make the cut for the final Go 1.27 API, breaking tailscaled at runtime and four packages' tests. This change adapts to the final API while keeping the wire format byte-for-byte identical on all Go versions. First, the `format` tag option was demoted to experimental. Its mere presence in a struct tag now makes marshaling and unmarshaling fail at runtime. tailcfg.SSHAction.SessionDuration had `format:nano` (added in a2dc517d7 to pin the v1 representation), so on Go 1.27 any netmap containing an SSH policy failed to decode, breaking every PollNetMap. Remove the option here and in net/speedtest; time.Duration still marshals as int64 nanoseconds under encoding/json on all Go versions (Go 1.27's v1 mode sets FormatDurationAsNano by default), so old clients and servers are unaffected. Add a regression test locking in the exact wire format. Consequently, invert the cmd/vet jsontags rule: it previously required an explicit `format` tag on time.Duration fields, which is now exactly wrong. It now rejects any `format` tag option, which would have caught this bug in CI. Second, the `inline` tag option was renamed to `embed`. The standard library silently ignores `inline`, while the pinned go-json-experiment module (used on Go 1.26) only knows `inline`. Specify both options in types/prefs and logtail; each implementation ignores the option it does not know, producing identical output. Drop `inline` once we require Go 1.27. Third, encoding/json (v1) now dispatches to MarshalJSONTo and UnmarshalJSONFrom methods and its v1 options flow into nested jsonv2.MarshalEncode calls. Types whose v1 methods deliberately routed through jsonv2 for v2 semantics (types/opt.Value, the types/prefs preference types) would silently change wire format (e.g. nil slices becoming null). Pin jsonv2.DefaultOptionsV2 in their jsonv2 methods so the representation is the same regardless of the entry point. Finally, json.Marshal costs one more allocation under Go 1.27, tripping the types/logger.AsJSON alloc test. Switch its fmt.Formatter to jsonv2.MarshalWrite with explicit v1 options, which writes directly to the fmt.State: one allocation on both toolchains with unchanged output. Depaware files pick up the go-json-experiment/json/v1 options shim as a new dependency of types/logger. With this change, go test ./... passes with both Go 1.26.5 and go1.27rc2. Updates #20220 Fixes #20528 Fixes #20254 Signed-off-by: Brad Fitzpatrick Change-Id: I694c7d57fd81e55a579c579e9be10032bca569d4 --- cmd/derper/depaware.txt | 2 +- cmd/k8s-operator/depaware.txt | 2 +- cmd/stund/depaware.txt | 1 + cmd/tailscale/depaware.txt | 2 +- cmd/tailscaled/depaware-min.txt | 1 + cmd/tailscaled/depaware-minbox.txt | 1 + cmd/tsidp/depaware.txt | 2 +- cmd/vet/jsontags/analyzer.go | 37 +++++++++--------------------- cmd/vet/jsontags/report.go | 6 ++--- logtail/logtail.go | 9 +++++++- logtail/logtail_omit.go | 2 +- net/speedtest/speedtest.go | 2 +- tailcfg/tailcfg.go | 7 +++++- tailcfg/tailcfg_test.go | 27 ++++++++++++++++++++++ tailcfg/tailcfg_view.go | 5 ++++ tsnet/depaware.txt | 2 +- types/logger/logger.go | 9 +++++--- types/opt/value.go | 9 ++++++-- types/prefs/prefs.go | 20 +++++++++++++--- 19 files changed, 100 insertions(+), 46 deletions(-) diff --git a/cmd/derper/depaware.txt b/cmd/derper/depaware.txt index 9155da0ad..28b20ac56 100644 --- a/cmd/derper/depaware.txt +++ b/cmd/derper/depaware.txt @@ -19,7 +19,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa github.com/go-json-experiment/json/internal/jsonopts from github.com/go-json-experiment/json+ github.com/go-json-experiment/json/internal/jsonwire from github.com/go-json-experiment/json+ github.com/go-json-experiment/json/jsontext from github.com/go-json-experiment/json+ - github.com/go-json-experiment/json/v1 from tailscale.com/net/routecheck + github.com/go-json-experiment/json/v1 from tailscale.com/net/routecheck+ πŸ’£ github.com/go4org/hashtriemap from tailscale.com/derp/derpserver github.com/golang/groupcache/lru from tailscale.com/net/dnscache github.com/hdevalence/ed25519consensus from tailscale.com/tka diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index ce9833d6a..c1dd97db0 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -41,7 +41,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ github.com/go-json-experiment/json/internal/jsonopts from github.com/go-json-experiment/json/jsontext+ github.com/go-json-experiment/json/internal/jsonwire from github.com/go-json-experiment/json/jsontext+ github.com/go-json-experiment/json/jsontext from tailscale.com/logtail+ - github.com/go-json-experiment/json/v1 from tailscale.com/net/routecheck + github.com/go-json-experiment/json/v1 from tailscale.com/net/routecheck+ github.com/go-logr/logr from github.com/go-logr/logr/slogr+ github.com/go-logr/logr/slogr from github.com/go-logr/zapr github.com/go-logr/zapr from sigs.k8s.io/controller-runtime/pkg/log/zap+ diff --git a/cmd/stund/depaware.txt b/cmd/stund/depaware.txt index bdd02b70b..50538b6c3 100644 --- a/cmd/stund/depaware.txt +++ b/cmd/stund/depaware.txt @@ -9,6 +9,7 @@ tailscale.com/cmd/stund dependencies: (generated by github.com/tailscale/depawar github.com/go-json-experiment/json/internal/jsonopts from github.com/go-json-experiment/json+ github.com/go-json-experiment/json/internal/jsonwire from github.com/go-json-experiment/json+ github.com/go-json-experiment/json/jsontext from github.com/go-json-experiment/json+ + github.com/go-json-experiment/json/v1 from tailscale.com/types/logger github.com/munnerz/goautoneg from github.com/prometheus/common/expfmt πŸ’£ github.com/prometheus/client_golang/prometheus from tailscale.com/tsweb/promvarz github.com/prometheus/client_golang/prometheus/internal from github.com/prometheus/client_golang/prometheus diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index db23edc82..8abb5e082 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -125,7 +125,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep github.com/go-json-experiment/json/internal/jsonopts from github.com/go-json-experiment/json+ github.com/go-json-experiment/json/internal/jsonwire from github.com/go-json-experiment/json+ github.com/go-json-experiment/json/jsontext from github.com/go-json-experiment/json+ - github.com/go-json-experiment/json/v1 from tailscale.com/net/routecheck + github.com/go-json-experiment/json/v1 from tailscale.com/net/routecheck+ L πŸ’£ github.com/godbus/dbus/v5 from fyne.io/systray+ L github.com/godbus/dbus/v5/introspect from fyne.io/systray+ L github.com/godbus/dbus/v5/prop from fyne.io/systray diff --git a/cmd/tailscaled/depaware-min.txt b/cmd/tailscaled/depaware-min.txt index 5f599604d..e57c3bbb6 100644 --- a/cmd/tailscaled/depaware-min.txt +++ b/cmd/tailscaled/depaware-min.txt @@ -15,6 +15,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de github.com/go-json-experiment/json/internal/jsonopts from github.com/go-json-experiment/json+ github.com/go-json-experiment/json/internal/jsonwire from github.com/go-json-experiment/json+ github.com/go-json-experiment/json/jsontext from github.com/go-json-experiment/json+ + github.com/go-json-experiment/json/v1 from tailscale.com/types/logger github.com/golang/groupcache/lru from tailscale.com/net/dnscache πŸ’£ github.com/jsimonetti/rtnetlink from tailscale.com/net/netmon github.com/jsimonetti/rtnetlink/internal/unix from github.com/jsimonetti/rtnetlink diff --git a/cmd/tailscaled/depaware-minbox.txt b/cmd/tailscaled/depaware-minbox.txt index 7cb7fc727..e045c0090 100644 --- a/cmd/tailscaled/depaware-minbox.txt +++ b/cmd/tailscaled/depaware-minbox.txt @@ -15,6 +15,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de github.com/go-json-experiment/json/internal/jsonopts from github.com/go-json-experiment/json+ github.com/go-json-experiment/json/internal/jsonwire from github.com/go-json-experiment/json+ github.com/go-json-experiment/json/jsontext from github.com/go-json-experiment/json+ + github.com/go-json-experiment/json/v1 from tailscale.com/types/logger github.com/golang/groupcache/lru from tailscale.com/net/dnscache πŸ’£ github.com/jsimonetti/rtnetlink from tailscale.com/net/netmon github.com/jsimonetti/rtnetlink/internal/unix from github.com/jsimonetti/rtnetlink diff --git a/cmd/tsidp/depaware.txt b/cmd/tsidp/depaware.txt index e252f82fa..c8c6683b6 100644 --- a/cmd/tsidp/depaware.txt +++ b/cmd/tsidp/depaware.txt @@ -30,7 +30,7 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar github.com/go-json-experiment/json/internal/jsonopts from github.com/go-json-experiment/json+ github.com/go-json-experiment/json/internal/jsonwire from github.com/go-json-experiment/json+ github.com/go-json-experiment/json/jsontext from github.com/go-json-experiment/json+ - github.com/go-json-experiment/json/v1 from tailscale.com/net/routecheck + github.com/go-json-experiment/json/v1 from tailscale.com/net/routecheck+ L πŸ’£ github.com/godbus/dbus/v5 from tailscale.com/net/dns github.com/golang/groupcache/lru from tailscale.com/net/dnscache github.com/google/btree from gvisor.dev/gvisor/pkg/tcpip/transport/tcp diff --git a/cmd/vet/jsontags/analyzer.go b/cmd/vet/jsontags/analyzer.go index c69634ecd..5842fc1c1 100644 --- a/cmd/vet/jsontags/analyzer.go +++ b/cmd/vet/jsontags/analyzer.go @@ -38,7 +38,7 @@ func run(pass *analysis.Pass) (any, error) { if tag == "" { continue } - var seenName, hasFormat bool + var seenName bool for opt := range strings.SplitSeq(tag, ",") { if !seenName { seenName = true @@ -116,13 +116,18 @@ func run(pass *analysis.Pass) (any, error) { report(pass, structType, fieldVar, StringOnNonNumericKind) } default: - key, _, ok := strings.Cut(opt, ":") - hasFormat = key == "format" && ok + // The `format` tag option did not make the cut for + // Go 1.27's encoding/json/v2. Its mere presence in a + // struct tag makes both v1 and v2 marshaling fail at + // runtime unless the caller passes the + // jsonv2.ExperimentalSupportFormatTag option. + // See https://go.dev/issue/71631 and + // https://github.com/tailscale/tailscale/issues/20528. + if key, _, ok := strings.Cut(opt, ":"); ok && key == "format" { + report(pass, structType, fieldVar, FormatUnsupported) + } } } - if !hasFormat && isTimeDuration(mayPointerElem(fieldVar.Type())) { - report(pass, structType, fieldVar, FormatMissingOnTimeDuration) - } } }) return nil, nil @@ -152,26 +157,6 @@ func isString(t types.Type) bool { return ok && basic.Kind() == types.String } -// isTimeDuration reports whether t is a time.Duration type. -func isTimeDuration(t types.Type) bool { - return isNamed(t, "time", "Duration") -} - -// mayPointerElem returns the pointed-at type if t is a pointer, -// otherwise it returns t as-is. -func mayPointerElem(t types.Type) types.Type { - if pointer, ok := t.(*types.Pointer); ok { - return pointer.Elem() - } - return t -} - -// isNamed reports t is a named typed of the given path and name. -func isNamed(t types.Type, path, name string) bool { - gotPath, gotName := typeName(t) - return gotPath == path && gotName == name -} - // typeName reports the pkgPath and name of the type. // It recursively follows type aliases to get the underlying named type. func typeName(t types.Type) (pkgPath, name string) { diff --git a/cmd/vet/jsontags/report.go b/cmd/vet/jsontags/report.go index 702de1c4d..022bfc2f1 100644 --- a/cmd/vet/jsontags/report.go +++ b/cmd/vet/jsontags/report.go @@ -74,7 +74,7 @@ func RegisterAllowlist(allowlist map[ReportKind]set.Set[string]) { OmitEmptyShouldBeOmitZero ReportKind = "OmitEmptyShouldBeOmitZero" OmitEmptyShouldBeOmitZeroButHasIsZero ReportKind = "OmitEmptyShouldBeOmitZeroButHasIsZero" StringOnNonNumericKind ReportKind = "StringOnNonNumericKind" - FormatMissingOnTimeDuration ReportKind = "FormatMissingOnTimeDuration" + FormatUnsupported ReportKind = "FormatUnsupported" ) func (k ReportKind) message() string { @@ -89,8 +89,8 @@ func (k ReportKind) message() string { return "should probably use `omitzero` instead of `omitempty`" case StringOnNonNumericKind: return "must not use `string` on non-numeric types" - case FormatMissingOnTimeDuration: - return "must use an explicit `format` tag (e.g., `format:nano`) on a time.Duration type; see https://go.dev/issue/71631" + case FormatUnsupported: + return "must not use the `format` tag option; Go 1.27's encoding/json rejects it at runtime (see tailscale/tailscale#20528)" default: return string(k) } diff --git a/logtail/logtail.go b/logtail/logtail.go index 563745b3a..03bb0f7d7 100644 --- a/logtail/logtail.go +++ b/logtail/logtail.go @@ -186,7 +186,14 @@ type Logtail struct { // differently-shaped payloads in a single upload. type LogEntry[T any] struct { Logtail Logtail `json:"logtail,omitzero"` - Value T `json:",inline"` + // The `inline` tag option was renamed to `embed` in Go 1.27's + // encoding/json/v2, but the pinned go-json-experiment module + // (used on older Go versions) only knows `inline`. Each + // implementation ignores the option it doesn't know, so specify + // both until we require Go 1.27 and drop `inline`. + // + //lint:ignore SA5008 staticcheck doesn't know Go 1.27's `embed` option yet + Value T `json:",inline,embed"` } // UploadLogs uploads entries to the log server described by conf and returns diff --git a/logtail/logtail_omit.go b/logtail/logtail_omit.go index 34cb10337..6a5957430 100644 --- a/logtail/logtail_omit.go +++ b/logtail/logtail_omit.go @@ -28,7 +28,7 @@ type Logtail struct { type LogEntry[T any] struct { Logtail Logtail `json:"logtail,omitzero"` - Value T `json:",inline"` + Value T `json:",inline,embed"` // both options; see the non-omit variant in logtail.go } func UploadLogs[T any](ctx context.Context, conf Config, entries iter.Seq[LogEntry[T]]) error { diff --git a/net/speedtest/speedtest.go b/net/speedtest/speedtest.go index 8b887a8ef..4cd67a410 100644 --- a/net/speedtest/speedtest.go +++ b/net/speedtest/speedtest.go @@ -24,7 +24,7 @@ // conduct the test. type config struct { Version int `json:"version"` - TestDuration time.Duration `json:"time,format:nano"` + TestDuration time.Duration `json:"time"` // int64 nanoseconds; no jsonv2 format tag (tailscale/tailscale#20528) Direction Direction `json:"direction"` } diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index b58fe133d..0b9f99593 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -3088,7 +3088,12 @@ type SSHAction struct { // SessionDuration, if non-zero, is how long the session can stay open // before being forcefully terminated. - SessionDuration time.Duration `json:"sessionDuration,omitempty,format:nano"` + // It is encoded as an int64 of nanoseconds (Go's time.Duration + // wire format for encoding/json v1). It must not use a jsonv2 + // format tag; the mere presence of one makes Go 1.27's + // encoding/json fail to decode the struct. See + // https://github.com/tailscale/tailscale/issues/20528. + SessionDuration time.Duration `json:"sessionDuration,omitempty"` // AllowAgentForwarding, if true, allows accepted connections to forward // the ssh agent if requested. diff --git a/tailcfg/tailcfg_test.go b/tailcfg/tailcfg_test.go index 4a3b9bcff..5658f3111 100644 --- a/tailcfg/tailcfg_test.go +++ b/tailcfg/tailcfg_test.go @@ -1310,3 +1310,30 @@ func TestServiceActionTypeValid(t *testing.T) { } } } + +// TestSSHActionJSON verifies that SSHAction round-trips through +// encoding/json with SessionDuration encoded as int64 nanoseconds. +// It notably guards against jsonv2 `format` tag options in struct +// tags, which Go 1.27's encoding/json rejects at runtime. +// See https://github.com/tailscale/tailscale/issues/20528. +func TestSSHActionJSON(t *testing.T) { + a := SSHAction{ + Accept: true, + SessionDuration: 5 * time.Second, + } + got, err := json.Marshal(a) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + const want = `{"accept":true,"sessionDuration":5000000000}` + if string(got) != want { + t.Errorf("Marshal = %s; want %s", got, want) + } + var back SSHAction + if err := json.Unmarshal(got, &back); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + if !reflect.DeepEqual(back, a) { + t.Errorf("round trip = %+v; want %+v", back, a) + } +} diff --git a/tailcfg/tailcfg_view.go b/tailcfg/tailcfg_view.go index 2423120c6..008c6fcfa 100644 --- a/tailcfg/tailcfg_view.go +++ b/tailcfg/tailcfg_view.go @@ -2107,6 +2107,11 @@ func (v SSHActionView) Accept() bool { return v.ΠΆ.Accept } // SessionDuration, if non-zero, is how long the session can stay open // before being forcefully terminated. +// It is encoded as an int64 of nanoseconds (Go's time.Duration +// wire format for encoding/json v1). It must not use a jsonv2 +// format tag; the mere presence of one makes Go 1.27's +// encoding/json fail to decode the struct. See +// https://github.com/tailscale/tailscale/issues/20528. func (v SSHActionView) SessionDuration() time.Duration { return v.ΠΆ.SessionDuration } // AllowAgentForwarding, if true, allows accepted connections to forward diff --git a/tsnet/depaware.txt b/tsnet/depaware.txt index 954557c3f..0390cb577 100644 --- a/tsnet/depaware.txt +++ b/tsnet/depaware.txt @@ -30,7 +30,7 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware) github.com/go-json-experiment/json/internal/jsonopts from github.com/go-json-experiment/json+ github.com/go-json-experiment/json/internal/jsonwire from github.com/go-json-experiment/json+ github.com/go-json-experiment/json/jsontext from github.com/go-json-experiment/json+ - github.com/go-json-experiment/json/v1 from tailscale.com/net/routecheck + github.com/go-json-experiment/json/v1 from tailscale.com/net/routecheck+ L πŸ’£ github.com/godbus/dbus/v5 from tailscale.com/net/dns github.com/golang/groupcache/lru from tailscale.com/net/dnscache github.com/google/btree from gvisor.dev/gvisor/pkg/tcpip/transport/tcp diff --git a/types/logger/logger.go b/types/logger/logger.go index f76bbf19c..e74cb8a65 100644 --- a/types/logger/logger.go +++ b/types/logger/logger.go @@ -21,6 +21,8 @@ "context" + jsonv2 "github.com/go-json-experiment/json" + jsonv1 "github.com/go-json-experiment/json/v1" "go4.org/mem" "tailscale.com/envknob" "tailscale.com/util/ctxkey" @@ -379,12 +381,13 @@ func AsJSON(v any) fmt.Formatter { type asJSONResult struct{ v any } func (a asJSONResult) Format(s fmt.State, verb rune) { - v, err := json.Marshal(a.v) + // Write directly to s rather than going through json.Marshal's + // returned []byte to avoid an allocation. The explicit v1 + // options keep the output identical to encoding/json. + err := jsonv2.MarshalWrite(s, a.v, jsonv1.DefaultOptionsV1()) if err != nil { fmt.Fprintf(s, "%%!JSON-ERROR:%v", err) - return } - s.Write(v) } // TestLogger returns a logger that logs to tb.Logf diff --git a/types/opt/value.go b/types/opt/value.go index 1ccdd75a4..4acff5690 100644 --- a/types/opt/value.go +++ b/types/opt/value.go @@ -105,7 +105,11 @@ func (o Value[T]) MarshalJSONTo(enc *jsontext.Encoder) error { if !o.set { return enc.WriteToken(jsontext.Null) } - return jsonv2.MarshalEncode(enc, &o.value) + // Pin v2 semantics so the wire format (e.g. nil slices as "[]", + // nil maps as "{}") stays the same even when this method is + // reached via encoding/json v1, which as of Go 1.27 dispatches + // here with v1 options that would otherwise leak into this call. + return jsonv2.MarshalEncode(enc, &o.value, jsonv2.DefaultOptionsV2()) } // UnmarshalJSONFrom implements [jsonv2.UnmarshalerFrom]. @@ -116,7 +120,8 @@ func (o *Value[T]) UnmarshalJSONFrom(dec *jsontext.Decoder) error { return err } o.set = true - return jsonv2.UnmarshalDecode(dec, &o.value) + // Pin v2 semantics; see MarshalJSONTo. + return jsonv2.UnmarshalDecode(dec, &o.value, jsonv2.DefaultOptionsV2()) } // MarshalJSON implements [json.Marshaler]. diff --git a/types/prefs/prefs.go b/types/prefs/prefs.go index 3f18886a7..5b8eedbb8 100644 --- a/types/prefs/prefs.go +++ b/types/prefs/prefs.go @@ -52,7 +52,15 @@ type serializable[T any] struct { // when the preference has not been configured. Default T `json:",omitzero"` // Metadata is any additional type-agnostic preference metadata to be serialized. - Metadata metadata `json:",inline"` + // + // The `inline` tag option was renamed to `embed` in Go 1.27's + // encoding/json/v2, but the pinned go-json-experiment module + // (used on older Go versions) only knows `inline`. Each + // implementation ignores the option it doesn't know, so specify + // both until we require Go 1.27 and drop `inline`. + // + //lint:ignore SA5008 staticcheck doesn't know Go 1.27's `embed` option yet + Metadata metadata `json:",inline,embed"` } // preference is an embeddable type that provides a common implementation for @@ -165,12 +173,18 @@ func (p *preference[T]) SetReadOnly(readonly bool) { // MarshalJSONTo implements [jsonv2.MarshalerTo]. func (p preference[T]) MarshalJSONTo(out *jsontext.Encoder) error { - return jsonv2.MarshalEncode(out, &p.s) + // Pin v2 semantics so the wire format (notably the handling of + // the `inline` and `omitzero` tag options in [serializable]) + // stays the same even when this method is reached via + // encoding/json v1, which as of Go 1.27 dispatches here with v1 + // options that would otherwise leak into this call. + return jsonv2.MarshalEncode(out, &p.s, jsonv2.DefaultOptionsV2()) } // UnmarshalJSONFrom implements [jsonv2.UnmarshalerFrom]. func (p *preference[T]) UnmarshalJSONFrom(in *jsontext.Decoder) error { - return jsonv2.UnmarshalDecode(in, &p.s) + // Pin v2 semantics; see MarshalJSONTo. + return jsonv2.UnmarshalDecode(in, &p.s, jsonv2.DefaultOptionsV2()) } // MarshalJSON implements [json.Marshaler].