crypto/tls: ConnectionState struct can not be json encoded in 1.11rc1

[briankassouf]

What version of Go are you using (go version)?

go1.11rc1

Does this issue reproduce with the latest release?

go1.11rc1 - yes
go1.10.3 - no

What operating system and processor architecture are you using (go env)?

darwin/amd64

What did you do?

package main

import (
	"crypto/tls"
	"encoding/json"
	"fmt"
)

func main() {
	connState := &tls.ConnectionState{}
	_, err := json.Marshal(connState)
	if err != nil {
		fmt.Println(err)
	}
}

Prints json: unsupported type: func(string, []uint8, int) ([]uint8, bool)

What did you expect to see?

in go1.10.3 nothing is printed

[FiloSottile]

@bradfitz @andybons WDYT? Should we add a json:"-" tag? It does solve the issue for encoding/json, but not for all the other reflect based encodings.

Also, does it qualify for backport?

[bradfitz]

Yeah, let's add that json struct tag to that one field only in master & cherry-pick it to Go 1.11.

In the same CL, fix the typo in the documentation. The doc says "ExportKeyMaterial" but the field is named "ExportKeyingMaterial".

[bradfitz]

Actually the ExportKeyingMaterial field is kinda weird. Why isn't it a method on the ConnectionState?

[bradfitz]

Could we unexport that field instead and add a method that used the unexported field?

Then we're back to having no func types in that previously-data-only struct.

And then we don't have to add JSON stuff, and we don't need to worry about XML or protobuf, etc.

[gopherbot]

Change https://golang.org/cl/130535 mentions this issue: crypto/tls: hide ConnectionState.ExportKeyingMaterial from encoding/json

[FiloSottile]

Yeah, I don't see any rationale in https://go-review.googlesource.com/c/go/+/85115, so I tend to agree. Does adding an unexported field to a struct that previously had none cause issues?

[bradfitz]

@FiloSottile, adding an unexported field to a struct without them isn't something we do lightly. (going from fully public to partially private is a big step) But in this case I think it's better.

The other big thing we consider is breaking comparability (notably going from being able to use types as map keys to not being able to). But we already have slices in this, so that's already out the window.

I see no problems with a hidden field here, and I think a method would be better API anyway.