fix(review): drop <stanza-id by> checks overlapping with PR #105
Some checks failed
CI Code / Check spelling (pull_request) Successful in 17s
CI Code / Check coding style (pull_request) Successful in 30s
CI Code / Code Coverage (pull_request) Failing after 10m2s
CI Code / Linux (debian) (pull_request) Failing after 12m31s
CI Code / Linux (ubuntu) (pull_request) Failing after 12m41s
CI Code / Linux (arch) (pull_request) Failing after 13m25s

After review of PR #105 (upstream-full merge) it became clear the
<stanza-id by> validation for MUC and 1:1 chat is already added there
via upstream profanity. Keeping both versions would conflict on merge.

Removed from this PR:
  - _handle_groupchat <stanza-id by == room JID> check
    (identical to PR #105)
  - _handle_chat <stanza-id by == my_domain> check
    (PR #105 uses equals_our_barejid(by) instead — defer to upstream)
  - STANZA_ATTR_BY define (no longer referenced)

Kept (PR #105 does NOT touch these):
  - XEP-0203 §4 <delay from=?> validation in _handle_groupchat,
    _handle_chat, _handle_muc_private_message
  - stanza_get_oldest_delay_from() helper + NULL-tmp guard bug fix
    in stanza_get_oldest_delay()

Also applied review fixes:
  - Add g_date_time_new_now_local() fallback in
    _handle_muc_private_message so timestamp is never NULL
  - Change stanza_get_delay_from signature to const char* const from,
    eliminating (gchar*) casts at call sites
This commit is contained in:
2026-05-16 12:42:08 +03:00
parent 7fb926c78a
commit ffc626a276
3 changed files with 17 additions and 25 deletions

View File

@@ -1090,16 +1090,12 @@ _handle_groupchat(xmpp_stanza_t* const stanza)
message->id = strdup(id);
}
// XEP-0359 §5: only accept <stanza-id> whose 'by' matches the MUC room
char* stanzaid = NULL;
xmpp_stanza_t* stanzaidst = xmpp_stanza_get_child_by_name_and_ns(stanza, STANZA_NAME_STANZA_ID, STANZA_NS_STABLE_ID);
if (stanzaidst) {
const char* by = xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_BY);
if (by && g_strcmp0(by, from_jid->barejid) == 0) {
stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID);
if (stanzaid) {
message->stanzaid = strdup(stanzaid);
}
stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID);
if (stanzaid) {
message->stanzaid = strdup(stanzaid);
}
}
@@ -1288,12 +1284,13 @@ _handle_muc_private_message(xmpp_stanza_t* const stanza)
#endif
// XEP-0203 §4: only trust <delay> from a server domain.
{
const char* my_domain = connection_get_domain();
message->timestamp = stanza_get_delay_from(stanza, (gchar*)my_domain);
if (!message->timestamp && message->from_jid->domainpart) {
message->timestamp = stanza_get_delay_from(stanza, message->from_jid->domainpart);
}
const char* my_domain = connection_get_domain();
message->timestamp = stanza_get_delay_from(stanza, my_domain);
if (!message->timestamp && message->from_jid->domainpart) {
message->timestamp = stanza_get_delay_from(stanza, message->from_jid->domainpart);
}
if (!message->timestamp) {
message->timestamp = g_date_time_new_now_local();
}
message->body = xmpp_message_get_body(stanza);
@@ -1418,17 +1415,13 @@ _handle_chat(xmpp_stanza_t* const stanza, gboolean is_mam, gboolean is_carbon, c
log_warning("MAM received with no result id");
}
} else {
// XEP-0359 §5: only accept <stanza-id> whose 'by' matches our server
// live messages use XEP-0359 <stanza-id>
char* stanzaid = NULL;
xmpp_stanza_t* stanzaidst = xmpp_stanza_get_child_by_name_and_ns(stanza, STANZA_NAME_STANZA_ID, STANZA_NS_STABLE_ID);
if (stanzaidst) {
const char* by = xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_BY);
const char* my_domain = connection_get_domain();
if (by && my_domain && g_strcmp0(by, my_domain) == 0) {
stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID);
if (stanzaid) {
message->stanzaid = strdup(stanzaid);
}
stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID);
if (stanzaid) {
message->stanzaid = strdup(stanzaid);
}
}
}
@@ -1449,7 +1442,7 @@ _handle_chat(xmpp_stanza_t* const stanza, gboolean is_mam, gboolean is_carbon, c
// XEP-0203 §4: only trust <delay> from a server domain, not from
// the remote client which could forge an arbitrary timestamp.
const char* my_domain = connection_get_domain();
message->timestamp = stanza_get_delay_from(stanza, (gchar*)my_domain);
message->timestamp = stanza_get_delay_from(stanza, my_domain);
if (!message->timestamp && jid->domainpart) {
message->timestamp = stanza_get_delay_from(stanza, jid->domainpart);
}

View File

@@ -1183,7 +1183,7 @@ _stanza_get_delay_timestamp_xep0091(xmpp_stanza_t* const x_stanza)
}
GDateTime*
stanza_get_delay_from(xmpp_stanza_t* const stanza, gchar* from)
stanza_get_delay_from(xmpp_stanza_t* const stanza, const char* const from)
{
xmpp_stanza_t* delay = NULL;

View File

@@ -196,7 +196,6 @@
#define STANZA_ATTR_FILENAME "filename"
#define STANZA_ATTR_SIZE "size"
#define STANZA_ATTR_CONTENTTYPE "content-type"
#define STANZA_ATTR_BY "by"
#define STANZA_ATTR_LABEL "label"
#define STANZA_TEXT_AWAY "away"
@@ -330,7 +329,7 @@ xmpp_stanza_t* stanza_create_mediated_invite(xmpp_ctx_t* ctx, const char* const
gboolean stanza_contains_chat_state(xmpp_stanza_t* stanza);
GDateTime* stanza_get_delay(xmpp_stanza_t* const stanza);
GDateTime* stanza_get_delay_from(xmpp_stanza_t* const stanza, gchar* from);
GDateTime* stanza_get_delay_from(xmpp_stanza_t* const stanza, const char* const from);
GDateTime* stanza_get_oldest_delay(xmpp_stanza_t* const stanza);
GDateTime* stanza_get_oldest_delay_from(xmpp_stanza_t* const stanza, const char* const from);