Commit ca2eacb1 authored by Levin Zimmermann's avatar Levin Zimmermann

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
parent c93d5dbc
...@@ -33,6 +33,8 @@ const ( ...@@ -33,6 +33,8 @@ const (
FixMap_4 Op = 0b1000_0000 // 1000_XXXX FixMap_4 Op = 0b1000_0000 // 1000_XXXX
FixArray_4 Op = 0b1001_0000 // 1001_XXXX FixArray_4 Op = 0b1001_0000 // 1001_XXXX
Nil Op = 0xc0
False Op = 0xc2 False Op = 0xc2
True Op = 0xc3 True Op = 0xc3
...@@ -285,3 +287,25 @@ func PutMapHead(data []byte, l int) (n int) { ...@@ -285,3 +287,25 @@ func PutMapHead(data []byte, l int) (n int) {
default: panic("len overflows uint32") 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
}
...@@ -185,3 +185,19 @@ func TestMap(t *testing.T) { ...@@ -185,3 +185,19 @@ func TestMap(t *testing.T) {
test1(t, &tEncMapHead{}, tt.l, tt.encoded) 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)})
}
...@@ -131,6 +131,13 @@ var memBuf types.Type // type of mem.Buf ...@@ -131,6 +131,13 @@ var memBuf types.Type // type of mem.Buf
// registry of enums // registry of enums
var enumRegistry = map[types.Type]int{} // type -> enum type serial var enumRegistry = map[types.Type]int{} // type -> enum type serial
// Define INVALID_ID because encoding behaves different depending
// on if we have an INVALID_{TID,OID} 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 // bytes.Buffer + bell & whistles
type Buffer struct { type Buffer struct {
...@@ -967,9 +974,14 @@ func (s *sizerM) genBasic(path string, typ *types.Basic, userType types.Type) { ...@@ -967,9 +974,14 @@ func (s *sizerM) genBasic(path string, typ *types.Basic, userType types.Type) {
upath = fmt.Sprintf("%s(%s)", typ.Name(), upath) 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 { if userType == zodbTid || userType == zodbOid {
s.size.Add(1+1+8) // mbin8 + 8 + [8]data // 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 return
} }
...@@ -1003,12 +1015,18 @@ func (e *encoderM) genBasic(path string, typ *types.Basic, userType types.Type) ...@@ -1003,12 +1015,18 @@ func (e *encoderM) genBasic(path string, typ *types.Basic, userType types.Type)
upath = fmt.Sprintf("%s(%s)", typ.Name(), upath) 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 { 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] = byte(msgpack.Bin8)", e.n); e.n++
e.emit("data[%v] = 8", 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.emit("binary.BigEndian.PutUint64(data[%v:], uint64(%s))", e.n, path)
e.n += 8 e.n += 8
e.resetPos()
e.emit("}")
return return
} }
...@@ -1059,23 +1077,27 @@ func (e *encoderM) genBasic(path string, typ *types.Basic, userType types.Type) ...@@ -1059,23 +1077,27 @@ func (e *encoderM) genBasic(path string, typ *types.Basic, userType types.Type)
} }
// decoder expects <op> // 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("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(" return 0, mdecodeOpErr(%q, op, %s)", d.pathName(assignto), op)
d.emit("}") d.emit("}")
if addOverflow {
d.overflow.Add(1) d.overflow.Add(1)
}
} }
// decoder expects mbin8 l // decoder expects mbin8 l
func (d *decoderM) expectBin8Fix(assignto string, l int) { func (d *decoderM) expectBin8Fix(assignto string, l int, addOverflow bool) {
d.expectOp(assignto, "msgpack.Bin8") d.expectOp(assignto, "msgpack.Bin8", addOverflow)
d.emit("if l := data[%v]; l != %d {", d.n, l); d.n++ 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(" return 0, mdecodeLen8Err(%q, l, %d)", d.pathName(assignto), l)
d.emit("}") d.emit("}")
if addOverflow {
d.overflow.Add(1) d.overflow.Add(1)
}
} }
// decoder expects mfixext1 <enumType> // decoder expects mfixext1 <enumType>
func (d *decoderM) expectEnum(assignto string, enumType int) { 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("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(" return 0, mdecodeEnumTypeErr(%q, enumType, %d)", d.pathName(assignto), enumType)
d.emit("}") d.emit("}")
...@@ -1083,13 +1105,26 @@ func (d *decoderM) expectEnum(assignto string, enumType int) { ...@@ -1083,13 +1105,26 @@ func (d *decoderM) expectEnum(assignto string, enumType int) {
} }
func (d *decoderM) genBasic(assignto string, typ *types.Basic, userType types.Type) { 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 { if userType == zodbTid || userType == zodbOid {
d.expectBin8Fix(assignto, 8) d.resetPos()
d.overflowCheck()
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.emit("%s= %s(binary.BigEndian.Uint64(data[%v:]))", assignto, typeName(userType), d.n)
d.n += 8 d.n += 8
d.overflow.Add(8) d.resetPos()
d.emit("}")
return return
} }
...@@ -1240,7 +1275,7 @@ func (d *decoderM) genArray1(assignto string, typ *types.Array) { ...@@ -1240,7 +1275,7 @@ func (d *decoderM) genArray1(assignto string, typ *types.Array) {
panic("TODO: array1 with > 255 elements") 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.emit("copy(%v[:], data[%v:%v])", assignto, d.n, d.n+l)
d.n += l d.n += l
d.overflow.Add(l) d.overflow.Add(l)
...@@ -1462,6 +1497,17 @@ func (e *encoderCommon) genSliceCommon(xe CodeGenCustomize, path string, typ *ty ...@@ -1462,6 +1497,17 @@ func (e *encoderCommon) genSliceCommon(xe CodeGenCustomize, path string, typ *ty
e.emit("}") 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) { func (d *decoderN) genSliceHead(assignto string, typ *types.Slice, obj types.Object) {
d.genBasic("l:", types.Typ[types.Uint32], nil) d.genBasic("l:", types.Typ[types.Uint32], nil)
} }
......
This source diff could not be displayed because it is too large. You can view the blob instead.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment