proto/msgpack: Fix {de,en}coding INVALID_{TID,OID}

In pre-msgpack protocol an 'INVALID_{TID,OID}' was always
decoded as 'None' in NEO/py [1]. But in msgpack protocol
this isn't true anymore. An `INVALID_TID` is now decoded
as an `INVALID_TID`. And this then leads to errors later [2].
We fix this by encoding 'INVALID_{TID,OID}' to NIL on the
wire and by decoding NIL to 'INVALID_{TID,OID}'.

---

[1] https://lab.nexedi.com/nexedi/neoppod/-/blob/6332112cba979dfd29b40fe9f98d097911fde696/neo/lib/protocol.py#L579-583
[2] With SQLite backend we can see the following exception:

Traceback (most recent call last):
  File "/home/levin/neo/neo/tests/functional/__init__.py", line 192, in start
    self.run()
  File "/home/levin/neo/neo/tests/functional/__init__.py", line 288, in run
    getattr(neo.scripts,  self.command).main()
  File "/home/levin/neo/neo/scripts/neostorage.py", line 32, in main
    app.run()
  File "/home/levin/neo/neo/storage/app.py", line 196, in run
    self._run()
  File "/home/levin/neo/neo/storage/app.py", line 227, in _run
    self.doOperation()
  File "/home/levin/neo/neo/storage/app.py", line 301, in doOperation
    poll()
  File "/home/levin/neo/neo/storage/app.py", line 145, in _poll
    self.em.poll(1)
  File "/home/levin/neo/neo/lib/event.py", line 160, in poll
    to_process.process()
  File "/home/levin/neo/neo/lib/connection.py", line 508, in process
    self._handlers.handle(self, self._queue.pop(0))
  File "/home/levin/neo/neo/lib/connection.py", line 93, in handle
    self._handle(connection, packet)
  File "/home/levin/neo/neo/lib/connection.py", line 108, in _handle
    pending[0][1].packetReceived(connection, packet)
  File "/home/levin/neo/neo/lib/handler.py", line 125, in packetReceived
    self.dispatch(*args)
  File "/home/levin/neo/neo/lib/handler.py", line 75, in dispatch
    method(conn, *args, **kw)
  File "/home/levin/neo/neo/storage/handlers/client.py", line 67, in askObject
    o = app.dm.getObject(oid, at, before)
  File "/home/levin/neo/neo/storage/database/manager.py", line 484, in getObject
    before_tid and u64(before_tid))
  File "/home/levin/neo/neo/storage/database/sqlite.py", line 336, in _getObject
    r = q(sql + ' AND tid=?', (partition, oid, tid))
OverflowError: Python int too large to convert to SQLite INTEGER

go/neo/internal/msgpack/msgpack.go

@@ -33,6 +33,8 @@ const (
 	FixMap_4   Op  = 0b1000_0000 // 1000_XXXX
 	FixArray_4 Op  = 0b1001_0000 // 1001_XXXX
 
+	Nil      Op  = 0xc0
+
 	False	 Op  = 0xc2
 	True	 Op  = 0xc3
 
@@ -285,3 +287,25 @@ func PutMapHead(data []byte, l int) (n int) {
 	default: panic("len overflows uint32")
 	}
 }
+
+// TidOrOidSize returns expected size of encoded TID or OID.
+// This size depends on if the {TID,OID} is valid or not,
+// because invalid {TID,OID} are encoded as NIL on the wire.
+func TidOrOidSize(data []byte) (n uint64) {
+	if len(data) < 1 {
+		// We already know the data can't contain any
+		// {TID,OID}, however we defer the error to
+		// the overflow checker in zproto-marshal.go
+		n = 1
+	// INVALID_{TID,OID}
+	} else if data[0] == byte(Nil) {
+		n = 1
+	// valid {TID,OID} must be encoded in 10 byte
+	// 1:    msgpack.Bin8 header
+	// 2:    msgpack.Len8
+	// 3-10: binary.BigEndian.Uint64
+	} else {
+		n = 10
+	}
+	return
+}

