From 8d3131f90f71271835e5fed91831565797894614 Mon Sep 17 00:00:00 2001
From: Vladimir Lavor <vlavor@cisco.com>
Date: Thu, 3 Dec 2020 14:40:09 +0100
Subject: [PATCH] Decode message context using the message type only

In order to prevent potential future issues, the method
returning message based on its ID but ignoring its package
was optimized.

Change-Id: I12aa2b243f32f38cb3dbc7731613c7ed9fc66539
Signed-off-by: Vladimir Lavor <vlavor@cisco.com>
---
 codec/msg_codec.go      | 12 ++++--------
 core/channel.go         |  4 ++--
 core/request_handler.go | 33 +++++++++------------------------
 3 files changed, 15 insertions(+), 34 deletions(-)

diff --git a/codec/msg_codec.go b/codec/msg_codec.go
index 6534e7d..227764d 100644
--- a/codec/msg_codec.go
+++ b/codec/msg_codec.go
@@ -30,8 +30,8 @@ func EncodeMsg(msg api.Message, msgID uint16) (data []byte, err error) {
 func DecodeMsg(data []byte, msg api.Message) (err error) {
 	return DefaultCodec.DecodeMsg(data, msg)
 }
-func DecodeMsgContext(data []byte, msg api.Message) (context uint32, err error) {
-	return DefaultCodec.DecodeMsgContext(data, msg)
+func DecodeMsgContext(data []byte, msgType api.MessageType) (context uint32, err error) {
+	return DefaultCodec.DecodeMsgContext(data, msgType)
 }
 
 // MsgCodec provides encoding and decoding functionality of `api.Message` structs into/from
@@ -106,12 +106,8 @@ func (*MsgCodec) DecodeMsg(data []byte, msg api.Message) (err error) {
 	return nil
 }
 
-func (*MsgCodec) DecodeMsgContext(data []byte, msg api.Message) (context uint32, err error) {
-	if msg == nil {
-		return 0, errors.New("nil message passed in")
-	}
-
-	switch msg.GetMessageType() {
+func (*MsgCodec) DecodeMsgContext(data []byte, msgType api.MessageType) (context uint32, err error) {
+	switch msgType {
 	case api.RequestMessage:
 		return binary.BigEndian.Uint32(data[6:10]), nil
 	case api.ReplyMessage:
diff --git a/core/channel.go b/core/channel.go
index fbb3e59..4cb5761 100644
--- a/core/channel.go
+++ b/core/channel.go
@@ -37,8 +37,8 @@ type MessageCodec interface {
 	EncodeMsg(msg api.Message, msgID uint16) ([]byte, error)
 	// DecodeMsg decodes binary-encoded data of a message into provided Message structure.
 	DecodeMsg(data []byte, msg api.Message) error
-	// DecodeMsgContext decodes context from message data.
-	DecodeMsgContext(data []byte, msg api.Message) (context uint32, err error)
+	// DecodeMsgContext decodes context from message data and type.
+	DecodeMsgContext(data []byte, msgType api.MessageType) (context uint32, err error)
 }
 
 // MessageIdentifier provides identification of generated API messages.
diff --git a/core/request_handler.go b/core/request_handler.go
index f9d972a..29685f6 100644
--- a/core/request_handler.go
+++ b/core/request_handler.go
@@ -17,7 +17,6 @@ package core
 import (
 	"errors"
 	"fmt"
-	"reflect"
 	"sync/atomic"
 	"time"
 
@@ -210,7 +209,7 @@ func (c *Connection) msgCallback(msgID uint16, data []byte) {
 		return
 	}
 
-	msg, err := c.getMessageByID(msgID)
+	msgType, name, crc, err := c.getMessageDataByID(msgID)
 	if err != nil {
 		log.Warnln(err)
 		return
@@ -221,7 +220,7 @@ func (c *Connection) msgCallback(msgID uint16, data []byte) {
 	// - replies that don't have context as first field (comes as zero)
 	// - events that don't have context at all (comes as non zero)
 	//
-	context, err := c.codec.DecodeMsgContext(data, msg)
+	context, err := c.codec.DecodeMsgContext(data, msgType)
 	if err != nil {
 		log.WithField("msg_id", msgID).Warnf("Unable to decode message context: %v", err)
 		return
@@ -230,14 +229,6 @@ func (c *Connection) msgCallback(msgID uint16, data []byte) {
 	chanID, isMulti, seqNum := unpackRequestContext(context)
 
 	if log.Level == logger.DebugLevel { // for performance reasons - logrus does some processing even if debugs are disabled
-		msg = reflect.New(reflect.TypeOf(msg).Elem()).Interface().(api.Message)
-
-		// decode the message
-		if err = c.codec.DecodeMsg(data, msg); err != nil {
-			err = fmt.Errorf("decoding message failed: %w", err)
-			return
-		}
-
 		log.WithFields(logger.Fields{
 			"context":  context,
 			"msg_id":   msgID,
@@ -245,8 +236,8 @@ func (c *Connection) msgCallback(msgID uint16, data []byte) {
 			"channel":  chanID,
 			"is_multi": isMulti,
 			"seq_num":  seqNum,
-			"msg_crc":  msg.GetCrcString(),
-		}).Debugf("<-- govpp RECEIVE: %s %+v", msg.GetMessageName(), msg)
+			"msg_crc":  crc,
+		}).Debugf("<-- govpp RECEIVE: %s %+v", name)
 	}
 
 	if context == 0 || c.isNotificationMessage(msgID) {
@@ -420,18 +411,12 @@ func compareSeqNumbers(seqNum1, seqNum2 uint16) int {
 	return 1
 }
 
-// Returns first message from any package where the message ID matches
-// Note: the msg is further used only for its MessageType which is not
-// affected by the message's package
-func (c *Connection) getMessageByID(msgID uint16) (msg api.Message, err error) {
-	var ok bool
+// Returns message data based on the message ID not depending on the message path
+func (c *Connection) getMessageDataByID(msgID uint16) (typ api.MessageType, name, crc string, err error) {
 	for _, msgs := range c.msgMapByPath {
-		if msg, ok = msgs[msgID]; ok {
-			break
+		if msg, ok := msgs[msgID]; ok {
+			return msg.GetMessageType(), msg.GetMessageName(), msg.GetCrcString(), nil
 		}
 	}
-	if !ok {
-		return nil, fmt.Errorf("unknown message received, ID: %d", msgID)
-	}
-	return msg, nil
+	return typ, name, crc, fmt.Errorf("unknown message received, ID: %d", msgID)
 }
-- 
2.16.6