From 4560e447e1970ce173407583fbb430c7664e89dc Mon Sep 17 00:00:00 2001
From: Leonard Lyubich <leonard@nspcc.ru>
Date: Fri, 19 Nov 2021 19:50:29 +0300
Subject: [PATCH] [#356] netmap: Fix potential double-processing of zero subnet

Signed-off-by: Leonard Lyubich <leonard@nspcc.ru>
---
 netmap/attributes.go      | 74 ++++++++++++++++++++++++---------------
 netmap/attributes_test.go | 27 ++++++++++++++
 2 files changed, 72 insertions(+), 29 deletions(-)

diff --git a/netmap/attributes.go b/netmap/attributes.go
index e1be340a..ff01a0d9 100644
--- a/netmap/attributes.go
+++ b/netmap/attributes.go
@@ -126,11 +126,24 @@ var errNoSubnets = errors.New("no subnets")
 func IterateSubnets(node *NodeInfo, f func(refs.SubnetID) error) error {
 	attrs := node.GetAttributes()
 
+	type zeroStatus uint8
+
+	const (
+		_ zeroStatus = iota
+		// missing attribute of zero subnet
+		zeroNoAttr
+		// with `False` attribute
+		zeroExit
+		// with `True` attribute
+		zeroEntry
+	)
+
 	var (
 		err     error
 		id      refs.SubnetID
-		metZero bool // if zero subnet's attribute was met in for-loop
 		entries uint
+
+		stZero = zeroNoAttr
 	)
 
 	for i := 0; i < len(attrs); i++ { // range must not be used because of attrs mutation in body
@@ -144,14 +157,9 @@ func IterateSubnets(node *NodeInfo, f func(refs.SubnetID) error) error {
 		}
 
 		// check value
-		switch val := attrs[i].GetValue(); val {
-		default:
+		val := attrs[i].GetValue()
+		if val != attrSubnetValExit && val != attrSubnetValEntry {
 			return fmt.Errorf("invalid attribute value: %s", val)
-		case attrSubnetValExit:
-			// node is outside the subnet
-			continue
-		case attrSubnetValEntry:
-			// required to avoid default case
 		}
 
 		// decode subnet ID
@@ -159,6 +167,25 @@ func IterateSubnets(node *NodeInfo, f func(refs.SubnetID) error) error {
 			return fmt.Errorf("invalid ID text: %w", err)
 		}
 
+		// update status of zero subnet
+		isZero := refs.IsZeroSubnet(&id)
+
+		if stZero == zeroNoAttr { // in order to not reset if has been already set
+			if isZero {
+				if val == attrSubnetValEntry {
+					// clear True attribute for zero subnet is also possible
+					stZero = zeroEntry
+				} else {
+					stZero = zeroExit
+				}
+			}
+		}
+
+		// continue to process only the subnets to which the node belongs
+		if val == attrSubnetValExit {
+			continue
+		}
+
 		// pass ID to the handler
 		err = f(id)
 
@@ -168,34 +195,23 @@ func IterateSubnets(node *NodeInfo, f func(refs.SubnetID) error) error {
 			return err
 		}
 
-		if !metZero { // in order to not reset if has been already set
-			metZero = refs.IsZeroSubnet(&id)
-
-			if !isRemoveErr {
-				// no handler's error and non-zero subnet
-				entries++
-				continue
-			} else if metZero {
-				// removal error and zero subnet.
-				// we don't remove attribute of zero subnet because it means entry
-				attrs[i].SetValue(attrSubnetValExit)
-
-				continue
-			}
-		}
-
 		if isRemoveErr {
-			// removal error and non-zero subnet.
-			// we can set False or remove attribute, latter is more memory/network efficient.
-			attrs = append(attrs[:i], attrs[i+1:]...)
-			i--
+			if isZero {
+				// we can't remove attribute of zero subnet because it means entry
+				attrs[i].SetValue(attrSubnetValExit)
+			} else {
+				// we can set False or remove attribute, latter is more memory/network efficient.
+				attrs = append(attrs[:i], attrs[i+1:]...)
+				i--
+			}
+
 			continue
 		}
 
 		entries++
 	}
 
-	if !metZero {
+	if stZero == zeroNoAttr {
 		// missing attribute of zero subnet equivalent to entry
 		refs.MakeZeroSubnet(&id)
 
diff --git a/netmap/attributes_test.go b/netmap/attributes_test.go
index 7e03b8cf..f282c868 100644
--- a/netmap/attributes_test.go
+++ b/netmap/attributes_test.go
@@ -264,4 +264,31 @@ func TestSubnets(t *testing.T) {
 			require.Error(t, err)
 		})
 	})
+
+	t.Run("zero subnet removal via attribute", func(t *testing.T) {
+		var (
+			node netmap.NodeInfo
+
+			attrZero, attrOther netmap.Attribute
+		)
+
+		attrZero.SetKey(subnetAttrKey("0"))
+		attrZero.SetValue("False")
+
+		attrOther.SetKey(subnetAttrKey("1"))
+		attrOther.SetValue("True")
+
+		node.SetAttributes([]*netmap.Attribute{&attrZero, &attrOther})
+
+		calledCount := 0
+
+		err := netmap.IterateSubnets(&node, func(id refs.SubnetID) error {
+			require.False(t, refs.IsZeroSubnet(&id))
+			calledCount++
+			return nil
+		})
+
+		require.NoError(t, err)
+		require.EqualValues(t, 1, calledCount)
+	})
 }