VOL-1952 return error if message has missing field
Change-Id: Iaa07f736aa517b793c633d6762933e12488ec7f5
diff --git a/internal/pkg/afrouter/affinity-router.go b/internal/pkg/afrouter/affinity-router.go
index 90c0e7f..c86530f 100644
--- a/internal/pkg/afrouter/affinity-router.go
+++ b/internal/pkg/afrouter/affinity-router.go
@@ -227,6 +227,11 @@
b := proto.NewBuffer([]byte{})
//b.DebugPrint("The Buffer", payload)
for { // Find the route selector field
+ if idx >= len(payload) {
+ err := fmt.Errorf("At end of message, attribute %d not found", fieldId)
+ return "", err
+ }
+
log.Debugf("Decoding afinity value attributeNumber: %d from %v at index %d", fieldId, payload, idx)
log.Debugf("Attempting match with payload: %d, methodTable: %d", payload[idx], fieldId)
if payload[idx]>>3 == fieldId {
diff --git a/internal/pkg/afrouter/affinity-router_test.go b/internal/pkg/afrouter/affinity-router_test.go
index 922c08a..8a34575 100644
--- a/internal/pkg/afrouter/affinity-router_test.go
+++ b/internal/pkg/afrouter/affinity-router_test.go
@@ -393,6 +393,35 @@
assert.EqualError(t, err, "Only integer and string route selectors are permitted")
}
+func TestAffinityRouterDecodeProtoFieldMissingField(t *testing.T) {
+ AffinityTestSetup()
+
+ _, routerConfig := MakeAffinityTestConfig(2, 1)
+
+ _, err := newRouter(routerConfig)
+ assert.Nil(t, err)
+
+ // Get the created AffinityRouter so we can inspect its state
+ aRouter := allRouters["vcoredev_manager"].(AffinityRouter)
+
+ // Pick something to test with lots of field types. Port is a good candidate.
+ portMessage := &voltha_pb.Port{PortNo: 123,
+ Label: "testlabel",
+ Type: 3,
+ DeviceId: "5678",
+ RxPackets: 9876,
+ }
+
+ portData, err := proto.Marshal(portMessage)
+ assert.Nil(t, err)
+
+ // Field #5 is not present.
+ // See VOL-1952. This used to cause a panic.
+
+ _, err = aRouter.decodeProtoField(portData, 5) // field 5 is PortNo
+ assert.EqualError(t, err, "At end of message, attribute 5 not found")
+}
+
// Test setting affinity for a key to a backend
func TestAffinitySetAffinity(t *testing.T) {
AffinityTestSetup()
diff --git a/internal/pkg/afrouter/source-router.go b/internal/pkg/afrouter/source-router.go
index 61754b3..3366dd8 100644
--- a/internal/pkg/afrouter/source-router.go
+++ b/internal/pkg/afrouter/source-router.go
@@ -198,6 +198,11 @@
b := proto.NewBuffer([]byte{})
//b.DebugPrint("The Buffer", payload)
for { // Find the route selector field
+ if idx >= len(payload) {
+ err := fmt.Errorf("At end of message, attribute %d not found", fieldId)
+ return "", err
+ }
+
log.Debugf("Decoding source value attributeNumber: %d from %v at index %d", fieldId, payload, idx)
log.Debugf("Attempting match with payload: %d, methodTable: %d", payload[idx], fieldId)
if payload[idx]>>3 == fieldId {
diff --git a/internal/pkg/afrouter/source-router_test.go b/internal/pkg/afrouter/source-router_test.go
index 3be2a6e..9810ace 100644
--- a/internal/pkg/afrouter/source-router_test.go
+++ b/internal/pkg/afrouter/source-router_test.go
@@ -84,7 +84,7 @@
_, err := newRouter(routerConfig)
assert.Nil(t, err)
- // Get the created AffinityRouter so we can inspect its state
+ // Get the created SourceRouter so we can inspect its state
sourceRouter := allRouters["vcorelogger"].(SourceRouter)
loggingMessage := &common_pb.Logging{Level: 1,
@@ -103,6 +103,25 @@
assert.Equal(t, s, "ro_vcore0.ro_vcore01")
}
+func TestSourceRouterDecodeProtoFieldMissingField(t *testing.T) {
+ _, routerConfig := MakeSourceRouterTestConfig()
+ _, err := newRouter(routerConfig)
+ assert.Nil(t, err)
+
+ // Get the created SourceRouter so we can inspect its state
+ sourceRouter := allRouters["vcorelogger"].(SourceRouter)
+
+ loggingMessage := &common_pb.Logging{Level: 1,
+ PackageName: "default",
+ ComponentName: "ro_vcore0.ro_vcore01"}
+
+ loggingData, err := proto.Marshal(loggingMessage)
+ assert.Nil(t, err)
+
+ _, err = sourceRouter.decodeProtoField(loggingData, 123) // field 123 does not exist
+ assert.EqualError(t, err, "At end of message, attribute 123 not found")
+}
+
func TestSourceRouterRoute(t *testing.T) {
_, routerConfig := MakeSourceRouterTestConfig()
_, err := newRouter(routerConfig)