diff --git a/cmd/server/nodes_pagination_postfilter_test.go b/cmd/server/nodes_pagination_postfilter_test.go index d172038b..7711d42f 100644 --- a/cmd/server/nodes_pagination_postfilter_test.go +++ b/cmd/server/nodes_pagination_postfilter_test.go @@ -1,9 +1,12 @@ package main import ( + "bytes" "encoding/json" "fmt" + "log" "net/http/httptest" + "strings" "testing" ) @@ -114,3 +117,61 @@ func TestHandleNodes_PostFilterDoesNotTruncatePagination(t *testing.T) { t.Error("hidden-prefix node's pubkey should never appear across any page") } } + +// TestHandleNodes_PostFilterCompensationLoopLogsOnExhaustion covers the +// bounded worst case of the compensation loop added above: if a post-filter +// drops an extreme, sustained run of consecutive rows (more than +// maxIterations*limit), the loop gives up rather than scanning forever — +// but must leave a log breadcrumb so a short response is diagnosable +// instead of silently under-filling the page (bot review MINOR, PR #1852). +func TestHandleNodes_PostFilterCompensationLoopLogsOnExhaustion(t *testing.T) { + srv, router := setupTestServer(t) + if _, err := srv.db.conn.Exec(`DELETE FROM nodes`); err != nil { + t.Fatalf("clear nodes: %v", err) + } + + // With limit=1, maxIterations=50 caps the loop at 50 DB pages. Seed 51 + // consecutive hidden nodes (so every page in that window filters to + // zero) followed by one real node the loop should never reach. + for i := 0; i < 51; i++ { + pk := fmt.Sprintf("pkhidden%056d", i) + lastSeen := fmt.Sprintf("2026-06-01T%02d:00:00Z", 23-(i%24)) + if _, err := srv.db.conn.Exec(`INSERT INTO nodes + (public_key, name, role, lat, lon, last_seen, first_seen, advert_count) + VALUES (?, '🚫 Hidden', 'repeater', 55.0, 10.0, ?, '2026-05-01T00:00:00Z', 1)`, + pk, lastSeen); err != nil { + t.Fatalf("insert hidden %d: %v", i, err) + } + } + if _, err := srv.db.conn.Exec(`INSERT INTO nodes + (public_key, name, role, lat, lon, last_seen, first_seen, advert_count) + VALUES ('pkreal0000000000000000000000000000000000000000000000000001', 'RealNode', 'repeater', 55.0, 10.0, '2026-01-01T00:00:00Z', '2026-01-01T00:00:00Z', 1)`, + ); err != nil { + t.Fatalf("insert real node: %v", err) + } + srv.cfg.SetHiddenNamePrefixes([]string{"🚫"}) + + var buf bytes.Buffer + prev := log.Writer() + log.SetOutput(&buf) + defer log.SetOutput(prev) + + req := httptest.NewRequest("GET", "/api/nodes?limit=1&offset=0", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != 200 { + t.Fatalf("status=%d body=%s", w.Code, w.Body.String()) + } + var resp struct { + Nodes []map[string]interface{} `json:"nodes"` + } + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v body=%s", err, w.Body.String()) + } + if len(resp.Nodes) != 0 { + t.Errorf("expected 0 nodes (RealNode is beyond the 50-iteration cap), got %d: %+v", len(resp.Nodes), resp.Nodes) + } + if !strings.Contains(buf.String(), "maxIterations") { + t.Errorf("expected a log breadcrumb mentioning maxIterations when the compensation loop is exhausted, got log output: %q", buf.String()) + } +} diff --git a/cmd/server/routes.go b/cmd/server/routes.go index 1264aa04..93fa1127 100644 --- a/cmd/server/routes.go +++ b/cmd/server/routes.go @@ -1471,6 +1471,7 @@ func (s *Server) handleNodes(w http.ResponseWriter, r *http.Request) { // (a raw page shorter than requested = genuine end of data). const maxIterations = 50 // bounds worst case to 50*requestedLimit DB rows scanned curOffset := requestedOffset + exhaustedIterations := true for iter := 0; iter < maxIterations; iter++ { page, _, pageCounts, err := s.db.GetNodes(requestedLimit, curOffset, role, search, before, lastHeard, sortBy, region) if err != nil { @@ -1484,12 +1485,29 @@ func (s *Server) handleNodes(w http.ResponseWriter, r *http.Request) { nodes = append(nodes, filters.apply(page)...) curOffset += requestedLimit if dbPageLen < requestedLimit || len(nodes) >= requestedLimit { + exhaustedIterations = false break } } + if exhaustedIterations { + // Only reachable if a post-filter is dropping an extreme, + // sustained run of consecutive rows (>50*requestedLimit) — + // astronomically unlikely for blacklist/hidden-prefix in + // practice, but log it so a truncated response is diagnosable + // instead of silently under-filling the page. + log.Printf("WARN handleNodes: pagination compensation loop hit maxIterations=%d (offset=%d, limit=%d) — returning %d rows, possibly short", + maxIterations, requestedOffset, requestedLimit, len(nodes)) + } if len(nodes) > requestedLimit { nodes = nodes[:requestedLimit] } + // total reflects only this returned page-slice under any active + // post-filter, not a true all-pages count — matches the + // pre-existing filtered-path semantics (each filter block used to + // set total = len(filtered) the same way). fetchAllNodes + // (public/app.js) never reads `total` for its own pagination + // termination — it relies on page-length — so this is display-only + // and safe to leave approximate. total = len(nodes) }