Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

encoding/json: Enforcement of string-only key type #17188

Closed
dsoprea opened this issue Sep 22, 2016 · 3 comments

Comments

Projects
None yet
4 participants
@dsoprea
Copy link

commented Sep 22, 2016

So, the JSON requirement for string-only dictionary keys is enforced by Go, which is fine. However, Go does not accommodate any custom encoders/decoders that might help the developer get around this. I was trying to investigate where in the JSON code this became an error in order to see if I could quickly throw-together a PR to add this, but I'm having difficulty. For your convenience, this is a walkthrough of the execution path and why it seems I'm getting mixed signals about whether this should actually be a problem.

I'm running on master (>1.7.1), by the way.

If a map needs to be marshaled, and we don't yet have a cached encoder for a map, newTypeEncoder() creates a mapEncoder struct:

    case reflect.Struct:
        return newStructEncoder(t)
    case reflect.Map:
        return newMapEncoder(t)
    case reflect.Slice:
        return newSliceEncoder(t)

newMapEncoder() seems to approve of a multitude of different key types, not just string:

func newMapEncoder(t reflect.Type) encoderFunc {
    switch t.Key().Kind() {
    case reflect.String,
        reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
        reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
    default:
        if !t.Key().Implements(textMarshalerType) {
            return unsupportedTypeEncoder
        }
    }
    me := &mapEncoder{typeEncoder(t.Elem())}
    return me.encode
}

So, when Marshal() calls marshal() calls reflectValue() calls the cached callback pointing to mapEncoder.encode(), the last collects a list of the keys and calls reflectWithString.resolve() on each:

    sv := make([]reflectWithString, len(keys))
    for i, v := range keys {
        sv[i].v = v
        if err := sv[i].resolve(); err != nil {
            e.error(&MarshalerError{v.Type(), err})
        }
    }
    sort.Sort(byString(sv))

reflectWithString.resolve() also doesn't seem to have a problem with the key being any of a number of different types:

func (w *reflectWithString) resolve() error {
    if w.v.Kind() == reflect.String {
        w.s = w.v.String()
        return nil
    }
    if tm, ok := w.v.Interface().(encoding.TextMarshaler); ok {
        buf, err := tm.MarshalText()
        w.s = string(buf)
        return err
    }
    switch w.v.Kind() {
    case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
        w.s = strconv.FormatInt(w.v.Int(), 10)
        return nil
    case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
        w.s = strconv.FormatUint(w.v.Uint(), 10)
        return nil
    }
    panic("unexpected map key type")

So, what's actually complaining and why?

    PST: {
      I:  921001155220166838 ,
      T:  /* json: unsupported type: map[int64][]int64 */null ,
      L:  /* json: unsupported type: map[int64]*ilfdata.Classifier */null 
    }
};
@cespare

This comment has been minimized.

Copy link
Contributor

commented Sep 22, 2016

So, the JSON requirement for string-only dictionary keys is enforced by Go, which is fine.

encoding/json also automatically marshals/unmarshals maps with integer-typed keys as well as any type that implements TextMarshaler/TextUnmarshaler.

However, Go does not accommodate any custom encoders/decoders that might help the developer get around this.

For even more exotic use cases, you can always implement json.Marshaler/json.Unmarshaler yourself. The output must be valid JSON (e.g., all maps must have string keys).

The fact that you're seeing json: unsupported type: map[int64][]int64 makes me wonder if you're really using Go 1.7+, though. That works just fine as of Go 1.7:

https://play.golang.org/p/1s94vUjxG5

This seems more like a question about how to use encoding/json, so I suggest you post to a forum (https://github.com/golang/go/wiki/Questions) instead.

@dsoprea

This comment has been minimized.

Copy link
Author

commented Sep 22, 2016

I was looking to understand the mechanics and to potentially extend the package. I don't know if the wiki/forum would be sufficient.

I'm running from AppEngine and I had provided a path with the current version of AppEngine, which I had compiled a few minutes before, without realizing that Go had been built-into the SDK (which makes sense). The SDK isn't yet available with something newer than 1.6.2 and the version in production probably isn't available with anything later yet. On the off-chance that you or someone listening happens to know if there's a newer version in production and how to update the AppEngine SDK to use the newer one (it needs the "goapp" binary) than I'm going to be stuck having to pre-translate my keys.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Sep 22, 2016

I don't see a bug here, so closing.

We don't use the issue tracker for questions or discussion. We use the forums for that. The forums are the place to discuss the mechanics of a package, and to discuss possible extensions.

@golang golang locked and limited conversation to collaborators Sep 22, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.