Skip to content

Commit

Permalink
[dev.boringcrypto] cmd/compile: refine BoringCrypto kludge
Browse files Browse the repository at this point in the history
Did not consider these fields being embedded or adopted
into structs defined in other packages, but that's possible too.
Refine the import path check to account for that.

Fixes 'go test -short golang.org/x/crypto/ssh' but also
adds a new test in internal/boring for the same problem.

Change-Id: Ied2d04fe2b0ac3b0a34f07bc8dfc50fc203abb9f
Reviewed-on: https://go-review.googlesource.com/62152
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
  • Loading branch information
rsc committed Sep 14, 2017
1 parent 7b49445 commit e8eec3f
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 1 deletion.
9 changes: 8 additions & 1 deletion src/cmd/compile/internal/gc/reflect.go
Original file line number Diff line number Diff line change
Expand Up @@ -1285,7 +1285,14 @@ ok:
// limited to the dev.boringcrypto branch and avoids
// much more invasive effects elsewhere.
omitFieldForAwfulBoringCryptoKludge := func(t *types.Field) bool {
return strings.HasPrefix(myimportpath, "crypto/") && t.Sym != nil && t.Sym.Name == "boring"
if t.Sym == nil || t.Sym.Name != "boring" || t.Sym.Pkg == nil {
return false
}
path := t.Sym.Pkg.Path
if t.Sym.Pkg == localpkg {
path = myimportpath
}
return strings.HasPrefix(path, "crypto/")
}

n := 0
Expand Down
8 changes: 8 additions & 0 deletions src/internal/boringtest/boring.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Copyright 2017 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// Nothing to see here but the tests.
// This file keeps 'go install internal/...' working.

package boring
47 changes: 47 additions & 0 deletions src/internal/boringtest/boring_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Copyright 2017 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// Like crypto/rsa/boring_test.go but outside the crypto/ tree.
// Tests what happens if a package outside the crypto/ tree
// "adopts" a struct definition. This happens in golang.org/x/crypto/ssh.

package boring

import (
"crypto/rand"
"crypto/rsa"
"encoding/asn1"
"reflect"
"testing"
)

type publicKey rsa.PublicKey

func TestBoringASN1Marshal(t *testing.T) {
k, err := rsa.GenerateKey(rand.Reader, 128)
if err != nil {
t.Fatal(err)
}
pk := (*publicKey)(&k.PublicKey)
// This used to fail, because of the unexported 'boring' field.
// Now the compiler hides it [sic].
_, err = asn1.Marshal(*pk)
if err != nil {
t.Fatal(err)
}
}

func TestBoringDeepEqual(t *testing.T) {
k0, err := rsa.GenerateKey(rand.Reader, 128)
if err != nil {
t.Fatal(err)
}
k := (*publicKey)(&k0.PublicKey)
k2 := *k
rsa.EncryptPKCS1v15(rand.Reader, (*rsa.PublicKey)(&k2), []byte("hello")) // initialize hidden boring field
if !reflect.DeepEqual(k, &k2) {
// compiler should be hiding the boring field from reflection
t.Fatalf("DeepEqual compared boring fields")
}
}

0 comments on commit e8eec3f

Please sign in to comment.