devs/cproof
1
2
Fork 1
Code Issues 43 Pull Requests 6 Actions Packages Projects 1 Releases Wiki Activity

Issue: XEP-0030 Violation - disco#items errors are silently ignored #91

New Issue
Open
opened 2026-02-11 12:21:36 +00:00 by jabber.developer2 · 0 comments
jabber.developer2 commented 2026-02-11 12:21:36 +00:00
Collaborator
Copy Link

Description:

The /disco items command silently ignores error responses from the server. When a disco#items request returns an error (e.g., <item-not-found/>, <service-unavailable/>, <forbidden/>), the user receives no feedback.

XEP-0030 Reference

Per XEP-0030 Section 7 (Error Conditions):

"If a specific entity (JID or JID+node) does not support the disco namespace, refuses to return disco results to the specific requesting entity, or refuses to return disco results to any requesting entity, it SHOULD return an appropriate error message (such as <service-unavailable/>, <forbidden/>, or <not-allowed/>, respectively)."

The specification defines error conditions for both disco#info and disco#items queries:

Condition Cause
<item-not-found/> The JID or JID+NodeID does not exist
<service-unavailable/> Target doesn't support protocol or entity doesn't exist (privacy)

Current Behavior

/disco items nonexistent.server

Result: Nothing happens. No output, no error message.

Expected Behavior

/disco items nonexistent.server

Expected: Service discovery failed for nonexistent.server: item-not-found

(Similar to how /disco info handles errors correctly)

Root Cause Analysis

In iq.c, the disco#items implementation differs from disco#info:

disco#info (correct):

void iq_disco_info_request(const char* jid) {
    auto_char char* id = connection_create_stanza_id();  // dynamic id
    // ...
    iq_id_handler_add(id, _disco_info_response_id_handler, NULL, NULL);  // handles all response types
}

disco#items (buggy):

void iq_disco_items_request(const char* jid) {
    xmpp_stanza_t* iq = stanza_create_disco_items_iq(ctx, "discoitemsreq", jid, NULL);  // fixed id
    iq_send_stanza(iq);  // NO id_handler registered!
}

The response routing in the IQ handler (line ~205):

if (discoitems && (g_strcmp0(type, STANZA_TYPE_RESULT) == 0)) {
    _disco_items_result_handler(stanza);
}
// No handler for type='error'!

Comparison Table

Aspect disco#info disco#items
Stanza ID Dynamic (connection_create_stanza_id()) Fixed ("discoitemsreq")
Response handler iq_id_handler_add() None (namespace-based routing)
Error handling ✅ cons_show_error() ❌ Silently ignored
XEP-0030 compliant ✅ Yes ❌ No

Suggested Fix

Refactor iq_disco_items_request() to follow the pattern of iq_disco_info_request():

  1. Use dynamic stanza ID
  2. Register an id_handler for response processing
  3. Create _disco_items_response_id_handler() that checks for type='error'
  4. Show error message to user via cons_show_error()

Test Case

A functional test has been added that documents this bug:

void disco_items_error_not_handled(void **state)

This test currently uses assert_false(prof_output_regex("Service discovery failed")) to verify the bug exists. When fixed, it should be changed to assert_true().

Related

### Description: The `/disco items` command silently ignores error responses from the server. When a disco#items request returns an error (e.g., `<item-not-found/>`, `<service-unavailable/>`, `<forbidden/>`), the user receives no feedback. ### XEP-0030 Reference Per [XEP-0030 Section 7 (Error Conditions)](https://xmpp.org/extensions/xep-0030.html#errors): > "If a specific entity (JID or JID+node) does not support the disco namespace, refuses to return disco results to the specific requesting entity, or refuses to return disco results to any requesting entity, **it SHOULD return an appropriate error message** (such as `<service-unavailable/>`, `<forbidden/>`, or `<not-allowed/>`, respectively)." The specification defines error conditions for **both** disco#info and disco#items queries: | Condition | Cause | |-----------|-------| | `<item-not-found/>` | The JID or JID+NodeID does not exist | | `<service-unavailable/>` | Target doesn't support protocol or entity doesn't exist (privacy) | ### Current Behavior ``` /disco items nonexistent.server ``` **Result**: Nothing happens. No output, no error message. ### Expected Behavior ``` /disco items nonexistent.server ``` **Expected**: `Service discovery failed for nonexistent.server: item-not-found` (Similar to how `/disco info` handles errors correctly) ### Root Cause Analysis In iq.c, the disco#items implementation differs from disco#info: **disco#info (correct)**: ```c void iq_disco_info_request(const char* jid) { auto_char char* id = connection_create_stanza_id(); // dynamic id // ... iq_id_handler_add(id, _disco_info_response_id_handler, NULL, NULL); // handles all response types } ``` **disco#items (buggy)**: ```c void iq_disco_items_request(const char* jid) { xmpp_stanza_t* iq = stanza_create_disco_items_iq(ctx, "discoitemsreq", jid, NULL); // fixed id iq_send_stanza(iq); // NO id_handler registered! } ``` The response routing in the IQ handler (line ~205): ```c if (discoitems && (g_strcmp0(type, STANZA_TYPE_RESULT) == 0)) { _disco_items_result_handler(stanza); } // No handler for type='error'! ``` ### Comparison Table | Aspect | disco#info | disco#items | |--------|------------|-------------| | Stanza ID | Dynamic (`connection_create_stanza_id()`) | Fixed (`"discoitemsreq"`) | | Response handler | `iq_id_handler_add()` | None (namespace-based routing) | | Error handling | ✅ `cons_show_error()` | ❌ Silently ignored | | XEP-0030 compliant | ✅ Yes | ❌ No | ### Suggested Fix Refactor `iq_disco_items_request()` to follow the pattern of `iq_disco_info_request()`: 1. Use dynamic stanza ID 2. Register an `id_handler` for response processing 3. Create `_disco_items_response_id_handler()` that checks for `type='error'` 4. Show error message to user via `cons_show_error()` ### Test Case A functional test has been added that documents this bug: ```c void disco_items_error_not_handled(void **state) ``` This test currently uses `assert_false(prof_output_regex("Service discovery failed"))` to verify the bug exists. When fixed, it should be changed to `assert_true()`. ### Related - XEP-0030: https://xmpp.org/extensions/xep-0030.html - Section 7 - Error Conditions: https://xmpp.org/extensions/xep-0030.html#errors - Related fix: #XX (fix for empty disco#items result)
jabber.developer2 added the
Kind/Bug
Status
Need More Info
2
 labels 2026-02-11 12:21:36 +00:00
jabber.developer added this to the Plans (2025-2026) project 2026-04-25 18:10:56 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Compat/Breaking
Breaking change that won't be backward compatible
Kind/Bug
Something is not working
Kind/Documentation
Documentation changes
Kind/Enhancement
Improve existing functionality
Kind/Feature
New functionality
Kind/Security
This is security issue
Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No Label
Kind/Bug
Status
Need More Info
2
Milestone
No items
No Milestone
Projects
Clear projects
No project Plans (2025-2026)
Assignees
Clear assignees
admin jabber.developer jabber.developer2
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: devs/cproof#91
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?