fix(blocklist): include LID + pn_jid in block IQ (modern WA) - #600
Merged
Conversation
Modern WhatsApp servers (2026+) reject `<item action="block" jid="...@s.whatsapp.net"/>` with `code=400 bad-request`. After the LID rollout the blocklist IQ requires both halves of the LID↔PN pair: <item action="block" jid="<lid>@lid" pn_jid="<phone>@s.whatsapp.net"/> Verified against Baileys' `updateBlockStatus` (`Socket/chats.ts`), which now resolves both sides via its LID mapping store before sending. Changes: * `BlocklistItemRequest` gains an optional `pn_jid` attribute and a new `block_with_pn(lid, pn)` constructor. Old `block(jid)` / `unblock(jid)` keep working for callers that already have the right shape. * `Blocking::block` and `Blocking::unblock` now resolve LID↔PN through `client.get_lid_pn_entry` regardless of which side the caller passes, then send the modern stanza: - block: `jid=LID, pn_jid=PN` - unblock: `jid=LID` (PN not required by the server for removals) * Returns a structured `IqError::ServerError` when no LID↔PN mapping is available — block silently no-op'd before, masking the bug. Tests: * `test_block_with_pn_emits_pn_jid_attr` — asserts both attrs on the wire * `test_unblock_omits_pn_jid_attr` — asserts unblock keeps the old shape
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughBlock/unblock now normalize input JIDs to non-AD form and resolve LID↔PN mappings via a new resolver that accepts either PN or LID. Block sends an IQ using Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Feature as features/blocking.rs
participant Store as client.get_lid_pn_entry
participant BlockSpec as wacore::UpdateBlocklistSpec
participant IQ as IQ Handler
rect rgba(33, 150, 243, 0.5)
Client->>Feature: block(jid)
Feature->>Feature: jid.to_non_ad()
Feature->>Store: get_lid_pn_entry(normalized_jid)
Store-->>Feature: (lid_jid, pn_jid) or error
alt mapping found
Feature->>BlockSpec: block_with_pn(&lid_jid, &pn_jid)
BlockSpec->>IQ: send IQ with pn_jid attribute
IQ-->>Feature: success
else mapping missing / invalid
Feature-->>Client: return IqError::ServerError
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/blocking.rs`:
- Around line 31-64: Extract the duplicated LID/PN mapping logic into a single
helper method (e.g. impl method resolve_lid_pn(&self, bare: &Jid) -> Result<(Jid
/*lid*/, Jid /*pn*/), IqError>) that encapsulates the two branches: call
self.client.get_lid_pn_entry(bare).await, map client errors into
IqError::ServerError with the existing messages, return (bare,
Jid::pn(entry.phone_number)) when bare.is_lid(), return (Jid::lid(entry.lid),
bare) when bare.is_pn(), and return the "neither PN nor LID" IqError for other
cases; then replace the duplicate blocks in block and unblock (and the similar
code at the 79-100 region) to call resolve_lid_pn and use its returned (lid_jid,
pn_jid).
- Around line 42-43: The code currently includes raw PN/LID identifiers (e.g.,
using the variable bare in text: format!("no LID↔PN mapping for {bare}")) which
leaks user IDs; replace those raw identifiers with a redacted representation
everywhere referenced (lines around the format call and the other occurrences at
the listed locations). Implement or call a small helper (e.g., redact_id or
redact) to convert bare into a non-identifying token (like "<redacted>", a fixed
prefix+hash, or masked string) and use that in all user-facing logs/errors such
as the message in the error return and the other similar format! calls so the
message becomes generic (e.g., "no LID↔PN mapping for <redacted>") while keeping
code paths and error types unchanged (update references to bare inside format!
calls and any log invocations).
In `@wacore/src/iq/blocklist.rs`:
- Around line 28-33: The blocklist comment includes time-bound operational
details (e.g., "2026-04") and verbose protocol explanation; replace it with a
single concise "why" line such as "Wire requires jid in LID and pn_jid for
blocking" immediately above the ProtocolNode derive, and move the empirical
verification and dated specifics into docs or integration tests (e.g.,
compatibility notes or a test file). Apply the same trim-and-move change to the
other similar comment blocks referenced (the comments around the other
ProtocolNode or struct docs) so comments remain brief and non-time-bound while
detailed evidence lives in docs/tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8fd8c629-a7c7-43fb-adae-8386efe3306e
📒 Files selected for processing (2)
src/features/blocking.rswacore/src/iq/blocklist.rs
* Extract `Blocking::resolve_lid_pn` to share the LID↔PN resolution between `block` and `unblock` (was duplicated, would drift). * Stop interpolating raw PN/LID identifiers into error messages and debug logs — the `Blocking` API now returns generic "blocklist: no LID↔PN mapping for provided jid" / "lookup failed" / "neither PN nor LID" so user IDs don't end up in surfaces that get logged or surfaced upstream. * Trim the protocol comment on `BlocklistItemRequest` and the `block_with_pn` constructors down to a single "why" line; drop dated empirical notes (those belong in tests, which already cover the wire shape).
…k_with_pn
Preflight failures (no LID↔PN mapping, jid neither PN nor LID, backend
lookup error) were emitted as `IqError::ServerError { code: 0 }`, which
collides with real server codes and discards the backend error. Switch
to `IqError::EncodeError(anyhow::Error)` — the existing variant for
"couldn't build the IQ" — and preserve the backend source.
Also collapse the two LID/PN branches into a single `get_lid_pn_entry`
call and have `BlocklistItemRequest::block_with_pn` delegate to
`Self::new` instead of repeating the field list.
jlucaso1
approved these changes
Apr 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Modern WhatsApp servers (2026+) reject the legacy block stanza with
code=400 bad-request:After the LID rollout the blocklist IQ now requires both halves of the LID↔PN pair on the wire:
Reproduced empirically — every call to
Blocking::blockagainst a real account currently logs:Cross-referenced against Baileys'
updateBlockStatus(Socket/chats.ts), which:jid,blockaddspn_jidwith the PN,unblockkeeps only the LIDjid.This PR mirrors that behavior.
Changes
BlocklistItemRequestgains an optionalpn_jidattribute and a newblock_with_pn(lid, pn)constructor. The legacyblock(jid)/unblock(jid)constructors keep their signatures for callers that already pass a pre-resolved JID.Blocking::blockandBlocking::unblocknow useclient.get_lid_pn_entryto resolve the missing side regardless of whether the caller passed LID or PN, then send the modern stanza:jid=LID, pn_jid=PNjid=LIDonlyIqError::ServerErrorinstead of silently sending a malformed stanza — the bug was previously invisible to callers because the server-side 400 isn't surfaced as an error in some flows.Test plan
cargo test -p wacore --lib blocklist— 9 passed (addedtest_block_with_pn_emits_pn_jid_attrandtest_unblock_omits_pn_jid_attr)cargo clippy --all --tests— cleancargo fmt --all— appliedBlocking::blockagainst a live account now succeeds and shows up in WA Web's blocklistNotes for reviewers
BlocklistItemRequest::block(&jid)(nopn_jid) is kept so existing callers compile, but in practice the modern server will reject any IQ produced this way. The high-levelBlocking::blockalways routes throughblock_with_pn. We could deprecate the no-pn_jidform in a follow-up.Blocking::unblockstill only ships LID — Baileys does the same and the server accepts it.