From e715fbb3629c40eac0fe371b15405ad114ffa19c Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Thu, 12 Nov 2026 06:21:21 -0800 Subject: [PATCH] fix(relay): treat empty filter sets as match-nothing in SQL (#9342) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix for a COUNT regression introduced by #9339 (now on main), found in its review. With #9339, a filter with a nonempty `#a` takes the exact SQL COUNT path. But `filter_to_query_params` dropped an empty `authors`, `ids` or `#e` list and never represented an empty `#p`, `#d` or unpushed tag set. So `{"kinds":[7], "#a":[A], "authors":[]}` counted every reaction to `A`, while REQ (which post-filters) correctly returned nothing. Before #9339 that COUNT returned 0. The same gap already existed for filters without `#a`. Any empty `authors`, `ids` or tag set now maps to the existing `kinds: []` match-nothing sentinel, which every read (REQ, COUNT, `/query`, `/count`) answers without a query. An empty string value such as `"#d": [""]` is still a real constraint. Why nothing gets worse: only filters that provably match nothing change, and they now skip the database instead of reading or counting. ### Validation Added tests: - Unit table over empty `authors`, `ids`, `#e`, `#p`, `#d`, `#h`, `#k` and `#t`, each with and without `#a`, plus `"#d": [""]`. It fails with the sentinel disabled. - Postgres table: WebSocket and HTTP COUNT are 0, and REQ and `/query` are empty, for each empty set beside a matching `#a` and for `{kinds:[7], authors:[]}`. The first commit contains only the tests. Existing workspace unit tests pass (pre-push). The Postgres lanes run separately on an isolated cluster. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Tom Brow Co-authored-by: Tom Brow Co-authored-by: Claude Opus 5.5 --- Justfile | 2 +- crates/buzz-relay/src/handlers/req.rs | 35 ++++++++ .../handlers/tag_pushdown_postgres_tests.rs | 85 +++++++++++++++++++ 3 files changed, 121 insertions(+), 1 deletion(-) diff --git a/Justfile b/Justfile index 033decbbae..60828f79cf 100644 --- a/Justfile +++ b/Justfile @@ -563,7 +563,7 @@ test-unit: # cross-node fan-out ordering and drops; its hung-delivery case fakes a # database that never answers on loopback, and none use DATABASE_URL. cargo nextest run -p buzz-relay --lib --bin buzz-relay \ - -E 'test(/^api::media_access::tests::/) + test(/^handlers::media_access::tests::/) + test(/^subscription::tests::/) + test(/^handlers::multinode_fanout::tests::/) + test(/^api::admin::/) + test(/^access::tests::/) + test(/^api::git::transport::access_log_tests::/) + test(=api::bridge::postgres_tests::nip98_replay_check_logs_only_its_refusal) + test(=api::bridge::postgres_tests::warn_only_artifact_read_lines_keep_the_filter_summary) + test(=connection::tests::close_totals_count_frames_the_writer_flushed) + test(=connection::tests::dispatcher_counts_each_req_and_count_frame_once) + test(=handlers::req::tests::unqueued_eose_logs_aborted_not_served) + test(=api::bridge::postgres_tests::reject_code_keeps_only_a_known_prefix) + test(=api::bridge::postgres_tests::nip98_replay_check_fails_closed_when_guard_errors) + test(=api::bridge::postgres_tests::verify_bridge_auth_rejects_nip98_event_signed_for_wrong_communitys_host) + test(=api::bridge::postgres_tests::verify_bridge_auth_can_require_payload_tag_for_json_body_endpoints) + test(=api::bridge::postgres_tests::verify_bridge_auth_accepts_nip98_event_signed_for_matching_host) + test(=api::bridge::postgres_tests::moderation_read_query_bearing_nip98_event_verifies_with_matching_query) + test(=api::bridge::postgres_tests::moderation_read_query_bearing_nip98_event_rejected_against_bare_path) + test(=api::bridge::postgres_tests::moderation_read_audit_query_bearing_nip98_event_verifies) + test(=api::bridge::postgres_tests::moderation_read_restricted_no_query_still_verifies) + test(=connection::tests::aborting_lifecycle_owner_is_drop_safe) + test(=connection::tests::shutdown_terminalizes_auth_stalled_on_database_before_late_success) + test(=connection::terminal_close_delivery_tests::upgrade_would_deny_line_without_auth_logs_absent_at_close) + test(=handlers::event::tests::observer_owner_cache_is_scoped_to_community) + test(=state::tests::shared_counter_between_direct_and_fanout) + test(=connection::tests::startup_cancellation_writes_one_close_line) + test(=connection::tests::connection_limit_refusal_writes_one_close_line) + test(=connection::tests::dropped_authenticated_handler_writes_one_close_line) + test(=connection::tests::missed_pongs_close_with_their_reason_and_count) + test(=connection::tests::client_reply_to_a_jittered_restart_closes_as_shutdown) + test(=connection::tests::client_close_racing_a_policy_disconnect_keeps_the_policy_reason) + test(=connection::tests::server_close_reason_ranks_shutdown_policy_then_cancellation) + test(=connection::tests::auth_timeout_after_a_rejected_auth_closes_as_auth_failed) + test(=connection::tests::close_totals_count_a_batch_flushed_by_the_drain_after_cancellation) + test(=connection::tests::close_totals_skip_a_frame_the_socket_rejects) + test(=connection::tests::close_totals_skip_frames_stranded_by_a_failed_flush) + test(=connection::tests::read_before_login_logs_one_denial_from_the_handler) + test(=connection::tests::read_refused_by_a_full_handler_semaphore_logs_one_denial) + test(=connection::tests::saturated_handler_write_line_records_whether_its_ok_was_queued) + test(=connection::tests::read_refused_by_unavailable_admission_logs_one_denial) + test(=handlers::auth::tests::repeat_auth_after_a_terminal_logs_one_denial) + test(=nip_fi_upgrade::tests::deny_protected_logs_one_denied_auth_line) + test(=api::bridge::moderation_refusal_tests::moderation_lookup_failure_is_unavailable) + test(/^push_runtime::/) + test(/^operator_listener::tests::/) + test(/^handlers::channel_authz::/) + test(/^handlers::moderation_authz::/) + test(/^handlers::side_effects::tests::/) + test(/^handlers::coalesced_refresh::tests::/) + test(/^handlers::system_channels::tests::/) + test(/^nip11::tests::/) + test(/^storage_sweep::tests::/) + test(/^nip_fi_core::tests::/) + test(/^nip_fi_http::tests::/) + test(/^nip_fi_policy::tests::/) + test(/^nip_fi_config::tests::/) + test(/^nip_fi_resolver::tests::/) + test(/^readiness::tests::/) + test(/^router::tests::/) + test(/^api::parse_query_tests::/) + test(/^api::db_read_error_tests::/) + test(/^api::profiles::/) + test(/^api::bridge::search_slots::tests::/) + test(/^api::git::transport::off_mode_precedence_tests::/) + test(/^api::git::store::tests::/) + test(/^api::git::store::fault_probe::/) + test(/^audio::join::tests::/) + test(/^audio::handler::tests::/) + test(/^nip_fi_gate::tests::/) + test(/^nip_fi_session::tests::/) + test(/^nip_fi_shadow::tests::/) + test(/^nip_fi_shadow_session::tests::/) + test(/^startup_steps::tests::/) + test(/^mesh_boot::tests::/) + test(/^telemetry::tests::/) + test(/^config::tests::/) + test(=state::tests::accessible_channel_invalidation_is_scoped_to_community) + test(=state::tests::accessible_channels_join_member_and_open_halves) + test(=state::tests::channel_deleted_invalidation_is_scoped_to_community) + test(=state::tests::neither_a_confirmed_inactive_community_nor_a_failed_lookup_admits_the_socket) + test(=state::tests::periodic_revalidation_disconnects_inside_the_fenced_callback) + test(=state::tests::bare_community_disconnect_fails_closed_when_the_fence_is_unavailable) + test(=connection::tests::send_loop_sends_policy_close_when_community_is_archived) + test(=state::tests::community_disconnect_then_nip_fi_keeps_community_deleted_reason) + test(=state::tests::disconnect_community_wins_reason_losing_nip_fi_does_not_enqueue_frame) + test(=handlers::req::tests::timed_out_historical_read_deregisters_before_closed) + test(=handlers::req::tests::superseded_timeout_leaves_replacement_intact) + test(=handlers::req::tests::search_claim_retires_live_and_yields_to_replacement) + test(=handlers::req::tests::failed_search_closes_without_eose_or_trying_later_filters) + test(=handlers::req::tests::concurrent_claims_and_stale_teardowns_keep_the_last_owner) + test(=handlers::req::tests::timeout_closed_is_emitted_before_a_replacement_can_claim) + test(=handlers::req::tests::revoke_then_replacement_keeps_replacement_whole) + test(=handlers::req::tests::revoke_of_a_replaced_holder_spares_the_pending_replacement) + test(=handlers::req::tests::superseded_or_closed_requests_cannot_claim) + test(=connection::tests::close_fences_a_req_whose_handler_has_not_run) + test(=connection::tests::only_the_newest_same_id_req_answers) + test(=handlers::req::tests::claims_after_connection_cleanup_are_refused) + test(=handlers::req::tests::dropped_terminal_frame_cancels_connection) + test(=handlers::req::tests::revoke_dropped_terminal_frame_cancels_connection) + test(=handlers::req::tests::count_fallback_covers_filters_that_can_match_private_deletions) + test(=handlers::req::tests::count_fallback_fetches_one_extra_candidate) + test(=handlers::req::tests::group_state_t_and_creator_tags_reach_sql_only_for_group_state_kinds) + test(=api::bridge::postgres_tests::bridge_search_mode_extension_accepts_prefix_snake_or_camel_case) + test(=api::bridge::postgres_tests::bridge_search_sort_extension_selects_newest_only_when_named) + test(=api::invites::postgres_tests::restriction_tests::postgres_tests::destructive_fixture_rejects_shared_database_urls) + (kind(bin) & (test(/^tests::/) + test(/^composition_tests::/) + test(/^env_filter_tests::/)) - test(/^tests::postgres_tests::/))' + -E 'test(/^api::media_access::tests::/) + test(/^handlers::media_access::tests::/) + test(/^subscription::tests::/) + test(/^handlers::multinode_fanout::tests::/) + test(/^api::admin::/) + test(/^access::tests::/) + test(/^api::git::transport::access_log_tests::/) + test(=api::bridge::postgres_tests::nip98_replay_check_logs_only_its_refusal) + test(=api::bridge::postgres_tests::warn_only_artifact_read_lines_keep_the_filter_summary) + test(=connection::tests::close_totals_count_frames_the_writer_flushed) + test(=connection::tests::dispatcher_counts_each_req_and_count_frame_once) + test(=handlers::req::tests::unqueued_eose_logs_aborted_not_served) + test(=api::bridge::postgres_tests::reject_code_keeps_only_a_known_prefix) + test(=api::bridge::postgres_tests::nip98_replay_check_fails_closed_when_guard_errors) + test(=api::bridge::postgres_tests::verify_bridge_auth_rejects_nip98_event_signed_for_wrong_communitys_host) + test(=api::bridge::postgres_tests::verify_bridge_auth_can_require_payload_tag_for_json_body_endpoints) + test(=api::bridge::postgres_tests::verify_bridge_auth_accepts_nip98_event_signed_for_matching_host) + test(=api::bridge::postgres_tests::moderation_read_query_bearing_nip98_event_verifies_with_matching_query) + test(=api::bridge::postgres_tests::moderation_read_query_bearing_nip98_event_rejected_against_bare_path) + test(=api::bridge::postgres_tests::moderation_read_audit_query_bearing_nip98_event_verifies) + test(=api::bridge::postgres_tests::moderation_read_restricted_no_query_still_verifies) + test(=connection::tests::aborting_lifecycle_owner_is_drop_safe) + test(=connection::tests::shutdown_terminalizes_auth_stalled_on_database_before_late_success) + test(=connection::terminal_close_delivery_tests::upgrade_would_deny_line_without_auth_logs_absent_at_close) + test(=handlers::event::tests::observer_owner_cache_is_scoped_to_community) + test(=state::tests::shared_counter_between_direct_and_fanout) + test(=connection::tests::startup_cancellation_writes_one_close_line) + test(=connection::tests::connection_limit_refusal_writes_one_close_line) + test(=connection::tests::dropped_authenticated_handler_writes_one_close_line) + test(=connection::tests::missed_pongs_close_with_their_reason_and_count) + test(=connection::tests::client_reply_to_a_jittered_restart_closes_as_shutdown) + test(=connection::tests::client_close_racing_a_policy_disconnect_keeps_the_policy_reason) + test(=connection::tests::server_close_reason_ranks_shutdown_policy_then_cancellation) + test(=connection::tests::auth_timeout_after_a_rejected_auth_closes_as_auth_failed) + test(=connection::tests::close_totals_count_a_batch_flushed_by_the_drain_after_cancellation) + test(=connection::tests::close_totals_skip_a_frame_the_socket_rejects) + test(=connection::tests::close_totals_skip_frames_stranded_by_a_failed_flush) + test(=connection::tests::read_before_login_logs_one_denial_from_the_handler) + test(=connection::tests::read_refused_by_a_full_handler_semaphore_logs_one_denial) + test(=connection::tests::saturated_handler_write_line_records_whether_its_ok_was_queued) + test(=connection::tests::read_refused_by_unavailable_admission_logs_one_denial) + test(=handlers::auth::tests::repeat_auth_after_a_terminal_logs_one_denial) + test(=nip_fi_upgrade::tests::deny_protected_logs_one_denied_auth_line) + test(=api::bridge::moderation_refusal_tests::moderation_lookup_failure_is_unavailable) + test(/^push_runtime::/) + test(/^operator_listener::tests::/) + test(/^handlers::channel_authz::/) + test(/^handlers::moderation_authz::/) + test(/^handlers::side_effects::tests::/) + test(/^handlers::coalesced_refresh::tests::/) + test(/^handlers::system_channels::tests::/) + test(/^nip11::tests::/) + test(/^storage_sweep::tests::/) + test(/^nip_fi_core::tests::/) + test(/^nip_fi_http::tests::/) + test(/^nip_fi_policy::tests::/) + test(/^nip_fi_config::tests::/) + test(/^nip_fi_resolver::tests::/) + test(/^readiness::tests::/) + test(/^router::tests::/) + test(/^api::parse_query_tests::/) + test(/^api::db_read_error_tests::/) + test(/^api::profiles::/) + test(/^api::bridge::search_slots::tests::/) + test(/^api::git::transport::off_mode_precedence_tests::/) + test(/^api::git::store::tests::/) + test(/^api::git::store::fault_probe::/) + test(/^audio::join::tests::/) + test(/^audio::handler::tests::/) + test(/^nip_fi_gate::tests::/) + test(/^nip_fi_session::tests::/) + test(/^nip_fi_shadow::tests::/) + test(/^nip_fi_shadow_session::tests::/) + test(/^startup_steps::tests::/) + test(/^mesh_boot::tests::/) + test(/^telemetry::tests::/) + test(/^config::tests::/) + test(=state::tests::accessible_channel_invalidation_is_scoped_to_community) + test(=state::tests::accessible_channels_join_member_and_open_halves) + test(=state::tests::channel_deleted_invalidation_is_scoped_to_community) + test(=state::tests::neither_a_confirmed_inactive_community_nor_a_failed_lookup_admits_the_socket) + test(=state::tests::periodic_revalidation_disconnects_inside_the_fenced_callback) + test(=state::tests::bare_community_disconnect_fails_closed_when_the_fence_is_unavailable) + test(=connection::tests::send_loop_sends_policy_close_when_community_is_archived) + test(=state::tests::community_disconnect_then_nip_fi_keeps_community_deleted_reason) + test(=state::tests::disconnect_community_wins_reason_losing_nip_fi_does_not_enqueue_frame) + test(=handlers::req::tests::timed_out_historical_read_deregisters_before_closed) + test(=handlers::req::tests::superseded_timeout_leaves_replacement_intact) + test(=handlers::req::tests::search_claim_retires_live_and_yields_to_replacement) + test(=handlers::req::tests::failed_search_closes_without_eose_or_trying_later_filters) + test(=handlers::req::tests::concurrent_claims_and_stale_teardowns_keep_the_last_owner) + test(=handlers::req::tests::timeout_closed_is_emitted_before_a_replacement_can_claim) + test(=handlers::req::tests::revoke_then_replacement_keeps_replacement_whole) + test(=handlers::req::tests::revoke_of_a_replaced_holder_spares_the_pending_replacement) + test(=handlers::req::tests::superseded_or_closed_requests_cannot_claim) + test(=connection::tests::close_fences_a_req_whose_handler_has_not_run) + test(=connection::tests::only_the_newest_same_id_req_answers) + test(=handlers::req::tests::claims_after_connection_cleanup_are_refused) + test(=handlers::req::tests::dropped_terminal_frame_cancels_connection) + test(=handlers::req::tests::revoke_dropped_terminal_frame_cancels_connection) + test(=handlers::req::tests::count_fallback_covers_filters_that_can_match_private_deletions) + test(=handlers::req::tests::count_fallback_fetches_one_extra_candidate) + test(=handlers::req::tests::group_state_t_and_creator_tags_reach_sql_only_for_group_state_kinds) + test(=handlers::req::tests::empty_filter_sets_match_nothing_in_sql) + test(=api::bridge::postgres_tests::bridge_search_mode_extension_accepts_prefix_snake_or_camel_case) + test(=api::bridge::postgres_tests::bridge_search_sort_extension_selects_newest_only_when_named) + test(=api::invites::postgres_tests::restriction_tests::postgres_tests::destructive_fixture_rejects_shared_database_urls) + (kind(bin) & (test(/^tests::/) + test(/^composition_tests::/) + test(/^env_filter_tests::/)) - test(/^tests::postgres_tests::/))' # Note on audio::join::tests scope: the full suite is infra-free (no DB, # no Redis). The infra-free audio/FI regression witnesses — bootstrap # ordering barrier, CommitConfirmed arm, pending-close invisibility, diff --git a/crates/buzz-relay/src/handlers/req.rs b/crates/buzz-relay/src/handlers/req.rs index 002ba34d95..cd19e86dcb 100644 --- a/crates/buzz-relay/src/handlers/req.rs +++ b/crates/buzz-relay/src/handlers/req.rs @@ -1342,6 +1342,13 @@ fn filter_to_query_params( ks.iter().map(|k| k.as_u16() as i32).collect() } }); + // An empty `authors`, `ids` or tag set also matches nothing (NIP-01), but + // the conversions below drop or ignore it, so COUNT would count without + // it. Use the same sentinel, so every read answers it without a query. + let matches_nothing = filter.authors.as_ref().is_some_and(|a| a.is_empty()) + || filter.ids.as_ref().is_some_and(|i| i.is_empty()) + || filter.generic_tags.values().any(|values| values.is_empty()); + let kinds = if matches_nothing { Some(vec![]) } else { kinds }; let since = filter .since @@ -3482,6 +3489,34 @@ mod tests { ); } + #[test] + fn empty_filter_sets_match_nothing_in_sql() { + let community = buzz_core::tenant::CommunityId::from_uuid(uuid::Uuid::nil()); + let addr = format!("30175:{}:persona", "ab".repeat(32)); + for key in ["authors", "ids", "#e", "#p", "#d", "#h", "#k", "#t"] { + for with_a in [false, true] { + let mut json = serde_json::json!({"kinds": [7], key: []}); + if with_a { + json["#a"] = serde_json::json!([addr]); + } + let filter: Filter = serde_json::from_value(json).unwrap(); + assert_eq!( + filter_to_query_params(&filter, None, community).kinds, + Some(vec![]), + "empty {key} (with #a: {with_a}) must match nothing" + ); + } + } + + let empty_d_value: Filter = + serde_json::from_value(serde_json::json!({"kinds": [30175], "#d": [""]})).unwrap(); + assert_eq!( + filter_to_query_params(&empty_d_value, None, community).kinds, + Some(vec![30175]), + "an empty string is a real #d value, not an empty set" + ); + } + #[test] fn test_extract_channel_id_single_channel() { let channel_id = uuid::Uuid::new_v4(); diff --git a/crates/buzz-relay/src/handlers/tag_pushdown_postgres_tests.rs b/crates/buzz-relay/src/handlers/tag_pushdown_postgres_tests.rs index 5a8a8b48f3..d919c7e971 100644 --- a/crates/buzz-relay/src/handlers/tag_pushdown_postgres_tests.rs +++ b/crates/buzz-relay/src/handlers/tag_pushdown_postgres_tests.rs @@ -158,3 +158,88 @@ async fn address_reaction_reqs_over_several_channels() { ); assert_eq!(count(&state, &tenant, &member, filter).await, 1); } + +/// POST one filter to the HTTP bridge as `reader` and return the JSON reply. +async fn bridge( + state: &Arc, + tenant: &TenantContext, + reader: &Keys, + path: &str, + filter: &serde_json::Value, +) -> serde_json::Value { + use axum::body::Body; + use axum::http::{header, Request}; + use tower::ServiceExt; + + let request = Request::post(path) + .header(header::HOST, tenant.host()) + .header("x-pubkey", reader.public_key().to_hex()) + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from(serde_json::to_vec(&[filter]).unwrap())) + .unwrap(); + let response = crate::router::build_router(state.clone()) + .oneshot(request) + .await + .unwrap(); + let status = response.status(); + let body = axum::body::to_bytes(response.into_body(), 1 << 20) + .await + .unwrap(); + assert!(status.is_success(), "{path}: {status}"); + serde_json::from_slice(&body).expect("bridge JSON") +} + +/// An empty `authors`, `ids` or tag set matches nothing, so COUNT is 0 and +/// REQ is empty on WebSocket and HTTP, even beside a matching `#a`. +/// Mutation: drop the empty-set sentinel from `filter_to_query_params` → RED +/// (each COUNT counts the `#a` match, or every kind:7 without `#a`). +#[tokio::test] +#[ignore = "requires Postgres"] +async fn empty_sets_beside_an_address_match_nothing() { + let Some(state) = crate::api::bridge::postgres_tests::bridge_handler_test_state().await else { + panic!("requires local Postgres and Redis"); + }; + let tenant = community(&state, "a-tag-empty-sets").await; + let reactor = Keys::generate(); + let reader = Keys::generate(); + let needle = format!("30175:{}:needle", reader.public_key().to_hex()); + let event = EventBuilder::new(Kind::Reaction, "+") + .tags([ + Tag::parse(["a", &needle]).unwrap(), + Tag::parse(["e", &"cd".repeat(32)]).unwrap(), + Tag::parse(["p", &reader.public_key().to_hex()]).unwrap(), + Tag::parse(["k", "30175"]).unwrap(), + ]) + .sign_with_keys(&reactor) + .unwrap(); + state + .db + .insert_event(tenant.community(), &event, None) + .await + .expect("insert reaction"); + + let base = serde_json::json!({"kinds": [7], "#a": [needle]}); + assert_eq!(count(&state, &tenant, &reader, base.clone()).await, 1); + let mut filters: Vec<_> = ["authors", "ids", "#e", "#p", "#d", "#k", "#t", "#A"] + .into_iter() + .map(|key| { + let mut filter = base.clone(); + filter[key] = serde_json::json!([]); + filter + }) + .collect(); + filters.push(serde_json::json!({"kinds": [7], "authors": []})); + + for filter in filters { + let ws_count = count(&state, &tenant, &reader, filter.clone()).await; + let http_count = bridge(&state, &tenant, &reader, "/count", &filter).await["count"].clone(); + let ws_req = req_event_ids(&state, &tenant, &reader, filter.clone()).await; + let http_req = bridge(&state, &tenant, &reader, "/query", &filter).await; + assert_eq!( + (ws_count, http_count, ws_req.len()), + (0, serde_json::json!(0), 0), + "{filter}" + ); + assert_eq!(http_req, serde_json::json!([]), "{filter} /query"); + } +}