diff --git a/feature/conn25/addrAssignments.go b/feature/conn25/addrAssignments.go index 318cdd5a7..a6c637128 100644 --- a/feature/conn25/addrAssignments.go +++ b/feature/conn25/addrAssignments.go @@ -58,22 +58,16 @@ func (a *addrAssignments) insertWithExpiry(as *addrs, d time.Duration) error { if !as.expiresAt.IsZero() && !as.expiresAt.Before(now) { return errors.New("expiresAt already set") } - // we don't expect for addresses to be reused before expiry - if existing, ok := a.byMagicIP[as.magic]; ok { - if !existing.expiresAt.Before(now) { - return errors.New("byMagicIP key exists") - } + // addresses must be removed (eg by popExpired) before they can be reused + if _, ok := a.byMagicIP[as.magic]; ok { + return errors.New("byMagicIP key exists") } ddst := domainDst{domain: as.domain, dst: as.dst} - if existing, ok := a.byDomainDst[ddst]; ok { - if !existing.expiresAt.Before(now) { - return errors.New("byDomainDst key exists") - } + if _, ok := a.byDomainDst[ddst]; ok { + return errors.New("byDomainDst key exists") } - if existing, ok := a.byTransitIP[as.transit]; ok { - if !existing.expiresAt.Before(now) { - return errors.New("byTransitIP key exists") - } + if _, ok := a.byTransitIP[as.transit]; ok { + return errors.New("byTransitIP key exists") } as.expiresAt = now.Add(d) mak.Set(&a.byMagicIP, as.magic, as) diff --git a/feature/conn25/addrAssignments_test.go b/feature/conn25/addrAssignments_test.go index 262354f55..4811564b1 100644 --- a/feature/conn25/addrAssignments_test.go +++ b/feature/conn25/addrAssignments_test.go @@ -51,20 +51,10 @@ func TestAssignmentsExpire(t *testing.T) { if foundAsAfter.isValid() { t.Fatal("expected zero val") } - // Now we can reuse the addresses + // We should only be able to write old addresses again if they've been removed from the maps (eg with popExpired). err = assignments.insert(as) - if err != nil { - t.Fatal(err) - } - foundAs, ok = assignments.lookupByMagicIP(as.magic) - if !ok { - t.Fatal("expected to find") - } - if foundAs.dst != as.dst { - t.Fatalf("want %v; got %v", as.dst, foundAs.dst) - } - if !foundAs.expiresAt.After(clock.Now()) { - t.Fatalf("expected foundAs to expire after now") + if err == nil { + t.Fatal("expected an error but got nil") } }