go/neo/internal/msgpack/msgpack_test.go

@@ -185,3 +185,19 @@ func TestMap(t *testing.T) {
 		test1(t, &tEncMapHead{}, tt.l, tt.encoded)
 	}
 }
+
+func TestTidOrOidSize(t *testing.T) {
+	ts := func (wanted int, value []byte) {
+		v := TidOrOidSize(value)
+		if v != uint64(wanted) {
+			t.Errorf("wanted: %v; got: %v", wanted, v)
+		}
+	}
+
+	// Overflow
+	ts(1, []byte {})
+	// INVALID_{TID,OID}
+	ts(1, []byte {byte(Nil)})
+	// valid {TID,OID}
+	ts(10, []byte {byte(1), byte(2)})
+}

go/neo/proto/protogen.go

@@ -131,6 +131,13 @@ var memBuf           types.Type       // type of mem.Buf
 // registry of enums
 var enumRegistry = map[types.Type]int{} // type -> enum type serial
 
+// Define INVALID_TID because encoding behaves different depending
+// on if we have an INVALID_TID or not.
+//
+// NOTE This assumes that INVALID_OID == INVALID_TID
+//
+// XXX Duplication wrt proto.go
+const INVALID_ID uint64 = 1<<64 - 1
 
 // bytes.Buffer + bell & whistles
 type Buffer struct {
@@ -967,10 +974,15 @@ func (s *sizerM) genBasic(path string, typ *types.Basic, userType types.Type) {
 		upath = fmt.Sprintf("%s(%s)", typ.Name(), upath)
 	}
 
-	// zodb.Tid and zodb.Oid are encoded as [8]bin	XXX or nil for INVALID_{TID_OID}
+	// zodb.Tid and zodb.Oid are encoded as [8]bin or nil for INVALID_{TID_OID}
 	if userType == zodbTid || userType == zodbOid {
-		 s.size.Add(1+1+8) // mbin8 + 8 + [8]data
-		 return
+		// INVALID_{TID,OID} must be NIL on the wire
+		s.emit("if uint64(%s) == %v {", path, INVALID_ID)
+		s.emit("%v += 1 // mnil", s.var_("size"))
+		s.emit("} else {")
+		s.emit("%v += 1+1+8 // mbin8 + 8 + [8]data", s.var_("size"))
+		s.emit("}")
+		return
 	}
 
 	// enums are encoded as extensions
@@ -1003,12 +1015,18 @@ func (e *encoderM) genBasic(path string, typ *types.Basic, userType types.Type)
 		upath = fmt.Sprintf("%s(%s)", typ.Name(), upath)
 	}
 
-	// zodb.Tid and zodb.Oid are encoded as [8]bin	XXX or nil
+	// zodb.Tid and zodb.Oid are encoded as [8]bin or nil
 	if userType == zodbTid || userType == zodbOid {
+		e.emit("if %s == %v {", path, INVALID_ID)   // INVALID_{TID,OID} =>
+		e.emit("data[%v] = byte(msgpack.Nil)", e.n) // mnil
+		e.emit("data = data[%v:]", e.n + 1)
+		e.emit("} else {")
 		e.emit("data[%v] = byte(msgpack.Bin8)", e.n);  e.n++
 		e.emit("data[%v] = 8",     e.n);               e.n++
 		e.emit("binary.BigEndian.PutUint64(data[%v:], uint64(%s))", e.n, path)
 		e.n += 8
+		e.resetPos()
+		e.emit("}")
 		return
 	}
 
@@ -1059,23 +1077,27 @@ func (e *encoderM) genBasic(path string, typ *types.Basic, userType types.Type)
 }
 
 // decoder expects <op>
