code review 5489073: crypto/tls: don't assume an RSA private key in the API.

crypto/tls: don't assume an RSA private key in the API.

We still very much assume it in the code, but with this change in
place we can implement other things later without changing and users
of the package.

Fixes issue 2319.

[agl1, 2011-12-17 17:23:48]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[bradfitz, 2011-12-17 17:40:50]

http://codereview.appspot.com/5489073/diff/3/src/pkg/crypto/tls/common.go
File src/pkg/crypto/tls/common.go (right):

http://codereview.appspot.com/5489073/diff/3/src/pkg/crypto/tls/common.go#new...
src/pkg/crypto/tls/common.go:257: PrivateKey  interface{}
sadness, but understandable.  at least add a comment here about what types
interface{} is expected to be.

[bradfitz, 2011-12-17 17:44:00]

http://codereview.appspot.com/5489073/diff/3/src/pkg/crypto/tls/common.go
File src/pkg/crypto/tls/common.go (right):

http://codereview.appspot.com/5489073/diff/3/src/pkg/crypto/tls/common.go#new...
src/pkg/crypto/tls/common.go:257: PrivateKey  interface{}
talking to myself, but would it be less gross to do something like:

type Certificate struct {
     ....
     PrivateKey  crypto.PrivateKey
     ...
}

package crypto

type PrivateKey interface {
    PrivateKey() interface{}
}


That is, a dummy type just for documentation, less gross than an empty
interface.

Then *rsa.PrivateKey can implement PrivateKey like:

func (k *PrivateKey) PrivateKey() interface{} {
   return k
}

... or something.

[agl1, 2011-12-17 18:33:57]

http://codereview.appspot.com/5489073/diff/3/src/pkg/crypto/tls/common.go
File src/pkg/crypto/tls/common.go (right):

http://codereview.appspot.com/5489073/diff/3/src/pkg/crypto/tls/common.go#new...
src/pkg/crypto/tls/common.go:257: PrivateKey  interface{}
On 2011/12/17 17:44:00, bradfitz wrote:
> talking to myself, but would it be less gross to do something like:

I don't much like the .PrivateKey.PrivateKey().(*rsa.PrivateKey) that occurs
with that, so I did

type PrivateKey interface{}

See how you feel about it.

[r, 2011-12-17 18:53:23]

LGTM

[bradfitz, 2011-12-17 20:22:40]

LGTM

But I'd still put a comment after it in the struct:

PrivateKey  crypto.PrivateKey  // supported types: *rsa.PrivateKey

... or something.

In a follow-up CL, I think there are a couple other places that
crypto.PrivateKey could be used instead of interface{}.


On Sat, Dec 17, 2011 at 10:33 AM, <agl@golang.org> wrote:

>
> http://codereview.appspot.com/**5489073/diff/3/src/pkg/crypto/**
>
tls/common.go<http://codereview.appspot.com/5489073/diff/3/src/pkg/crypto/tls/common.go>
> File src/pkg/crypto/tls/common.go (right):
>
> http://codereview.appspot.com/**5489073/diff/3/src/pkg/crypto/**
>
tls/common.go#newcode257<http://codereview.appspot.com/5489073/diff/3/src/pkg/crypto/tls/common.go#newcode257>
> src/pkg/crypto/tls/common.go:**257: PrivateKey  interface{}
> On 2011/12/17 17:44:00, bradfitz wrote:
>
>> talking to myself, but would it be less gross to do something like:
>>
>
> I don't much like the .PrivateKey.PrivateKey().(***rsa.PrivateKey) that
> occurs with that, so I did
>
> type PrivateKey interface{}
>
> See how you feel about it.
>
>
http://codereview.appspot.com/**5489073/<http://codereview.appspot.com/5489073/>
>

[agl1, 2011-12-19 15:39:38]

*** Submitted as http://code.google.com/p/go/source/detail?r=6392106665e7 ***

crypto/tls: don't assume an RSA private key in the API.

We still very much assume it in the code, but with this change in
place we can implement other things later without changing and users
of the package.

Fixes issue 2319.

R=golang-dev, bradfitz, r
CC=golang-dev
http://codereview.appspot.com/5489073