MarshalJSON encodes NumberLong's as numbers in some cases

[eljasala]

Currently bson.MarshalJSON encodes numbers lower than 2^53 as numbers rather than as strings. This also includes all valid negative numbers. I guess former was originally intentional optimization while latter was a bug. However, even former appears to be a violation of the spec & it at causes issues with at least mongoimport (error getting extended BSON for document #3: expected $numberLong field to have string value).

It should be pretty simple fix (removing https://github.com/globalsign/mgo/blob/f76e4f9da92ecd56e3be26f5ba92580af1ef97b4/bson/json.go#L315 special case & fixing tests https://github.com/globalsign/mgo/blob/f76e4f9da92ecd56e3be26f5ba92580af1ef97b4/bson/json_test.go#L91), but as this at least somewhat of backwards incompatible change (especially the buggy behavior) & I'm not sure about the project's stance in general regarding them I decided to open issue rather than just submitting a PR.

[domodwyer]

Hi @eljasala

I've not come across this personally but not following the spec is a little worrying - does this behaviour match (or expected by) mongo (I expect not given the mongoimport error)?

Dom

[eljasala]

I assume that at least all of mongodb/mongo-tools are affected (current NumberLong parsing logic seems to date back to 4 year old commit). Mongo shell doesn't support extended JSON as far as I know. I'm not sure about REST interfaces, MongoDB compass & various unofficial tools.

[domodwyer]

Hi @eljasala

Can you describe under what situations you got the above error? I'm struggling to understand the scope of the issue (as no one has brought it up before, I imagine it's not too much of an issue?) and I'm not sure the best way to proceed without breaking backwards compatibility.

If you have any info that could help, that would be great.

Thanks! Dom

---

For what it's worth, I've used the mongo shell with NumberLong(42) for example to store a integer that wasn't string encoded.

[eljasala]

Note that the issue is with $numberLong rather than NumberLong. Former is strict mode extended JSON while latter is mongo shell mode extended JSON. mongoimport supports both, but does not support mgo's current optimized strict JSON mode (mongo shell only supports shell mode, inserting strict mode JSON will cause field names cannot start with $ error).

I ran into this problem when trying to use mgo to export data that's in MongoDB, but our restore pipeline uses mongoimport to get data back when needed. For now I have copied the bson library & made the modification I mentioned to the application's own codebase, but would of course prefer to use upstream version if it becomes available.

Here is a simplified version of what I'm doing:

package main

import (
    "os"

    "github.com/globalsign/mgo"
    "github.com/globalsign/mgo/bson"
)

func main() {
    sess, err := mgo.Dial("127.0.0.1")
    defer sess.Close()
    if err != nil {
        panic(err)
    }
    sess.DB("foo").C("bar").RemoveAll(bson.M{})
    err = sess.DB("foo").C("bar").Insert(
        bson.M{
            "longint": int64(1),
        },
    )
    if err != nil {
        panic(err)
    }

    out, err := os.Create("numberlongtest")
    if err != nil {
        panic(err)
    }
    defer out.Close()

    q := sess.DB("foo").C("bar").Find(bson.M{})
    iter := q.Iter()
    defer iter.Close()
    var result bson.M
    for iter.Next(&result) {
        p, err := bson.MarshalJSON(result)
        if err != nil {
            panic(err)
        }
        _, err = out.Write(p)
        if err != nil {
            panic(err)
        }
    }
}

$ go run ./t2.go
$ cat numberlongtest 
{"_id":{"$oid":"5ae1dc769dc3acc4bbff5d07"},"longint":{"$numberLong":1}}
$ mongoimport --db foo --collection bar2 numberlongtest 
2018-04-26T13:51:59.280+0000    connected to: localhost
2018-04-26T13:51:59.288+0000    Failed: error getting extended BSON for document #0: expected $numberLong field to have string value
2018-04-26T13:51:59.288+0000    imported 0 documents

[domodwyer]

Thanks for the really clear example code - this is definitely a problem! I'm a bit surprised no one has picked up on this before.

Dom