Skip to content

Commit

Permalink
Remove bitmaskCodec (#2792)
Browse files Browse the repository at this point in the history
  • Loading branch information
StephenButtolph authored Mar 6, 2024
1 parent 5793120 commit dc2c5d0
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 94 deletions.
32 changes: 5 additions & 27 deletions network/peer/msg_length.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +12,20 @@ import (
)

var (
errInvalidMaxMessageLength = errors.New("invalid maximum message length")
errInvalidMessageLength = errors.New("invalid message length")
errMaxMessageLengthExceeded = errors.New("maximum message length exceeded")
)

// Used to mask the most significant bit that was used to indicate that the
// message format uses protocol buffers.
//
// TODO: Once the v1.11 is activated, this mask should be removed.
const bitmaskCodec = uint32(1 << 31)

// Assumes the specified [msgLen] will never >= 1<<31.
func writeMsgLen(msgLen uint32, maxMsgLen uint32) ([wrappers.IntLen]byte, error) {
if maxMsgLen >= bitmaskCodec {
if msgLen > maxMsgLen {
return [wrappers.IntLen]byte{}, fmt.Errorf(
"%w; maximum message length must be <%d to be able to embed codec information at most significant bit",
errInvalidMaxMessageLength,
bitmaskCodec,
"%w; the message length %d exceeds the specified limit %d",
errMaxMessageLengthExceeded,
msgLen,
maxMsgLen,
)
}
if msgLen > maxMsgLen {
return [wrappers.IntLen]byte{}, fmt.Errorf("%w; the message length %d exceeds the specified limit %d", errMaxMessageLengthExceeded, msgLen, maxMsgLen)
}

b := [wrappers.IntLen]byte{}
binary.BigEndian.PutUint32(b[:], msgLen)
Expand All @@ -44,13 +35,6 @@ func writeMsgLen(msgLen uint32, maxMsgLen uint32) ([wrappers.IntLen]byte, error)

// Assumes the read [msgLen] will never >= 1<<31.
func readMsgLen(b []byte, maxMsgLen uint32) (uint32, error) {
if maxMsgLen >= bitmaskCodec {
return 0, fmt.Errorf(
"%w; maximum message length must be <%d to be able to embed codec information at most significant bit",
errInvalidMaxMessageLength,
bitmaskCodec,
)
}
if len(b) != wrappers.IntLen {
return 0, fmt.Errorf(
"%w; readMsgLen only supports 4 bytes (got %d bytes)",
Expand All @@ -61,12 +45,6 @@ func readMsgLen(b []byte, maxMsgLen uint32) (uint32, error) {

// parse the message length
msgLen := binary.BigEndian.Uint32(b)

// Because we always use proto messages, there's no need to check the most
// significant bit to inspect the message format. So, we just zero the proto
// flag.
msgLen &^= bitmaskCodec

if msgLen > maxMsgLen {
return 0, fmt.Errorf(
"%w; the message length %d exceeds the specified limit %d",
Expand Down
76 changes: 9 additions & 67 deletions network/peer/msg_length_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,9 @@ func TestWriteMsgLen(t *testing.T) {
expectedErr error
}{
{
msgLen: math.MaxUint32,
msgLimit: math.MaxUint32,
expectedErr: errInvalidMaxMessageLength,
},
{
msgLen: bitmaskCodec,
msgLimit: bitmaskCodec,
expectedErr: errInvalidMaxMessageLength,
},
{
msgLen: bitmaskCodec - 1,
msgLimit: bitmaskCodec - 1,
expectedErr: nil,
msgLen: constants.DefaultMaxMessageSize,
msgLimit: 1,
expectedErr: errMaxMessageLengthExceeded,
},
{
msgLen: constants.DefaultMaxMessageSize,
Expand All @@ -45,11 +35,6 @@ func TestWriteMsgLen(t *testing.T) {
msgLimit: constants.DefaultMaxMessageSize,
expectedErr: nil,
},
{
msgLen: constants.DefaultMaxMessageSize,
msgLimit: 1,
expectedErr: errMaxMessageLengthExceeded,
},
}
for _, tv := range tt {
msgLenBytes, err := writeMsgLen(tv.msgLen, tv.msgLimit)
Expand All @@ -73,39 +58,27 @@ func TestReadMsgLen(t *testing.T) {
expectedErr error
expectedMsgLen uint32
}{
{
msgLenBytes: []byte{0xFF, 0xFF, 0xFF, 0xFF},
msgLimit: math.MaxUint32,
expectedErr: errInvalidMaxMessageLength,
expectedMsgLen: 0,
},
{
msgLenBytes: []byte{0b11111111, 0xFF},
msgLimit: math.MaxInt32,
expectedErr: errInvalidMessageLength,
expectedMsgLen: 0,
},
{
msgLenBytes: []byte{0b11111111, 0xFF, 0xFF, 0xFF},
msgLenBytes: []byte{0xFF, 0xFF, 0xFF, 0xFF},
msgLimit: constants.DefaultMaxMessageSize,
expectedErr: errMaxMessageLengthExceeded,
expectedMsgLen: 0,
},
{
msgLenBytes: []byte{0b11111111, 0xFF, 0xFF, 0xFF},
msgLimit: math.MaxInt32,
expectedErr: nil,
expectedMsgLen: math.MaxInt32,
},
{
msgLenBytes: []byte{0b10000000, 0x00, 0x00, 0x01},
msgLimit: math.MaxInt32,
msgLenBytes: []byte{0xFF, 0xFF, 0xFF, 0xFF},
msgLimit: math.MaxUint32,
expectedErr: nil,
expectedMsgLen: 1,
expectedMsgLen: math.MaxUint32,
},
{
msgLenBytes: []byte{0b10000000, 0x00, 0x00, 0x01},
msgLimit: 1,
msgLenBytes: []byte{0x00, 0x00, 0x00, 0x01},
msgLimit: 10,
expectedErr: nil,
expectedMsgLen: 1,
},
Expand All @@ -126,34 +99,3 @@ func TestReadMsgLen(t *testing.T) {
require.Equal(tv.expectedMsgLen, msgLenAfterWrite)
}
}

func TestBackwardsCompatibleReadMsgLen(t *testing.T) {
require := require.New(t)

tt := []struct {
msgLenBytes []byte
msgLimit uint32
expectedMsgLen uint32
}{
{
msgLenBytes: []byte{0b01111111, 0xFF, 0xFF, 0xFF},
msgLimit: math.MaxInt32,
expectedMsgLen: math.MaxInt32,
},
{
msgLenBytes: []byte{0b00000000, 0x00, 0x00, 0x01},
msgLimit: math.MaxInt32,
expectedMsgLen: 1,
},
{
msgLenBytes: []byte{0b00000000, 0x00, 0x00, 0x01},
msgLimit: 1,
expectedMsgLen: 1,
},
}
for _, tv := range tt {
msgLen, err := readMsgLen(tv.msgLenBytes, tv.msgLimit)
require.NoError(err)
require.Equal(tv.expectedMsgLen, msgLen)
}
}

0 comments on commit dc2c5d0

Please sign in to comment.