encoding/gob: don't cache broken encoding engines.

Fixes a situation where a nested bad type would still
permit the outer type to install a working engine, leading
to inconsistent behavior.

Fixes #3273.

R=bsiegert, rsc
CC=golang-dev
https://golang.org/cl/6294067

diff --git a/src/pkg/encoding/gob/encode.go b/src/pkg/encoding/gob/encode.go
index 764351db6d427d0d085c4238d3850ef584642150..e89f68fa0810a6878f9597b28ddb782f7a8a2232 100644 (file)
--- a/src/pkg/encoding/gob/encode.go
+++ b/src/pkg/encoding/gob/encode.go
@@ -704,9 +704,20 @@ func (enc *Encoder) getEncEngine(ut *userTypeInfo) *encEngine {
                error_(err1)
        }
        if info.encoder == nil {
-               // mark this engine as underway before compiling to handle recursive types.
+               // Assign the encEngine now, so recursive types work correctly. But...
                info.encoder = new(encEngine)
+               // ... if we fail to complete building the engine, don't cache the half-built machine.
+               // Doing this here means we won't cache a type that is itself OK but
+               // that contains a nested type that won't compile. The result is consistent
+               // error behavior when Encode is called multiple times on the top-level type.
+               ok := false
+               defer func() {
+                       if !ok {
+                               info.encoder = nil
+                       }
+               }()
                info.encoder = enc.compileEnc(ut)
+               ok = true
        }
        return info.encoder
 }

diff --git a/src/pkg/encoding/gob/encoder_test.go b/src/pkg/encoding/gob/encoder_test.go
index c3c87dc4f1a6c5fcfb847641915c543c5ac0d5e7..35ca6caceabeb7e6bae981ba55b2d18455f9e923 100644 (file)
--- a/src/pkg/encoding/gob/encoder_test.go
+++ b/src/pkg/encoding/gob/encoder_test.go
@@ -780,3 +780,36 @@ func TestNilPointerInsideInterface(t *testing.T) {
                t.Fatal("expected error about nil pointer and interface, got:", errMsg)
        }
 }
+
+type Bug4Public struct {
+       Name   string
+       Secret Bug4Secret
+}
+
+type Bug4Secret struct {
+       a int // error: no exported fields.
+}
+
+// Test that a failed compilation doesn't leave around an executable encoder.
+// Issue 3273.
+func TestMutipleEncodingsOfBadType(t *testing.T) {
+       x := Bug4Public{
+               Name:   "name",
+               Secret: Bug4Secret{1},
+       }
+       buf := new(bytes.Buffer)
+       enc := NewEncoder(buf)
+       err := enc.Encode(x)
+       if err == nil {
+               t.Fatal("first encoding: expected error")
+       }
+       buf.Reset()
+       enc = NewEncoder(buf)
+       err = enc.Encode(x)
+       if err == nil {
+               t.Fatal("second encoding: expected error")
+       }
+       if !strings.Contains(err.Error(), "no exported fields") {
+               t.Errorf("expected error about no exported fields; got %v", err)
+       }
+}