Skip to content

Commit b1f2c4c

Browse files
schzhnschzhn
committed
home: imp code
1 parent d0a5abd commit b1f2c4c

File tree

2 files changed

+59
-66
lines changed

2 files changed

+59
-66
lines changed

‎internal/home/clients.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ type clientsContainer struct {
2828
// filter. It must not be nil.
2929
baseLogger *slog.Logger
3030

31+
// logger is used for logging the operation of the client container. It
32+
// must not be nil.
33+
logger *slog.Logger
34+
3135
// storage stores information about persistent clients.
3236
storage *client.Storage
3337

@@ -81,6 +85,7 @@ func (clients *clientsContainer) Init(
8185
}
8286

8387
clients.baseLogger = baseLogger
88+
clients.logger = baseLogger.With(slogutil.KeyPrefix, "client_container")
8489
clients.safeSearchCacheSize = filteringConf.SafeSearchCacheSize
8590
clients.safeSearchCacheTTL = time.Minute * time.Duration(filteringConf.CacheTime)
8691

@@ -372,11 +377,7 @@ func (clients *clientsContainer) shouldCountClient(ids []string) (y bool) {
372377
err := params.Set(id)
373378
if err != nil {
374379
// Should not happen.
375-
clients.baseLogger.Warn(
376-
"parsing find params",
377-
slogutil.KeyPrefix, "client_container",
378-
slogutil.KeyError, err,
379-
)
380+
clients.logger.Warn("parsing find params", slogutil.KeyError, err)
380381

381382
continue
382383
}

‎internal/home/clientshttp.go

+53-61
Original file line numberDiff line numberDiff line change
@@ -64,22 +64,6 @@ type clientJSON struct {
6464
UpstreamsCacheEnabled aghalg.NullBool `json:"upstreams_cache_enabled"`
6565
}
6666

67-
// clientJSONWithBlockedInfo returns an initialized [*clientJSON] with the
68-
// access settings filled. cj is always non-nil.
69-
func (clients *clientsContainer) clientJSONWithBlockedInfo(
70-
addr netip.Addr,
71-
idStr string,
72-
) (cj *clientJSON) {
73-
disallowed, rule := clients.clientChecker.IsBlockedClient(addr, idStr)
74-
75-
return &clientJSON{
76-
IDs: []string{idStr},
77-
Disallowed: &disallowed,
78-
DisallowedRule: &rule,
79-
WHOIS: &whois.Info{},
80-
}
81-
}
82-
8367
// runtimeClientJSON is a JSON representation of the [client.Runtime].
8468
type runtimeClientJSON struct {
8569
WHOIS *whois.Info `json:"whois_info"`
@@ -444,7 +428,7 @@ func (clients *clientsContainer) handleUpdateClient(w http.ResponseWriter, r *ht
444428
// Deprecated: Remove it when migration to the new API is over.
445429
func (clients *clientsContainer) handleFindClient(w http.ResponseWriter, r *http.Request) {
446430
q := r.URL.Query()
447-
data := []map[string]*clientJSON{}
431+
data := make([]map[string]*clientJSON, 0, len(q))
448432
params := &client.FindParams{}
449433
var err error
450434

@@ -454,34 +438,39 @@ func (clients *clientsContainer) handleFindClient(w http.ResponseWriter, r *http
454438
break
455439
}
456440

457-
var cli *clientJSON
458-
459441
err = params.Set(idStr)
460442
if err != nil {
461-
cli = clients.clientJSONWithBlockedInfo(netip.Addr{}, idStr)
462-
} else {
463-
cli = clients.findClient(params)
443+
clients.logger.DebugContext(
444+
r.Context(),
445+
"finding client",
446+
"id", idStr,
447+
slogutil.KeyError, err,
448+
)
449+
450+
continue
464451
}
465452

466453
data = append(data, map[string]*clientJSON{
467-
idStr: cli,
454+
idStr: clients.findClient(idStr, params),
468455
})
469456
}
470457

471458
aghhttp.WriteJSONResponseOK(w, r, data)
472459
}
473460

474461
// findClient returns available information about a client by params from the
475-
// client's storage or access settings. cj is guaranteed to be non-nil.
476-
func (clients *clientsContainer) findClient(params *client.FindParams) (cj *clientJSON) {
462+
// client's storage or access settings. idStr is the string representation of
463+
// typed params. cj is guaranteed to be non-nil.
464+
//
465+
// TODO(s.chzhen): Remove idStr once [BlockedClientChecker] starts supporting
466+
// [client.FindParams].
467+
func (clients *clientsContainer) findClient(
468+
idStr string,
469+
params *client.FindParams,
470+
) (cj *clientJSON) {
477471
c, ok := clients.storage.Find(params)
478472
if !ok {
479-
return clients.findRuntime(params)
480-
}
481-
482-
idStr := string(params.ClientID)
483-
if idStr == "" {
484-
idStr = params.RemoteIP.String()
473+
return clients.findRuntime(idStr, params)
485474
}
486475

487476
cj = clientToJSON(c)
@@ -515,23 +504,25 @@ func (clients *clientsContainer) handleSearchClient(w http.ResponseWriter, r *ht
515504
return
516505
}
517506

518-
data := []map[string]*clientJSON{}
507+
data := make([]map[string]*clientJSON, 0, len(q.Clients))
519508
params := &client.FindParams{}
520509

521510
for _, c := range q.Clients {
522511
idStr := c.ID
523-
524-
var cli *clientJSON
525-
526512
err = params.Set(idStr)
527513
if err != nil {
528-
cli = clients.clientJSONWithBlockedInfo(netip.Addr{}, idStr)
529-
} else {
530-
cli = clients.findClient(params)
514+
clients.logger.DebugContext(
515+
r.Context(),
516+
"searching client",
517+
"id", idStr,
518+
slogutil.KeyError, err,
519+
)
520+
521+
continue
531522
}
532523

533524
data = append(data, map[string]*clientJSON{
534-
idStr: cli,
525+
idStr: clients.findClient(idStr, params),
535526
})
536527
}
537528

@@ -541,34 +532,35 @@ func (clients *clientsContainer) handleSearchClient(w http.ResponseWriter, r *ht
541532
// findRuntime looks up the IP in runtime and temporary storages, like
542533
// /etc/hosts tables, DHCP leases, or blocklists. cj is guaranteed to be
543534
// non-nil.
544-
func (clients *clientsContainer) findRuntime(params *client.FindParams) (cj *clientJSON) {
545-
ip := params.RemoteIP
546-
idStr := string(params.ClientID)
547-
if idStr == "" {
548-
idStr = ip.String()
549-
}
535+
func (clients *clientsContainer) findRuntime(
536+
idStr string,
537+
params *client.FindParams,
538+
) (cj *clientJSON) {
539+
var host string
540+
whois := &whois.Info{}
550541

542+
ip := params.RemoteIP
551543
rc := clients.storage.ClientRuntime(ip)
552-
if rc == nil {
553-
// It is still possible that the IP used to be in the runtime clients
554-
// list, but then the server was reloaded. So, check the DNS server's
555-
// blocked IP list.
556-
//
557-
// See https://github.com/AdguardTeam/AdGuardHome/issues/2428.
558-
return clients.clientJSONWithBlockedInfo(ip, idStr)
559-
}
560-
561-
_, host := rc.Info()
562-
cj = &clientJSON{
563-
Name: host,
564-
IDs: []string{idStr},
565-
WHOIS: whoisOrEmpty(rc),
544+
if rc != nil {
545+
_, host = rc.Info()
546+
whois = whoisOrEmpty(rc)
566547
}
567548

549+
// Check the DNS server's blocked IP list regardless of whether a runtime
550+
// client was found or not. This is because it's still possible that the
551+
// runtime client associated with the IP address was stored previously, but
552+
// then the server was reloaded.
553+
//
554+
// See https://github.com/AdguardTeam/AdGuardHome/issues/2428.
568555
disallowed, rule := clients.clientChecker.IsBlockedClient(ip, idStr)
569-
cj.Disallowed, cj.DisallowedRule = &disallowed, &rule
570556

571-
return cj
557+
return &clientJSON{
558+
Name: host,
559+
IDs: []string{idStr},
560+
WHOIS: whois,
561+
Disallowed: &disallowed,
562+
DisallowedRule: &rule,
563+
}
572564
}
573565

574566
// RegisterClientsHandlers registers HTTP handlers

0 commit comments

Comments
 (0)