Skip to content

Commit

Permalink
encoding/gob: make x509.Certificate marshalable again
Browse files Browse the repository at this point in the history
The OID type is not exported data like most of the other x509 structs.
Using it in x509.Certificate made Certificate not gob-compatible anymore,
which breaks real-world code. As a temporary fix, make gob ignore
that field, making it work as well as it did in Go 1.21.

For Go 1.23, we anticipate adding a proper fix and removing the gob
workaround. See #65633 and #66249 for more details.

For #66249.
Fixes #65633.

Change-Id: Idd1431d15063b3009e15d0565cd3120b9fa13f61
Reviewed-on: https://go-review.googlesource.com/c/go/+/571095
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
  • Loading branch information
rsc committed Mar 14, 2024
1 parent 4a1038f commit 376be64
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 3 deletions.
1 change: 1 addition & 0 deletions src/crypto/x509/x509.go
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,7 @@ type Certificate struct {
PolicyIdentifiers []asn1.ObjectIdentifier

// Policies contains all policy identifiers included in the certificate.
// In Go 1.22, encoding/gob cannot handle and ignores this field.
Policies []OID
}

Expand Down
11 changes: 11 additions & 0 deletions src/crypto/x509/x509_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"crypto/x509/pkix"
"encoding/asn1"
"encoding/base64"
"encoding/gob"
"encoding/hex"
"encoding/pem"
"fmt"
Expand Down Expand Up @@ -3999,3 +4000,13 @@ func TestCertificatePoliciesGODEBUG(t *testing.T) {
t.Errorf("cert.Policies = %v, want: %v", cert.Policies, expectPolicies)
}
}

func TestGob(t *testing.T) {
// Test that gob does not reject Certificate.
// See go.dev/issue/65633.
cert := new(Certificate)
err := gob.NewEncoder(io.Discard).Encode(cert)
if err != nil {
t.Fatal(err)
}
}
2 changes: 1 addition & 1 deletion src/encoding/gob/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ func compileEnc(ut *userTypeInfo, building map[*typeInfo]bool) *encEngine {
if ut.externalEnc == 0 && srt.Kind() == reflect.Struct {
for fieldNum, wireFieldNum := 0, 0; fieldNum < srt.NumField(); fieldNum++ {
f := srt.Field(fieldNum)
if !isSent(&f) {
if !isSent(srt, &f) {
continue
}
op, indir := encOpFor(f.Type, seen, building)
Expand Down
15 changes: 13 additions & 2 deletions src/encoding/gob/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ func newTypeObject(name string, ut *userTypeInfo, rt reflect.Type) (gobType, err
idToTypeSlice[st.id()] = st
for i := 0; i < t.NumField(); i++ {
f := t.Field(i)
if !isSent(&f) {
if !isSent(t, &f) {
continue
}
typ := userType(f.Type).base
Expand Down Expand Up @@ -576,7 +576,7 @@ func isExported(name string) bool {
// isSent reports whether this struct field is to be transmitted.
// It will be transmitted only if it is exported and not a chan or func field
// or pointer to chan or func.
func isSent(field *reflect.StructField) bool {
func isSent(struct_ reflect.Type, field *reflect.StructField) bool {
if !isExported(field.Name) {
return false
}
Expand All @@ -589,6 +589,17 @@ func isSent(field *reflect.StructField) bool {
if typ.Kind() == reflect.Chan || typ.Kind() == reflect.Func {
return false
}

// Special case for Go 1.22: the x509.Certificate.Policies
// field is unencodable but also unused by default.
// Ignore it, so that x509.Certificate continues to be encodeable.
// Go 1.23 will add the right methods so that gob can
// handle the Policies field, and then we can remove this check.
// See go.dev/issue/65633.
if field.Name == "Policies" && struct_.PkgPath() == "crypto/x509" && struct_.Name() == "Certificate" {
return false
}

return true
}

Expand Down

0 comments on commit 376be64

Please sign in to comment.