fix(history): use OR instead of AND for JID validation in extractLidPnFromMessage
Address PR #20 code review feedback: - Fix logical bug: changed `&&` to `||` in isPersonJid validation This ensures BOTH JIDs must be person JIDs before extracting a LID-PN mapping, preventing "poisoned" mappings where one side is a group/newsletter/broadcast - Add isPersonJid() helper to validate JID types - Add extractLidPnFromConversation() to extract mappings from conversation lidJid/pnJid properties - Add extractLidPnFromMessage() to extract mappings from message remoteJidAlt/participantAlt fields - Update processHistoryMessage() to use new extraction functions - Add comprehensive tests covering edge cases for mixed JID types (person + group, person + broadcast, person + newsletter) Note: Returning `undefined` is the correct TypeScript convention for indicating absence of value (as per TypeScript team guidelines)
This commit is contained in:
Claude
+151
-5
@@ -6,9 +6,134 @@ import { WAMessageStubType } from '../Types'
|
||||
import { toNumber } from './generics'
|
||||
import { normalizeMessageContent } from './messages'
|
||||
import { downloadContentFromMessage } from './messages-media'
|
||||
import {
|
||||
isHostedLidUser,
|
||||
isHostedPnUser,
|
||||
isJidBroadcast,
|
||||
isJidGroup,
|
||||
isJidNewsletter,
|
||||
isLidUser,
|
||||
isPnUser,
|
||||
jidDecode,
|
||||
jidNormalizedUser,
|
||||
} from '../WABinary/jid-utils'
|
||||
|
||||
const inflatePromise = promisify(inflate)
|
||||
|
||||
/**
|
||||
* Checks if a JID represents a person (individual user) rather than a group, broadcast, or newsletter.
|
||||
* Only person JIDs (LID or PN formats) can have LID-PN mappings.
|
||||
*/
|
||||
export function isPersonJid(jid: string | undefined): boolean {
|
||||
if(!jid) {
|
||||
return false
|
||||
}
|
||||
|
||||
// Groups, broadcasts, and newsletters don't have LID-PN mappings
|
||||
if(isJidGroup(jid) || isJidBroadcast(jid) || isJidNewsletter(jid)) {
|
||||
return false
|
||||
}
|
||||
|
||||
// Only person JIDs (LID or PN formats) can have mappings
|
||||
return !!(
|
||||
isLidUser(jid) ||
|
||||
isHostedLidUser(jid) ||
|
||||
isPnUser(jid) ||
|
||||
isHostedPnUser(jid)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts a LID-PN mapping from conversation data.
|
||||
* Returns undefined if the conversation doesn't represent a valid person-to-person mapping.
|
||||
*/
|
||||
export function extractLidPnFromConversation(
|
||||
chatId: string,
|
||||
lidJid: string | undefined | null,
|
||||
pnJid: string | undefined | null
|
||||
): LIDMapping | undefined {
|
||||
// Skip non-person JIDs (groups, broadcasts, newsletters)
|
||||
if(!isPersonJid(chatId)) {
|
||||
return undefined
|
||||
}
|
||||
|
||||
const chatIsLid = isLidUser(chatId) || isHostedLidUser(chatId)
|
||||
const chatIsPn = isPnUser(chatId) || isHostedPnUser(chatId)
|
||||
|
||||
if(chatIsLid && pnJid) {
|
||||
return {
|
||||
lid: jidNormalizedUser(chatId),
|
||||
pn: jidNormalizedUser(pnJid)
|
||||
}
|
||||
}
|
||||
|
||||
if(chatIsPn && lidJid) {
|
||||
return {
|
||||
lid: jidNormalizedUser(lidJid),
|
||||
pn: jidNormalizedUser(chatId)
|
||||
}
|
||||
}
|
||||
|
||||
return undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts a LID-PN mapping from message data (remoteJidAlt/participantAlt fields).
|
||||
* Returns undefined if the message doesn't contain a valid person-to-person mapping.
|
||||
*
|
||||
* IMPORTANT: Uses || (OR) to ensure BOTH JIDs are person JIDs before extracting.
|
||||
* This prevents "poisoned" mappings where one side is a group/newsletter/broadcast.
|
||||
*/
|
||||
export function extractLidPnFromMessage(
|
||||
remoteJid: string | undefined | null,
|
||||
remoteJidAlt: string | undefined | null,
|
||||
participant: string | undefined | null,
|
||||
participantAlt: string | undefined | null
|
||||
): LIDMapping | undefined {
|
||||
const primaryJid = participant || remoteJid
|
||||
const altJid = participantAlt || remoteJidAlt
|
||||
|
||||
if(!primaryJid || !altJid) {
|
||||
return undefined
|
||||
}
|
||||
|
||||
// FIXED: Use || (OR) instead of && (AND)
|
||||
// Both JIDs MUST be person JIDs to create a valid mapping.
|
||||
// Using && would allow mixed scenarios (e.g., person + group) to proceed,
|
||||
// resulting in invalid "poisoned" mappings.
|
||||
if(!isPersonJid(primaryJid) || !isPersonJid(altJid)) {
|
||||
return undefined
|
||||
}
|
||||
|
||||
const primaryDecoded = jidDecode(primaryJid)
|
||||
const altDecoded = jidDecode(altJid)
|
||||
|
||||
if(!primaryDecoded || !altDecoded) {
|
||||
return undefined
|
||||
}
|
||||
|
||||
const primaryIsLid = primaryDecoded.server === 'lid' ||
|
||||
primaryDecoded.server === 'hosted.lid'
|
||||
const altIsLid = altDecoded.server === 'lid' ||
|
||||
altDecoded.server === 'hosted.lid'
|
||||
|
||||
if(primaryIsLid && !altIsLid) {
|
||||
return {
|
||||
lid: jidNormalizedUser(primaryJid),
|
||||
pn: jidNormalizedUser(altJid)
|
||||
}
|
||||
}
|
||||
|
||||
if(!primaryIsLid && altIsLid) {
|
||||
return {
|
||||
lid: jidNormalizedUser(altJid),
|
||||
pn: jidNormalizedUser(primaryJid)
|
||||
}
|
||||
}
|
||||
|
||||
return undefined
|
||||
}
|
||||
|
||||
export const downloadHistory = async (msg: proto.Message.IHistorySyncNotification, options: RequestInit) => {
|
||||
const stream = await downloadContentFromMessage(msg, 'md-msg-hist', { options })
|
||||
const bufferArray: Buffer[] = []
|
||||
@@ -42,7 +167,7 @@ export const processHistoryMessage = (item: proto.IHistorySync) => {
|
||||
case proto.HistorySync.HistorySyncType.RECENT:
|
||||
case proto.HistorySync.HistorySyncType.FULL:
|
||||
case proto.HistorySync.HistorySyncType.ON_DEMAND:
|
||||
for (const chat of item.conversations! as Chat[]) {
|
||||
for(const chat of item.conversations! as Chat[]) {
|
||||
contacts.push({
|
||||
id: chat.id!,
|
||||
name: chat.name || undefined,
|
||||
@@ -50,23 +175,33 @@ export const processHistoryMessage = (item: proto.IHistorySync) => {
|
||||
phoneNumber: chat.pnJid || undefined
|
||||
})
|
||||
|
||||
// Extract LID-PN mapping from conversation (lidJid/pnJid properties)
|
||||
const convMapping = extractLidPnFromConversation(
|
||||
chat.id!,
|
||||
chat.lidJid,
|
||||
chat.pnJid
|
||||
)
|
||||
if(convMapping) {
|
||||
lidPnMappings.push(convMapping)
|
||||
}
|
||||
|
||||
const msgs = chat.messages || []
|
||||
delete chat.messages
|
||||
|
||||
for (const item of msgs) {
|
||||
for(const item of msgs) {
|
||||
const message = item.message! as WAMessage
|
||||
messages.push(message)
|
||||
|
||||
if (!chat.messages?.length) {
|
||||
if(!chat.messages?.length) {
|
||||
// keep only the most recent message in the chat array
|
||||
chat.messages = [{ message }]
|
||||
}
|
||||
|
||||
if (!message.key.fromMe && !chat.lastMessageRecvTimestamp) {
|
||||
if(!message.key.fromMe && !chat.lastMessageRecvTimestamp) {
|
||||
chat.lastMessageRecvTimestamp = toNumber(message.messageTimestamp)
|
||||
}
|
||||
|
||||
if (
|
||||
if(
|
||||
(message.messageStubType === WAMessageStubType.BIZ_PRIVACY_MODE_TO_BSP ||
|
||||
message.messageStubType === WAMessageStubType.BIZ_PRIVACY_MODE_TO_FB) &&
|
||||
message.messageStubParameters?.[0]
|
||||
@@ -76,6 +211,17 @@ export const processHistoryMessage = (item: proto.IHistorySync) => {
|
||||
verifiedName: message.messageStubParameters?.[0]
|
||||
})
|
||||
}
|
||||
|
||||
// Extract LID-PN mapping from message (remoteJidAlt/participantAlt fields)
|
||||
const msgMapping = extractLidPnFromMessage(
|
||||
message.key.remoteJid,
|
||||
(message.key as { remoteJidAlt?: string }).remoteJidAlt,
|
||||
message.key.participant,
|
||||
(message.key as { participantAlt?: string }).participantAlt
|
||||
)
|
||||
if(msgMapping) {
|
||||
lidPnMappings.push(msgMapping)
|
||||
}
|
||||
}
|
||||
|
||||
chats.push({ ...chat })
|
||||
|
||||
@@ -1,5 +1,10 @@
|
||||
import { proto } from '../../../WAProto/index.js'
|
||||
import { processHistoryMessage } from '../../Utils/history'
|
||||
import {
|
||||
extractLidPnFromConversation,
|
||||
extractLidPnFromMessage,
|
||||
isPersonJid,
|
||||
processHistoryMessage
|
||||
} from '../../Utils/history'
|
||||
|
||||
describe('processHistoryMessage', () => {
|
||||
describe('phoneNumberToLidMappings extraction', () => {
|
||||
@@ -90,5 +95,325 @@ describe('processHistoryMessage', () => {
|
||||
phoneNumber: '1234567890123@s.whatsapp.net'
|
||||
})
|
||||
})
|
||||
|
||||
it('should extract LID-PN mappings from conversation lidJid/pnJid properties', () => {
|
||||
const historySync: proto.IHistorySync = {
|
||||
syncType: proto.HistorySync.HistorySyncType.INITIAL_BOOTSTRAP,
|
||||
conversations: [
|
||||
{
|
||||
id: '1234567890123@s.whatsapp.net',
|
||||
name: 'Test User',
|
||||
lidJid: '11111111111111@lid',
|
||||
pnJid: '1234567890123@s.whatsapp.net'
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
const result = processHistoryMessage(historySync)
|
||||
|
||||
// Should include mapping extracted from conversation
|
||||
expect(result.lidPnMappings).toContainEqual({
|
||||
lid: '11111111111111@lid',
|
||||
pn: '1234567890123@s.whatsapp.net'
|
||||
})
|
||||
})
|
||||
|
||||
it('should NOT extract LID-PN mappings from group conversations', () => {
|
||||
const historySync: proto.IHistorySync = {
|
||||
syncType: proto.HistorySync.HistorySyncType.INITIAL_BOOTSTRAP,
|
||||
conversations: [
|
||||
{
|
||||
id: '123456789@g.us',
|
||||
name: 'Test Group',
|
||||
lidJid: '11111111111111@lid',
|
||||
pnJid: '1234567890123@s.whatsapp.net'
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
const result = processHistoryMessage(historySync)
|
||||
|
||||
// Should NOT include mapping from group conversation
|
||||
expect(result.lidPnMappings).toEqual([])
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('isPersonJid', () => {
|
||||
it('should return true for LID users', () => {
|
||||
expect(isPersonJid('11111111111111@lid')).toBe(true)
|
||||
})
|
||||
|
||||
it('should return true for hosted LID users', () => {
|
||||
expect(isPersonJid('11111111111111@hosted.lid')).toBe(true)
|
||||
})
|
||||
|
||||
it('should return true for PN users (s.whatsapp.net)', () => {
|
||||
expect(isPersonJid('1234567890123@s.whatsapp.net')).toBe(true)
|
||||
})
|
||||
|
||||
it('should return true for hosted PN users', () => {
|
||||
expect(isPersonJid('1234567890123@hosted')).toBe(true)
|
||||
})
|
||||
|
||||
it('should return false for groups', () => {
|
||||
expect(isPersonJid('123456789@g.us')).toBe(false)
|
||||
})
|
||||
|
||||
it('should return false for broadcasts', () => {
|
||||
expect(isPersonJid('status@broadcast')).toBe(false)
|
||||
})
|
||||
|
||||
it('should return false for newsletters', () => {
|
||||
expect(isPersonJid('123456789@newsletter')).toBe(false)
|
||||
})
|
||||
|
||||
it('should return false for undefined', () => {
|
||||
expect(isPersonJid(undefined)).toBe(false)
|
||||
})
|
||||
|
||||
it('should return false for empty string', () => {
|
||||
expect(isPersonJid('')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('extractLidPnFromConversation', () => {
|
||||
it('should extract mapping when chat is PN and lidJid is provided', () => {
|
||||
const result = extractLidPnFromConversation(
|
||||
'1234567890123@s.whatsapp.net',
|
||||
'11111111111111@lid',
|
||||
null
|
||||
)
|
||||
|
||||
expect(result).toEqual({
|
||||
lid: '11111111111111@lid',
|
||||
pn: '1234567890123@s.whatsapp.net'
|
||||
})
|
||||
})
|
||||
|
||||
it('should extract mapping when chat is LID and pnJid is provided', () => {
|
||||
const result = extractLidPnFromConversation(
|
||||
'11111111111111@lid',
|
||||
null,
|
||||
'1234567890123@s.whatsapp.net'
|
||||
)
|
||||
|
||||
expect(result).toEqual({
|
||||
lid: '11111111111111@lid',
|
||||
pn: '1234567890123@s.whatsapp.net'
|
||||
})
|
||||
})
|
||||
|
||||
it('should return undefined for group chats', () => {
|
||||
const result = extractLidPnFromConversation(
|
||||
'123456789@g.us',
|
||||
'11111111111111@lid',
|
||||
'1234567890123@s.whatsapp.net'
|
||||
)
|
||||
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should return undefined for broadcast chats', () => {
|
||||
const result = extractLidPnFromConversation(
|
||||
'status@broadcast',
|
||||
'11111111111111@lid',
|
||||
'1234567890123@s.whatsapp.net'
|
||||
)
|
||||
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should return undefined for newsletter chats', () => {
|
||||
const result = extractLidPnFromConversation(
|
||||
'123456789@newsletter',
|
||||
'11111111111111@lid',
|
||||
'1234567890123@s.whatsapp.net'
|
||||
)
|
||||
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should return undefined when no mapping data is provided', () => {
|
||||
const result = extractLidPnFromConversation(
|
||||
'1234567890123@s.whatsapp.net',
|
||||
null,
|
||||
null
|
||||
)
|
||||
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('extractLidPnFromMessage', () => {
|
||||
it('should extract mapping when primary is LID and alt is PN', () => {
|
||||
const result = extractLidPnFromMessage(
|
||||
'11111111111111@lid',
|
||||
'1234567890123@s.whatsapp.net',
|
||||
null,
|
||||
null
|
||||
)
|
||||
|
||||
expect(result).toEqual({
|
||||
lid: '11111111111111@lid',
|
||||
pn: '1234567890123@s.whatsapp.net'
|
||||
})
|
||||
})
|
||||
|
||||
it('should extract mapping when primary is PN and alt is LID', () => {
|
||||
const result = extractLidPnFromMessage(
|
||||
'1234567890123@s.whatsapp.net',
|
||||
'11111111111111@lid',
|
||||
null,
|
||||
null
|
||||
)
|
||||
|
||||
expect(result).toEqual({
|
||||
lid: '11111111111111@lid',
|
||||
pn: '1234567890123@s.whatsapp.net'
|
||||
})
|
||||
})
|
||||
|
||||
it('should prefer participant over remoteJid', () => {
|
||||
const result = extractLidPnFromMessage(
|
||||
'11111111111111@lid',
|
||||
'22222222222222@lid',
|
||||
'33333333333333@lid',
|
||||
'1234567890123@s.whatsapp.net'
|
||||
)
|
||||
|
||||
// participant (33333333333333@lid) and participantAlt (1234567890123@s.whatsapp.net) should be used
|
||||
expect(result).toEqual({
|
||||
lid: '33333333333333@lid',
|
||||
pn: '1234567890123@s.whatsapp.net'
|
||||
})
|
||||
})
|
||||
|
||||
it('should return undefined when primaryJid is missing', () => {
|
||||
const result = extractLidPnFromMessage(
|
||||
null,
|
||||
'1234567890123@s.whatsapp.net',
|
||||
null,
|
||||
null
|
||||
)
|
||||
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should return undefined when altJid is missing', () => {
|
||||
const result = extractLidPnFromMessage(
|
||||
'11111111111111@lid',
|
||||
null,
|
||||
null,
|
||||
null
|
||||
)
|
||||
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
|
||||
// CRITICAL: Edge cases for the && vs || fix
|
||||
describe('edge cases for mixed JID types (bug fix validation)', () => {
|
||||
it('should return undefined when primary is person but alt is GROUP', () => {
|
||||
const result = extractLidPnFromMessage(
|
||||
'11111111111111@lid',
|
||||
'123456789@g.us', // group JID
|
||||
null,
|
||||
null
|
||||
)
|
||||
|
||||
// With the fix (using ||), this should return undefined
|
||||
// because group JIDs cannot be part of a valid LID-PN mapping
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should return undefined when primary is person but alt is BROADCAST', () => {
|
||||
const result = extractLidPnFromMessage(
|
||||
'11111111111111@lid',
|
||||
'status@broadcast', // broadcast JID
|
||||
null,
|
||||
null
|
||||
)
|
||||
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should return undefined when primary is person but alt is NEWSLETTER', () => {
|
||||
const result = extractLidPnFromMessage(
|
||||
'11111111111111@lid',
|
||||
'123456789@newsletter', // newsletter JID
|
||||
null,
|
||||
null
|
||||
)
|
||||
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should return undefined when alt is person but primary is GROUP', () => {
|
||||
const result = extractLidPnFromMessage(
|
||||
'123456789@g.us', // group JID
|
||||
'1234567890123@s.whatsapp.net',
|
||||
null,
|
||||
null
|
||||
)
|
||||
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should return undefined when alt is person but primary is BROADCAST', () => {
|
||||
const result = extractLidPnFromMessage(
|
||||
'status@broadcast', // broadcast JID
|
||||
'1234567890123@s.whatsapp.net',
|
||||
null,
|
||||
null
|
||||
)
|
||||
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should return undefined when alt is person but primary is NEWSLETTER', () => {
|
||||
const result = extractLidPnFromMessage(
|
||||
'123456789@newsletter', // newsletter JID
|
||||
'1234567890123@s.whatsapp.net',
|
||||
null,
|
||||
null
|
||||
)
|
||||
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should return undefined when BOTH are non-person JIDs', () => {
|
||||
const result = extractLidPnFromMessage(
|
||||
'123456789@g.us', // group JID
|
||||
'status@broadcast', // broadcast JID
|
||||
null,
|
||||
null
|
||||
)
|
||||
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
it('should return undefined when both are same type (both LID)', () => {
|
||||
const result = extractLidPnFromMessage(
|
||||
'11111111111111@lid',
|
||||
'22222222222222@lid',
|
||||
null,
|
||||
null
|
||||
)
|
||||
|
||||
// Both are LID, no PN - can't determine mapping
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should return undefined when both are same type (both PN)', () => {
|
||||
const result = extractLidPnFromMessage(
|
||||
'1234567890123@s.whatsapp.net',
|
||||
'9876543210987@s.whatsapp.net',
|
||||
null,
|
||||
null
|
||||
)
|
||||
|
||||
// Both are PN, no LID - can't determine mapping
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user