Commit fdaa378f authored by Levin Zimmermann's avatar Levin Zimmermann

go/proto/msgpack: Fix Error -> ErrorCode is int on the wire and not extension type

It was already 'int' before the migration to msgpack:

https://lab.nexedi.com/nexedi/neoppod/-/blob/6332112cb/neo/lib/protocol.py#L653

It is 'int' after the migration to msgpack:

https://lab.nexedi.com/nexedi/neoppod/-/blob/9d0bf97a1/neo/lib/protocol.py#L842

(it's explicitly cast into an integer here)

Another verification that 'ErrorCode' is 'int' on the wire can be found
here:

https://lab.nexedi.com/nexedi/neoppod/-/blob/3ddb6663b/neo/tests/protocol#L76

In the Nexedi NEO protocol specification document v1 this is however not
specified (or I couldn't find it):

https://neo.nexedi.com/P-NEO-Protocol.Specification.2019
parent 32f40833
...@@ -177,7 +177,7 @@ func TestMsgMarshal(t *testing.T) { ...@@ -177,7 +177,7 @@ func TestMsgMarshal(t *testing.T) {
// uint32(N)/enum(M), string // uint32(N)/enum(M), string
{&Error{Code: 0x00000045, Message: "hello"}, {&Error{Code: 0x00000045, Message: "hello"},
"\x00\x00\x00\x45\x00\x00\x00\x05hello", "\x00\x00\x00\x45\x00\x00\x00\x05hello",
hex("92") + hex("d40245") + "\xc4\x05hello", hex("92") + hex("45") + "\xc4\x05hello",
}, },
// Oid, Tid, bool, Checksum, []byte // Oid, Tid, bool, Checksum, []byte
......
...@@ -986,6 +986,12 @@ func (s *sizerM) genBasic(path string, typ *types.Basic, userType types.Type) { ...@@ -986,6 +986,12 @@ func (s *sizerM) genBasic(path string, typ *types.Basic, userType types.Type) {
return return
} }
// Code argument of Error packet is not encoded as enum, but as int
if typeName(userType) == "ErrorCode" {
s.size.AddExpr("msgpack.Uint32Size(%s)", upath)
return
}
// enums are encoded as extensions // enums are encoded as extensions
if _, isEnum := enumRegistry[userType]; isEnum { if _, isEnum := enumRegistry[userType]; isEnum {
s.size.Add(1+1+1) // fixext1 enumType value s.size.Add(1+1+1) // fixext1 enumType value
...@@ -1031,17 +1037,6 @@ func (e *encoderM) genBasic(path string, typ *types.Basic, userType types.Type) ...@@ -1031,17 +1037,6 @@ func (e *encoderM) genBasic(path string, typ *types.Basic, userType types.Type)
return return
} }
// enums are encoded as `fixext1 enumType fixint<value>`
if enum, ok := enumRegistry[userType]; ok {
e.emit("data[%v] = byte(msgpack.FixExt1)", e.n); e.n++
e.emit("data[%v] = %d", e.n, enum); e.n++
e.emit("if !(0 <= %s && %s <= 0x7f) {", path, path) // mposfixint
e.emit(` panic("%s: invalid %s enum value)")`, path, typeName(userType))
e.emit("}")
e.emit("data[%v] = byte(%s)", e.n, path); e.n++
return
}
// mputint emits mput<kind>int<size>(path) // mputint emits mput<kind>int<size>(path)
mputint := func(kind string, size int) { mputint := func(kind string, size int) {
KI := "I" // I or <Kind>i KI := "I" // I or <Kind>i
...@@ -1055,6 +1050,23 @@ func (e *encoderM) genBasic(path string, typ *types.Basic, userType types.Type) ...@@ -1055,6 +1050,23 @@ func (e *encoderM) genBasic(path string, typ *types.Basic, userType types.Type)
e.n = 0 e.n = 0
} }
// Code argument of Error packet is not encoded as enum, but as int
if typeName(userType) == "ErrorCode" {
mputint("U", 32)
return
}
// enums are encoded as `fixext1 enumType fixint<value>`
if enum, ok := enumRegistry[userType]; ok {
e.emit("data[%v] = byte(msgpack.FixExt1)", e.n); e.n++
e.emit("data[%v] = %d", e.n, enum); e.n++
e.emit("if !(0 <= %s && %s <= 0x7f) {", path, path) // mposfixint
e.emit(` panic("%s: invalid %s enum value)")`, path, typeName(userType))
e.emit("}")
e.emit("data[%v] = byte(%s)", e.n, path); e.n++
return
}
switch typ.Kind() { switch typ.Kind() {
case types.Bool: case types.Bool:
e.emit("data[%v] = byte(msgpack.Bool(%s))", e.n, path) e.emit("data[%v] = byte(msgpack.Bool(%s))", e.n, path)
...@@ -1106,43 +1118,6 @@ func (d *decoderM) expectEnum(assignto string, enumType int) { ...@@ -1106,43 +1118,6 @@ 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 or Nil
if userType == zodbTid || userType == zodbOid {
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.n += 8
d.resetPos()
d.emit("}")
return
}
// enums are encoded as `fixext1 enumType fixint<value>`
if enum, ok := enumRegistry[userType]; ok {
d.expectEnum(assignto, enum)
d.emit("{")
d.emit("v := data[%v]", d.n); d.n++
d.emit("if !(0 <= v && v <= 0x7f) {") // mposfixint
d.emit(" return 0, mdecodeEnumValueErr(%q, v)", d.pathName(assignto))
d.emit("}")
d.emit("%s= %s(v)", assignto, typeName(userType))
d.emit("}")
d.overflow.Add(1)
return
}
// v represents basic decoded value casted to user type if needed // v represents basic decoded value casted to user type if needed
v := "v" v := "v"
if userType.Underlying() != userType { if userType.Underlying() != userType {
...@@ -1212,6 +1187,49 @@ func (d *decoderM) genBasic(assignto string, typ *types.Basic, userType types.Ty ...@@ -1212,6 +1187,49 @@ func (d *decoderM) genBasic(assignto string, typ *types.Basic, userType types.Ty
d.emit("}") d.emit("}")
} }
// zodb.Tid and zodb.Oid are encoded as [8]bin or Nil
if userType == zodbTid || userType == zodbOid {
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.n += 8
d.resetPos()
d.emit("}")
return
}
// Code argument of Error packet is not encoded as enum, but as int
if typeName(userType) == "ErrorCode" {
mgetint("", 32, "")
return
}
// enums are encoded as `fixext1 enumType fixint<value>`
if enum, ok := enumRegistry[userType]; ok {
d.expectEnum(assignto, enum)
d.emit("{")
d.emit("v := data[%v]", d.n); d.n++
d.emit("if !(0 <= v && v <= 0x7f) {") // mposfixint
d.emit(" return 0, mdecodeEnumValueErr(%q, v)", d.pathName(assignto))
d.emit("}")
d.emit("%s= %s(v)", assignto, typeName(userType))
d.emit("}")
d.overflow.Add(1)
return
}
// IdTime can be nil ('None' in py), in this case we use // IdTime can be nil ('None' in py), in this case we use
// infinite -1, see // infinite -1, see
// https://lab.nexedi.com/kirr/neo/-/blob/1ad088c8/go/neo/proto/proto.go#L352-357 // https://lab.nexedi.com/kirr/neo/-/blob/1ad088c8/go/neo/proto/proto.go#L352-357
......
...@@ -64,21 +64,19 @@ overflow: ...@@ -64,21 +64,19 @@ overflow:
} }
func (p *Error) neoMsgEncodedLenM() int { func (p *Error) neoMsgEncodedLenM() int {
return 3 + msgpack.ArrayHeadSize(reflect.TypeOf((*p)).NumField()) + msgpack.BinHeadSize(len(p.Message)) + len(p.Message) return msgpack.ArrayHeadSize(reflect.TypeOf((*p)).NumField()) + msgpack.Uint32Size(uint32(p.Code)) + msgpack.BinHeadSize(len(p.Message)) + len(p.Message)
} }
func (p *Error) neoMsgEncodeM(data []byte) { func (p *Error) neoMsgEncodeM(data []byte) {
data[0] = byte(msgpack.FixArray_4 | 2) data[0] = byte(msgpack.FixArray_4 | 2)
data[1] = byte(msgpack.FixExt1) {
data[2] = 2 n := msgpack.PutUint32(data[1:], uint32(p.Code))
if !(0 <= p.Code && p.Code <= 0x7f) { data = data[1+n:]
panic("p.Code: invalid ErrorCode enum value)")
} }
data[3] = byte(p.Code)
{ {
l := len(p.Message) l := len(p.Message)
n := msgpack.PutBinHead(data[4:], l) n := msgpack.PutBinHead(data[0:], l)
data = data[4+n:] data = data[0+n:]
copy(data, p.Message) copy(data, p.Message)
data = data[l:] data = data[l:]
} }
...@@ -94,23 +92,15 @@ func (p *Error) neoMsgDecodeM(data []byte) (int, error) { ...@@ -94,23 +92,15 @@ func (p *Error) neoMsgDecodeM(data []byte) (int, error) {
nread += uint64(len(data) - len(tail)) nread += uint64(len(data) - len(tail))
data = tail data = tail
} }
if len(data) < 3 {
goto overflow
}
if op := msgpack.Op(data[0]); op != msgpack.FixExt1 {
return 0, mdecodeOpErr("Error.Code", op, msgpack.FixExt1)
}
if enumType := data[1]; enumType != 2 {
return 0, mdecodeEnumTypeErr("Error.Code", enumType, 2)
}
{ {
v := data[2] v, tail, err := msgp.ReadInt32Bytes(data)
if !(0 <= v && v <= 0x7f) { if err != nil {
return 0, mdecodeEnumValueErr("Error.Code", v) return 0, mdecodeErr("Error.Code", err)
} }
p.Code = ErrorCode(v) p.Code = ErrorCode(v)
nread += uint64(len(data) - len(tail))
data = tail
} }
data = data[3:]
{ {
b, tail, err := msgp.ReadBytesZC(data) b, tail, err := msgp.ReadBytesZC(data)
if err != nil { if err != nil {
...@@ -120,10 +110,7 @@ func (p *Error) neoMsgDecodeM(data []byte) (int, error) { ...@@ -120,10 +110,7 @@ func (p *Error) neoMsgDecodeM(data []byte) (int, error) {
nread += uint64(len(data) - len(tail)) nread += uint64(len(data) - len(tail))
data = tail data = tail
} }
return 3 + int(nread), nil return 0 + int(nread), nil
overflow:
return 0, &ErrDecodeOverflow{"Error"}
} }
// 1. RequestIdentification // 1. RequestIdentification
......
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