From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 8FD021FF0ED for ; Fri, 14 Aug 2026 14:42:20 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 211F222540; Fri, 14 Aug 2026 14:42:20 +0200 (CEST) From: Robert Obkircher To: pbs-devel@lists.proxmox.com Subject: [PATCH v1 proxmox] router: compare the Fn trait object instead of the ApiHandler address Date: Fri, 14 Aug 2026 14:40:48 +0200 Message-ID: <20260814124206.309661-1-r.obkircher@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1786711317080 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.046 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust RDNS_NONE 1.274 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 2HPORX7KNSSE7KJ6GYDAHXZZSD27CS2X X-Message-ID-Hash: 2HPORX7KNSSE7KJ6GYDAHXZZSD27CS2X X-MailFrom: r.obkircher@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: The previous implementation was effectively `ptr::eq(self, rhs)`, because it compared the addresses of the fields instead of the trait object fat pointers. This worked anyway, because the tests only compare const values that the compiler manages to deduplicate. I discovered this problem because those handler tests fail under miri. However, this change does not fix that, because miri assigns a unique address every time a reference to a function is created, meaning even `ptr::eq(&some_fn, &some_fn)` evaluates to false. Signed-off-by: Robert Obkircher --- proxmox-router/src/router.rs | 56 ++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/proxmox-router/src/router.rs b/proxmox-router/src/router.rs index fea47ba6..a92c527d 100644 --- a/proxmox-router/src/router.rs +++ b/proxmox-router/src/router.rs @@ -514,42 +514,36 @@ impl Eq for ApiHandler {} #[cfg(feature = "test-harness")] impl PartialEq for ApiHandler { fn eq(&self, rhs: &Self) -> bool { - unsafe { - #[allow(clippy::missing_transmute_annotations)] - match (self, rhs) { - (ApiHandler::Sync(l), ApiHandler::Sync(r)) => { - core::mem::transmute::<_, usize>(l) == core::mem::transmute::<_, usize>(r) - } - (ApiHandler::SerializingSync(l), ApiHandler::SerializingSync(r)) => { - core::mem::transmute::<_, usize>(l) == core::mem::transmute::<_, usize>(r) - } - (ApiHandler::StreamSync(l), ApiHandler::StreamSync(r)) => { - core::mem::transmute::<_, usize>(l) == core::mem::transmute::<_, usize>(r) - } - (ApiHandler::Async(l), ApiHandler::Async(r)) => { - core::mem::transmute::<_, usize>(l) == core::mem::transmute::<_, usize>(r) - } - (ApiHandler::SerializingAsync(l), ApiHandler::SerializingAsync(r)) => { - core::mem::transmute::<_, usize>(l) == core::mem::transmute::<_, usize>(r) - } - (ApiHandler::StreamAsync(l), ApiHandler::StreamAsync(r)) => { - core::mem::transmute::<_, usize>(l) == core::mem::transmute::<_, usize>(r) - } - #[cfg(feature = "server")] - (ApiHandler::AsyncHttp(l), ApiHandler::AsyncHttp(r)) => { - core::mem::transmute::<_, usize>(l) == core::mem::transmute::<_, usize>(r) - } - #[cfg(feature = "server")] - ( - ApiHandler::AsyncHttpBodyParameters(l), - ApiHandler::AsyncHttpBodyParameters(r), - ) => core::mem::transmute::<_, usize>(l) == core::mem::transmute::<_, usize>(r), - _ => false, + // This can result in false positives and false negatives (see ptr::fn_addr_eq). + use core::ptr::eq; + match (self, rhs) { + (ApiHandler::Sync(l), ApiHandler::Sync(r)) => eq(*l, *r), + (ApiHandler::SerializingSync(l), ApiHandler::SerializingSync(r)) => eq(*l, *r), + (ApiHandler::StreamSync(l), ApiHandler::StreamSync(r)) => eq(*l, *r), + (ApiHandler::Async(l), ApiHandler::Async(r)) => eq(*l, *r), + (ApiHandler::SerializingAsync(l), ApiHandler::SerializingAsync(r)) => eq(*l, *r), + (ApiHandler::StreamAsync(l), ApiHandler::StreamAsync(r)) => eq(*l, *r), + #[cfg(feature = "server")] + (ApiHandler::AsyncHttp(l), ApiHandler::AsyncHttp(r)) => eq(*l, *r), + #[cfg(feature = "server")] + (ApiHandler::AsyncHttpBodyParameters(l), ApiHandler::AsyncHttpBodyParameters(r)) => { + eq(*l, *r) } + _ => false, } } } +#[test] +#[cfg(feature = "test-harness")] +fn test_handler_eq() { + static DUMMY: ApiHandlerFn = &|_, _, _| unimplemented!(); + static H1: ApiHandler = ApiHandler::Sync(DUMMY); + static H2: ApiHandler = ApiHandler::Sync(DUMMY); + assert!(!core::ptr::eq(&H1, &H2)); // statics have unique addresses + assert!(H1 == H2); // this used to fail because it compared &&dyn instead of &dyn +} + /// Lookup table to child `Router`s /// /// Stores a sorted list of `(name, router)` tuples: -- 2.47.3