From 301db87d1acc9caea0493fa74bd396b298092f27 Mon Sep 17 00:00:00 2001 From: Maximilian Winter <33990152+blaueeiner@users.noreply.github.com> Date: Wed, 1 Jul 2026 17:59:22 +0200 Subject: [PATCH] fix(receive): skip PDO placeholder-resend for view-once, ack instead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A message that arrives with only an `` child (no ``) currently triggers a PDO placeholder-resend to our own phone. But WhatsApp never fans view-once media out to companion/linked devices, so that request always comes back empty — the content is unrecoverable by design. Worse, the peer round-trip is surfaced by the phone as a spurious "Finished syncing with WhatsApp on " notification, once for every view-once message the companion receives. Skip the PDO for `UnavailableType::ViewOnce`: still dispatch the `UndecryptableMessage` event (consumers keep seeing the failure) and still send the transport ack directly (previously the ack was gated on the PDO going out), so the offline queue drains and the stanza isn't redelivered. `Unknown` unavailables keep the existing PDO recovery path unchanged. Adds `view_once_stub_acks_without_pdo` covering the new behavior via the capturing-transport harness (event fires + `` sent). --- src/message/receive.rs | 42 +++++++++++++++++++++++++++++++++-------- src/message/tests.rs | 43 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 75 insertions(+), 10 deletions(-) diff --git a/src/message/receive.rs b/src/message/receive.rs index aae0d2474..47fd669d6 100644 --- a/src/message/receive.rs +++ b/src/message/receive.rs @@ -93,11 +93,31 @@ impl Client { Some("view_once") => crate::types::events::UnavailableType::ViewOnce, _ => crate::types::events::UnavailableType::Unknown, }; - log::info!( - "[msg:{}] Message has child (type: {:?}), requesting from phone via PDO", - info.id, - unavailable_type + // View-once media is never fanned out to companion/linked devices, + // so a PDO placeholder-resend to our own phone always comes back + // empty — the content is unrecoverable by design. Worse, that peer + // round-trip is surfaced by the phone as a spurious "Finished + // syncing with WhatsApp on " notification for every + // view-once message received. Skip the PDO for view-once; still + // dispatch the event (so consumers see the failure) and still ack + // so the offline queue is cleared and the stanza is not redelivered. + let is_view_once = matches!( + unavailable_type, + crate::types::events::UnavailableType::ViewOnce ); + if is_view_once { + log::info!( + "[msg:{}] Message has child (type: ViewOnce); \ + unrecoverable via PDO — skipping request and acking", + info.id, + ); + } else { + log::info!( + "[msg:{}] Message has child (type: {:?}), requesting from phone via PDO", + info.id, + unavailable_type + ); + } // PDO is the only recovery here (no retry receipt), so run it before // the transport ack in one flush task: the ack must not clear the // offline queue before the PDO request goes out. status is acked by @@ -113,10 +133,16 @@ impl Client { let info2 = Arc::clone(&info); let skip_ack = info.source.chat.is_status_broadcast(); self.outbound_flush.spawn(&*self.runtime, async move { - // Only ack once the PDO request is out (or skipped as ancient); - // a transient send failure leaves it queued for redelivery. - let pdo_sent = client.run_pdo_request(&info2).await; - if !skip_ack && pdo_sent { + // View-once has no recoverable content, so skip the PDO entirely + // and ack directly. Otherwise PDO is the only recovery: ack only + // once the request is out (or skipped as ancient); a transient + // send failure leaves it queued for redelivery. + let should_ack = if is_view_once { + true + } else { + client.run_pdo_request(&info2).await + }; + if !skip_ack && should_ack { client.send_transport_ack(&info2).await; } }); diff --git a/src/message/tests.rs b/src/message/tests.rs index 2c190fa58..26ebed6ab 100644 --- a/src/message/tests.rs +++ b/src/message/tests.rs @@ -5218,8 +5218,9 @@ async fn test_unavailable_with_enc_skips_unavailable_shortcut() { /// Untrusted companions (web-class `PlatformType`) get the bare stub — /// `` without ``. That path must still emit a -/// `ViewOnce` `UndecryptableMessage` so consumers surface the failure -/// while the phone relays via PDO. +/// `ViewOnce` `UndecryptableMessage` so consumers surface the failure. +/// (The PDO resend is deliberately skipped for view-once — see +/// `view_once_stub_acks_without_pdo`.) #[tokio::test] async fn test_unavailable_without_enc_dispatches_view_once_event() { let client = create_test_client_for_retry_with_id("unavailable_stub").await; @@ -5236,6 +5237,44 @@ async fn test_unavailable_without_enc_dispatches_view_once_event() { ); } +/// View-once media is never shared with companion/linked devices, so a PDO +/// placeholder-resend to our own phone comes back empty — and that peer +/// round-trip surfaces as a spurious "Finished syncing with WhatsApp on +/// " notification on the user's phone for every view-once received. +/// The bare view-once stub must therefore ack the stanza directly (so the +/// server stops redelivering it) instead of gating the ack on a futile PDO +/// request, while still surfacing the failure to consumers. +#[tokio::test] +async fn view_once_stub_acks_without_pdo() { + let (client, transport) = capturing_client("view_once_ack").await; + let recorder = Arc::new(EventRecorder::default()); + client.register_handler(recorder.clone()); + + let node = build_unavailable_stanza("5511777776666@s.whatsapp.net", "VIEW_ONCE_ACK_1", false); + client.clone().handle_incoming_message(node).await; + + // The (unrecoverable) failure still surfaces to consumers. + assert_eq!( + recorder.view_once_unavailable_count(), + 1, + "bare view-once stub must still dispatch a ViewOnce UndecryptableMessage", + ); + + // The stanza is acked so the offline queue drains, without gating on a PDO. + let mut acked = false; + for _ in 0..80 { + if find_message_ack(&transport.sent()).is_some() { + acked = true; + break; + } + tokio::time::sleep(std::time::Duration::from_millis(25)).await; + } + assert!( + acked, + "view-once stub must emit an to drain the offline queue", + ); +} + /// The event struct has no "recovery pending" flag, so consumers cannot /// wait for a PDO outcome before surfacing failure — adding a field /// here forces a conscious UX decision.