-func (d *decoderM) expectOp(assignto string, op string) {
+func (d *decoderM) expectOp(assignto string, op string, addOverflow bool) {
 	d.emit("if op := msgpack.Op(data[%v]); op != %s {", d.n, op);  d.n++
 	d.emit("  return 0, mdecodeOpErr(%q, op, %s)", d.pathName(assignto), op)
 	d.emit("}")
-	d.overflow.Add(1)
+	if addOverflow {
+		d.overflow.Add(1)
+	}
 }
 // decoder expects mbin8 l
-func (d *decoderM) expectBin8Fix(assignto string, l int) {
-	d.expectOp(assignto, "msgpack.Bin8")
+func (d *decoderM) expectBin8Fix(assignto string, l int, addOverflow bool) {
+	d.expectOp(assignto, "msgpack.Bin8", addOverflow)
 	d.emit("if l := data[%v]; l != %d {", d.n, l);  d.n++
 	d.emit("  return 0, mdecodeLen8Err(%q, l, %d)", d.pathName(assignto), l)
 	d.emit("}")
-	d.overflow.Add(1)
+	if addOverflow {
+		d.overflow.Add(1)
+	}
 }
 // decoder expects mfixext1 <enumType>
 func (d *decoderM) expectEnum(assignto string, enumType int) {
-	d.expectOp(assignto, "msgpack.FixExt1")
+	d.expectOp(assignto, "msgpack.FixExt1", true)
 	d.emit("if enumType := data[%v]; enumType != %d {", d.n, enumType);  d.n++
 	d.emit("  return 0, mdecodeEnumTypeErr(%q, enumType, %d)", d.pathName(assignto), enumType)
 	d.emit("}")
@@ -1083,13 +1105,26 @@ func (d *decoderM) expectEnum(assignto string, enumType int) {
 }
 
 func (d *decoderM) genBasic(assignto string, typ *types.Basic, userType types.Type) {
-	// zodb.Tid and zodb.Oid are encoded as [8]bin
+	// zodb.Tid and zodb.Oid are encoded as [8]bin or Nil
 	if userType == zodbTid || userType == zodbOid {
-		d.expectBin8Fix(assignto, 8)
+		d.overflowCheck()
+		d.resetPos()
+		defer d.overflowCheck()
+
+		// Size depends on if we have an INVALID_{TID,OID}
+		// which is encoded as NIL or if we have a valid
+		// {TID,OID} which is encdoed as Bin8Fix.
+		d.overflow.AddExpr("msgpack.TidOrOidSize(data)")
+		d.emit("if data[%v] == byte(msgpack.Nil) {", d.n)
+		d.emit("%s = %s(%v)", assignto, typeName(userType), INVALID_ID)
+		d.n += 1
+		d.resetPos()
+		d.emit("} else {")
+		d.expectBin8Fix(assignto, 8, false)
 		d.emit("%s= %s(binary.BigEndian.Uint64(data[%v:]))", assignto, typeName(userType), d.n)
 		d.n += 8
-		d.overflow.Add(8)
-
+		d.resetPos()
+		d.emit("}")
 		return
 	}
 
@@ -1240,7 +1275,7 @@ func (d *decoderM) genArray1(assignto string, typ *types.Array) {
 		panic("TODO: array1 with > 255 elements")
 	}
 
-	d.expectBin8Fix(assignto, l)
+	d.expectBin8Fix(assignto, l, true)
 	d.emit("copy(%v[:], data[%v:%v])", assignto, d.n, d.n+l)
 	d.n += l
 	d.overflow.Add(l)
@@ -1462,6 +1497,17 @@ func (e *encoderCommon) genSliceCommon(xe CodeGenCustomize, path string, typ *ty
 	e.emit("}")
 }
 
+// data = data[n:]
+// n    = 0
+//
+// XXX duplication wrt decoderCommon.resetPost
+func (e *encoderCommon) resetPos() {
+	if e.n != 0 {
+		e.emit("data = data[%v:]", e.n)
+		e.n = 0
+	}
+}
+
 func (d *decoderN) genSliceHead(assignto string, typ *types.Slice, obj types.Object) {
 	d.genBasic("l:", types.Typ[types.Uint32], nil)
 }