From e3963203ab957d9f936d92dd27259bafaa091141 Mon Sep 17 00:00:00 2001
From: Hardik Kaurani <hardikkaurani1@gmail.com>
Date: Mon, 19 Oct 2026 18:33:28 +0530
Subject: [PATCH] fix: preserve numeric protocols in Cisco and Cisco
 object-group ACLs (#170)

When a numeric protocol (e.g. protocol:: 50) is specified in a policy term, cisco.Term.__str__ and cisco.ObjectGroupTerm previously attempted to map it using self.PROTO_MAP.get(proto). Because PROTO_MAP maps names to numbers, it returned None, rendering lines like 'permit None any any'.

Fallback to preserving the protocol number when it is not a protocol name in PROTO_MAP, ensure string protocol numbers resolve to named aliases under IOSXEMap.GetProtocolName when configure_replace_compatible is enabled, and avoid mutating class-level ALLOWED_PROTO_STRINGS on Term initialization.
---
 capirca/lib/cisco.py    | 13 +++++++---
 tests/lib/cisco_test.py | 54 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/capirca/lib/cisco.py b/capirca/lib/cisco.py
index 5c1cd8eb..cd15d8eb 100644
--- a/capirca/lib/cisco.py
+++ b/capirca/lib/cisco.py
@@ -698,7 +698,10 @@ def GetProtocolName(proto, af: int):
       proto_name = IOSXEMap._PROTO_NAMES[af][proto]
       return proto_name
     except KeyError:
-      return proto
+      try:
+        return IOSXEMap._PROTO_NAMES[af][int(proto)]
+      except (KeyError, ValueError):
+        return proto
 
   @staticmethod
   def GetPortName(port, proto: str):
@@ -766,8 +769,10 @@ def __init__(self, term, af=4, proto_int=True, enable_dsmo=False,
       self.text_af = 'inet'
     else:
       self.text_af = 'inet6'
+    self.ALLOWED_PROTO_STRINGS = list(self.ALLOWED_PROTO_STRINGS)
     self.ALLOWED_PROTO_STRINGS.extend([self.PROTO_MAP.get(x)  # pyrefly: ignore[bad-argument-type]
-                                       for x in self.ALWAYS_PROTO_NUM])
+                                       for x in self.ALWAYS_PROTO_NUM
+                                       if self.PROTO_MAP.get(x) not in self.ALLOWED_PROTO_STRINGS])
 
   def __str__(self):
     # Verify platform specific terms. Skip whole term if platform does not
@@ -822,7 +827,7 @@ def __str__(self):
       protocol = ['hbh']
     elif self.proto_int:
       protocol = [proto if proto in self.ALLOWED_PROTO_STRINGS
-                  else self.PROTO_MAP.get(proto)
+                  else self.PROTO_MAP.get(proto, proto)
                   for proto in self.term.protocol]
     else:
       protocol = self.term.protocol
@@ -1225,7 +1230,7 @@ def __str__(self):
 
     else:
       protocol = [proto if proto in self.ALLOWED_PROTO_STRINGS
-                  else self.PROTO_MAP.get(proto)
+                  else self.PROTO_MAP.get(proto, proto)
                   for proto in self.term.protocol]
 
     # addresses
diff --git a/tests/lib/cisco_test.py b/tests/lib/cisco_test.py
index cdb7ffe6..bbf31927 100644
--- a/tests/lib/cisco_test.py
+++ b/tests/lib/cisco_test.py
@@ -469,6 +469,12 @@
   action:: accept
 }
 """
+GOOD_TERM_31_NUMERIC_PROTO = """
+term good_term_31_numeric_proto {
+  protocol:: 50
+  action:: accept
+}
+"""
 
 SUPPORTED_TOKENS = {
     'action',
@@ -1893,5 +1899,53 @@ def testIcmpTypeCodeNames(self):
         )
 
 
+  def testNumericProtocol(self):
+    pol = policy.ParsePolicy(
+        GOOD_HEADER + GOOD_TERM_31_NUMERIC_PROTO, self.naming
+    )
+    acl = cisco.Cisco(pol, EXP_INFO)
+    self.assertIn('permit 50 any any', str(acl))
+    self.assertNotIn('None', str(acl))
+
+  def testNumericProtocolObjectGroup(self):
+    pol = policy.ParsePolicy(
+        GOOD_OBJGRP_HEADER_1 + GOOD_TERM_31_NUMERIC_PROTO, self.naming
+    )
+    acl = cisco.Cisco(pol, EXP_INFO)
+    self.assertIn('permit 50 any any', str(acl))
+    self.assertNotIn('None', str(acl))
+
+  def testNumericProtocolInet6(self):
+    header = """
+header {
+  target:: cisco test-filter inet6
+}
+"""
+    pol = policy.ParsePolicy(header + GOOD_TERM_31_NUMERIC_PROTO, self.naming)
+    acl = cisco.Cisco(pol, EXP_INFO)
+    self.assertIn('permit 50 any any', str(acl))
+    self.assertNotIn('None', str(acl))
+
+  def testNumericProtocolConfigureReplaceCompatible(self):
+    header = """
+header {
+  target:: cisco test-filter extended configure_replace_compatible
+}
+"""
+    pol = policy.ParsePolicy(header + GOOD_TERM_31_NUMERIC_PROTO, self.naming)
+    acl = cisco.Cisco(pol, EXP_INFO)
+    self.assertIn('permit esp any any', str(acl))
+    self.assertNotIn('None', str(acl))
+
+  def testAllowedProtoStringsNotMutated(self):
+    before = list(cisco.Term.ALLOWED_PROTO_STRINGS)
+    pol = policy.ParsePolicy(
+        GOOD_HEADER + GOOD_TERM_31_NUMERIC_PROTO, self.naming
+    )
+    _ = cisco.Cisco(pol, EXP_INFO)
+    _ = cisco.Cisco(pol, EXP_INFO)
+    self.assertEqual(before, cisco.Term.ALLOWED_PROTO_STRINGS)
+
+
 if __name__ == '__main__':
   absltest.main()