From 45151e7f80fdcc958791b347055beb00eb6a5c67 Mon Sep 17 00:00:00 2001 From: Grigory Date: Wed, 9 May 2018 19:16:35 +0300 Subject: [PATCH] Respect nil slices, maps in bson encoder (#147) * socket: only send client metadata once per socket (#105) Periodic cluster synchronisation calls isMaster() which currently resends the "client" metadata every call - the spec specifies: isMaster commands issued after the initial connection handshake MUST NOT contain handshake arguments https://github.com/mongodb/specifications/blob/master/source/mongodb-handshake/handshake.rst#connection-handshake This hotfix prevents subsequent isMaster calls from sending the client metadata again - fixes #101 and fixes #103. Thanks to @changwoo-nam @qhenkart @canthefason @jyoon17 for spotting the initial issue, opening tickets, and having the problem debugged with a PoC fix before I even woke up. * Merge Development (#111) * Brings in a patch on having flusher not suppress errors. (#81) https://github.com/go-mgo/mgo/pull/360 * Fallback to JSON tags when BSON tag isn't present (#91) * Fallback to JSON tags when BSON tag isn't present Cleanup. * Add test to demonstrate tagging fallback. - Test coverage for tagging test. * socket: only send client metadata once per socket Periodic cluster synchronisation calls isMaster() which currently resends the "client" metadata every call - the spec specifies: isMaster commands issued after the initial connection handshake MUST NOT contain handshake arguments https://github.com/mongodb/specifications/blob/master/source/mongodb-handshake/handshake.rst#connection-handshake This hotfix prevents subsequent isMaster calls from sending the client metadata again - fixes #101 and fixes #103. Thanks to @changwoo-nam @qhenkart @canthefason @jyoon17 for spotting the initial issue, opening tickets, and having the problem debugged with a PoC fix before I even woke up. * Cluster abended test 254 (#100) * Add a test that mongo Server gets their abended reset as necessary. See https://github.com/go-mgo/mgo/issues/254 and https://github.com/go-mgo/mgo/pull/255/files * Include the patch from Issue 255. This brings in a test which fails without the patch, and passes with the patch. Still to be tested, manual tcpkill of a socket. * changeStream support (#97) Add $changeStream support * readme: credit @peterdeka and @steve-gray (#110) * Hotfix #120 (#136) * cluster: fix deadlock in cluster synchronisation (#120) For a impressively thorough breakdown of the problem, see: https://github.com/globalsign/mgo/issues/120#issuecomment-371699575 Huge thanks to @dvic and @KJTsanaktsidis for the report and fix. * readme: credit @dvic and @KJTsanaktsidis * added support for marshalling/unmarshalling maps with non-string keys * refactor method receiver * added support for json-compatible support for slices and maps Marshal() func: nil slice or map converts to nil, not empty (initialized with len=0) * fix IsNil on slices and maps format * added godoc * fix sasl empty payload * fix scram-sha-1 auth * revert fix sasl empty payload --- bson/bson_test.go | 56 +++++++++++++++++++++++++++++++++++++++++ bson/compatibility.go | 15 ++++++++++- bson/encode.go | 6 +++++ internal/sasl/sasl.go | 1 + internal/scram/scram.go | 2 +- 5 files changed, 78 insertions(+), 2 deletions(-) diff --git a/bson/bson_test.go b/bson/bson_test.go index 905f48ba1..5a389a498 100644 --- a/bson/bson_test.go +++ b/bson/bson_test.go @@ -1933,3 +1933,59 @@ func (s *S) BenchmarkNewObjectId(c *C) { bson.NewObjectId() } } + +func (s *S) TestMarshalRespectNil(c *C) { + type T struct { + Slice []int + SlicePtr *[]int + Ptr *int + Map map[string]interface{} + MapPtr *map[string]interface{} + } + + bson.SetRespectNilValues(true) + defer bson.SetRespectNilValues(false) + + testStruct1 := T{} + + c.Assert(testStruct1.Slice, IsNil) + c.Assert(testStruct1.SlicePtr, IsNil) + c.Assert(testStruct1.Map, IsNil) + c.Assert(testStruct1.MapPtr, IsNil) + c.Assert(testStruct1.Ptr, IsNil) + + b, _ := bson.Marshal(testStruct1) + + testStruct2 := T{} + + bson.Unmarshal(b, &testStruct2) + + c.Assert(testStruct2.Slice, IsNil) + c.Assert(testStruct2.SlicePtr, IsNil) + c.Assert(testStruct2.Map, IsNil) + c.Assert(testStruct2.MapPtr, IsNil) + c.Assert(testStruct2.Ptr, IsNil) + + testStruct1 = T{ + Slice: []int{}, + SlicePtr: &[]int{}, + Map: map[string]interface{}{}, + MapPtr: &map[string]interface{}{}, + } + + c.Assert(testStruct1.Slice, NotNil) + c.Assert(testStruct1.SlicePtr, NotNil) + c.Assert(testStruct1.Map, NotNil) + c.Assert(testStruct1.MapPtr, NotNil) + + b, _ = bson.Marshal(testStruct1) + + testStruct2 = T{} + + bson.Unmarshal(b, &testStruct2) + + c.Assert(testStruct2.Slice, NotNil) + c.Assert(testStruct2.SlicePtr, NotNil) + c.Assert(testStruct2.Map, NotNil) + c.Assert(testStruct2.MapPtr, NotNil) +} diff --git a/bson/compatibility.go b/bson/compatibility.go index 6afecf53c..66efd465f 100644 --- a/bson/compatibility.go +++ b/bson/compatibility.go @@ -1,7 +1,8 @@ package bson -// Current state of the JSON tag fallback option. +// Current state of the JSON tag fallback option. var useJSONTagFallback = false +var useRespectNilValues = false // SetJSONTagFallback enables or disables the JSON-tag fallback for structure tagging. When this is enabled, structures // without BSON tags on a field will fall-back to using the JSON tag (if present). @@ -14,3 +15,15 @@ func SetJSONTagFallback(state bool) { func JSONTagFallbackState() bool { return useJSONTagFallback } + +// SetRespectNilValues enables or disables serializing nil slices or maps to `null` values. +// In other words it enables `encoding/json` compatible behaviour. +func SetRespectNilValues(state bool) { + useRespectNilValues = state +} + +// RespectNilValuesState returns the current status of the JSON nil slices and maps fallback compatibility option. +// See SetRespectNilValues for more information. +func RespectNilValuesState() bool { + return useRespectNilValues +} diff --git a/bson/encode.go b/bson/encode.go index 5f036edae..d0c6b2a85 100644 --- a/bson/encode.go +++ b/bson/encode.go @@ -245,6 +245,12 @@ func (e *encoder) addStruct(v reflect.Value) { if info.OmitEmpty && isZero(value) { continue } + if useRespectNilValues && + (value.Kind() == reflect.Slice || value.Kind() == reflect.Map) && + value.IsNil() { + e.addElem(info.Key, reflect.ValueOf(nil), info.MinSize) + continue + } e.addElem(info.Key, value, info.MinSize) } } diff --git a/internal/sasl/sasl.go b/internal/sasl/sasl.go index 25a537426..0b56f0b6f 100644 --- a/internal/sasl/sasl.go +++ b/internal/sasl/sasl.go @@ -127,6 +127,7 @@ func (ss *saslSession) Step(serverData []byte) (clientData []byte, done bool, er if rc == C.SASL_CONTINUE { return clientData, false, nil } + return nil, false, saslError(rc, ss.conn, "cannot establish SASL session") } diff --git a/internal/scram/scram.go b/internal/scram/scram.go index d3ddd02fd..03c14daf7 100644 --- a/internal/scram/scram.go +++ b/internal/scram/scram.go @@ -91,7 +91,7 @@ func NewClient(newHash func() hash.Hash, user, pass string) *Client { // Out returns the data to be sent to the server in the current step. func (c *Client) Out() []byte { if c.out.Len() == 0 { - return nil + return []byte{} } return c.out.Bytes() }