Repository: scionproto/scion
Pull request #3531 (2019-12-16 16:34:55)

Fix removing IPv6 registrations in godispatcher
Analogous to #3451, but now for removing registrations.

This time, I double checked: there should be no further instances of this particular bug. :)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/scionproto/scion/3531)
<!-- Reviewable:end -->



Patch:
diff --git a/go/godispatcher/internal/registration/udptable.go b/go/godispatcher/internal/registration/udptable.go
index d2db76b11..76a13c486 100644
--- a/go/godispatcher/internal/registration/udptable.go
+++ b/go/godispatcher/internal/registration/udptable.go
@@ -121,11 +121,12 @@ func copyIPAddr(ip net.IP) net.IP {
 }
 
 func (t *UDPPortTable) Remove(address *net.UDPAddr) {
-	ipTable, ok := t.v4PortTable[address.Port]
+	portTable := t.getPortTableByIP(address.IP)
+	ipTable, ok := portTable[address.Port]
 	if ok {
 		delete(ipTable, address.IP.String())
 		if len(ipTable) == 0 {
-			delete(t.v4PortTable, address.Port)
+			delete(portTable, address.Port)
 		}
 	}
 }


Test patch:
diff --git a/go/godispatcher/internal/registration/udptable_test.go b/go/godispatcher/internal/registration/udptable_test.go
index a22e9f543..bc9e8fa00 100644
--- a/go/godispatcher/internal/registration/udptable_test.go
+++ b/go/godispatcher/internal/registration/udptable_test.go
@@ -214,6 +214,55 @@ func TestUDPPortTableInsert(t *testing.T) {
 	})
 }
 
+func TestUDPPortTableRemove(t *testing.T) {
+	value := "test value"
+	Convey("", t, func() {
+		Convey("", func() {
+			table := testUDPTableWithPorts(
+				map[int]IPTable{
+					10080: {"10.2.3.4": value},
+					10081: {"0.0.0.0": value}},
+				map[int]IPTable{
+					10082: {docIPv6AddressStr: value},
+					10083: {"::": value}},
+			)
+
+			Convey("Remove non-zero addresses", func() {
+				addrs := []*net.UDPAddr{
+					{IP: net.IP{10, 2, 3, 4}, Port: 10080},
+					{IP: docIPv6Address, Port: 10082},
+				}
+				for _, address := range addrs {
+					_, lookupOk := table.Lookup(address)
+					SoMsg("lookup succeeds before removing", lookupOk, ShouldBeTrue)
+					table.Remove(address)
+					_, lookupOk = table.Lookup(address)
+					SoMsg("lookup fails after removing", lookupOk, ShouldBeFalse)
+				}
+			})
+			Convey("Remove zero address", func() {
+				addrs := map[*net.UDPAddr]net.IP{
+					{IP: net.IPv4zero, Port: 10081}: {10, 1, 2, 3},
+					{IP: net.IPv6zero, Port: 10083}: docIPv6Address,
+				}
+
+				for address, lookupAddress := range addrs {
+					_, lookupOk := table.Lookup(&net.UDPAddr{IP: lookupAddress, Port: address.Port})
+					SoMsg("lookup succeeds before removing", lookupOk, ShouldBeTrue)
+					table.Remove(address)
+					_, lookupOk = table.Lookup(&net.UDPAddr{IP: lookupAddress, Port: address.Port})
+					SoMsg("lookup fails after removing", lookupOk, ShouldBeFalse)
+				}
+			})
+		})
+		Convey("Removing non-existent entry does not panic", func() {
+			table := NewUDPPortTable(minPort, maxPort)
+			address := &net.UDPAddr{IP: net.IP{10, 2, 3, 4}, Port: 666}
+			So(func() { table.Remove(address) }, ShouldNotPanic)
+		})
+	})
+}
+
 func TestUDPPortAllocator(t *testing.T) {
 	address := net.IP{10, 2, 3, 4}
 	value := "test value"