all: imp code

CHANGELOG.md

@@ -20,6 +20,8 @@ NOTE: Add new changes BELOW THIS COMMENT.
 
 ### Fixed
 
+- Searching for persistent clients by CIDR in the `POST /clients/search HTTP API`.
+
 - Rules with the `client` modifier not working ([#7708]).
 
 - The search form not working in the query log ([#7704]).

internal/client/index.go

@@ -283,15 +283,13 @@ func (ci *index) findByIP(ip netip.Addr) (c *Persistent, found bool) {
 // identifier.
 func (ci *index) findByCIDR(subnet netip.Prefix) (c *Persistent, ok bool) {
 	var uid UID
-	ci.subnetToUID.Range(func(pref netip.Prefix, id UID) (cont bool) {
+	for pref, id := range ci.subnetToUID.Range {
 		if subnet == pref {
 			uid, ok = id, true
 
-			return false
+			break
 		}
-
-		return true
-	})
+	}
 
 	if ok {
 		return ci.uidToClient[uid], true

internal/client/persistent.go

@@ -256,7 +256,7 @@ func ValidateClientID(id string) (err error) {
 	return nil
 }
 
-// IDs returns a list of ClientIDs containing at least one element.
+// IDs returns a list of client identifiers containing at least one element.
 func (c *Persistent) IDs() (ids []string) {
 	ids = make([]string, 0, c.IDsLen())
 

internal/client/storage.go

@@ -440,22 +440,8 @@ func (s *Storage) Add(ctx context.Context, p *Persistent) (err error) {
 	return nil
 }
 
-// FindByName finds persistent client by name.  And returns its shallow copy.
-func (s *Storage) FindByName(name string) (p *Persistent, ok bool) {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-
-	p, ok = s.index.findByName(name)
-	if ok {
-		return p.ShallowClone(), ok
-	}
-
-	return nil, false
-}
-
-// FindParams represents the parameters for searching a client.
-//
-// TODO(s.chzhen):  Add a UID field.
+// FindParams represents the parameters for searching a client.  At least one
+// field must be non-empty.
 type FindParams struct {
 	// ClientID is a unique identifier for the client used in DoH, DoT, and DoQ
 	// DNS queries.
@@ -469,15 +455,24 @@ type FindParams struct {
 
 	// MAC is the physical hardware address used as a client search parameter.
 	MAC net.HardwareAddr
+
+	// UID is the unique ID of persistent client used as a search parameter.
+	//
+	// TODO(s.chzhen):  Use this.
+	UID UID
 }
 
 // ParseFindParams parses a string representation of the search parameter into
 // typed search parameters.
+//
+// TODO(s.chzhen):  Add support for UID.
 func ParseFindParams(id string) (params *FindParams) {
 	params = &FindParams{}
 
 	isClientID := true
 	addr, err := netip.ParseAddr(id)
+	// Even if id can be parsed as an IP address, it may be a MAC address.  So
+	// do not return prematurely, continue parsing.
 	if err == nil {
 		params.RemoteIP = addr
 		isClientID = false
@@ -502,16 +497,15 @@ func ParseFindParams(id string) (params *FindParams) {
 	return params
 }
 
-// FindParams represents the parameters for searching a client.  At least one
+// Find represents the parameters for searching a client.  At least one
 // field must be non-empty.
-func (s *Storage) FindParams(params *FindParams) (p *Persistent, ok bool) {
+func (s *Storage) Find(params *FindParams) (p *Persistent, ok bool) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
 	if params.ClientID != "" {
 		p, ok = s.index.findByClientID(params.ClientID)
 	}
-
 	if ok {
 		return p.ShallowClone(), true
 	}
@@ -519,7 +513,6 @@ func (s *Storage) FindParams(params *FindParams) (p *Persistent, ok bool) {
 	if params.RemoteIP != (netip.Addr{}) {
 		p, ok = s.findByIP(params.RemoteIP)
 	}
-
 	if ok {
 		return p.ShallowClone(), true
 	}
@@ -527,7 +520,6 @@ func (s *Storage) FindParams(params *FindParams) (p *Persistent, ok bool) {
 	if params.Subnet != (netip.Prefix{}) {
 		p, ok = s.index.findByCIDR(params.Subnet)
 	}
-
 	if ok {
 		return p.ShallowClone(), true
 	}
@@ -535,7 +527,6 @@ func (s *Storage) FindParams(params *FindParams) (p *Persistent, ok bool) {
 	if params.MAC != nil {
 		p, ok = s.index.findByMAC(params.MAC)
 	}
-
 	if ok {
 		return p.ShallowClone(), true
 	}
@@ -559,32 +550,6 @@ func (s *Storage) findByIP(addr netip.Addr) (p *Persistent, ok bool) {
 	return nil, false
 }
 
-// Find finds persistent client by string representation of the ClientID, IP
-// address, or MAC.  And returns its shallow copy.
-//
-// TODO(s.chzhen): !! Replace with [Storage.FindParams].
-func (s *Storage) Find(id string) (p *Persistent, ok bool) {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-
-	p, ok = s.index.find(id)
-	if ok {
-		return p.ShallowClone(), ok
-	}
-
-	ip, err := netip.ParseAddr(id)
-	if err != nil {
-		return nil, false
-	}
-
-	foundMAC := s.dhcp.MACByIP(ip)
-	if foundMAC != nil {
-		return s.FindByMAC(foundMAC)
-	}
-
-	return nil, false
-}
-
 // FindLoose is like [Storage.Find] but it also tries to find a persistent
 // client by IP address without zone.  It strips the IPv6 zone index from the
 // stored IP addresses before comparing, because querylog entries don't have it.
@@ -592,6 +557,8 @@ func (s *Storage) Find(id string) (p *Persistent, ok bool) {
 //
 // Note that multiple clients can have the same IP address with different zones.
 // Therefore, the result of this method is indeterminate.
+//
+// TODO(s.chzhen):  Consider accepting [FindParams].
 func (s *Storage) FindLoose(ip netip.Addr, id string) (p *Persistent, ok bool) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
@@ -609,17 +576,6 @@ func (s *Storage) FindLoose(ip netip.Addr, id string) (p *Persistent, ok bool) {
 	return nil, false
 }
 
-// FindByMAC finds persistent client by MAC and returns its shallow copy.  s.mu
-// is expected to be locked.
-func (s *Storage) FindByMAC(mac net.HardwareAddr) (p *Persistent, ok bool) {
-	p, ok = s.index.findByMAC(mac)
-	if ok {
-		return p.ShallowClone(), ok
-	}
-
-	return nil, false
-}
-
 // RemoveByName removes persistent client information.  ok is false if no such
 // client exists by that name.
 func (s *Storage) RemoveByName(ctx context.Context, name string) (ok bool) {

internal/client/storage_internal_test.go

@@ -0,0 +1,28 @@
+package client
+
+import "net"
+
+// FindByName finds persistent client by name.  And returns its shallow copy.
+// It is currently only used in tests.
+func (s *Storage) FindByName(name string) (p *Persistent, ok bool) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	p, ok = s.index.findByName(name)
+	if ok {
+		return p.ShallowClone(), ok
+	}
+
+	return nil, false
+}
+
+// FindByMAC finds persistent client by MAC and returns its shallow copy.  s.mu
+// is expected to be locked.  It is currently only used in tests.
+func (s *Storage) FindByMAC(mac net.HardwareAddr) (p *Persistent, ok bool) {
+	p, ok = s.index.findByMAC(mac)
+	if ok {
+		return p.ShallowClone(), ok
+	}
+
+	return nil, false
+}

internal/client/storage_test.go

@@ -519,7 +519,7 @@ func TestClientsDHCP(t *testing.T) {
 		})
 		require.NoError(t, err)
 
-		prsCli, ok := storage.FindParams(client.ParseFindParams(prsCliIP.String()))
+		prsCli, ok := storage.Find(client.ParseFindParams(prsCliIP.String()))
 		require.True(t, ok)
 
 		assert.Equal(t, prsCliName, prsCli.Name)
@@ -950,7 +950,7 @@ func TestStorage_Find(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			for _, id := range tc.ids {
-				c, ok := s.FindParams(client.ParseFindParams(id))
+				c, ok := s.Find(client.ParseFindParams(id))
 				require.True(t, ok)
 
 				assert.Equal(t, tc.want, c)
@@ -959,7 +959,7 @@ func TestStorage_Find(t *testing.T) {
 	}
 
 	t.Run("not_found", func(t *testing.T) {
-		_, ok := s.FindParams(client.ParseFindParams(cliIPNone))
+		_, ok := s.Find(client.ParseFindParams(cliIPNone))
 		assert.False(t, ok)
 	})
 }

internal/home/clients.go

@@ -364,7 +364,7 @@ func (clients *clientsContainer) shouldCountClient(ids []string) (y bool) {
 	defer clients.lock.Unlock()
 
 	for _, id := range ids {
-		client, ok := clients.storage.FindParams(client.ParseFindParams(id))
+		client, ok := clients.storage.Find(client.ParseFindParams(id))
 		if ok {
 			return !client.IgnoreStatistics
 		}

internal/home/clientshttp.go

@@ -446,8 +446,9 @@ func (clients *clientsContainer) handleFindClient(w http.ResponseWriter, r *http
 // findClient returns available information about a client by idStr from the
 // client's storage or access settings.  cj is guaranteed to be non-nil.
 func (clients *clientsContainer) findClient(idStr string) (cj *clientJSON) {
-	ip, _ := netip.ParseAddr(idStr)
-	c, ok := clients.storage.FindParams(client.ParseFindParams(idStr))
+	params := client.ParseFindParams(idStr)
+	ip := params.RemoteIP
+	c, ok := clients.storage.Find(params)
 	if !ok {
 		return clients.findRuntime(ip, idStr)
 	}