Skip to content

Fix XEP-0124 BOSH compliance: overactivity exception, ack handling, sentElements cleanup#3192

Closed
Copilot wants to merge 2 commits intomainfrom
copilot/review-xep-0124-implementation
Closed

Fix XEP-0124 BOSH compliance: overactivity exception, ack handling, sentElements cleanup#3192
Copilot wants to merge 2 commits intomainfrom
copilot/review-xep-0124-implementation

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 13, 2026

Openfire's BOSH implementation had several deviations from XEP-0124 around request acknowledgements, overactivity enforcement, and retransmission buffer management.

Changes

Overactivity check allows pause/terminate exception (§11)

Per spec, clients MAY send one additional simultaneous request if it is a pause or terminate. Previously the check fired at pendingConnections >= maxRequests unconditionally, rejecting valid pause/terminate requests from clients already at the limit.

// Before
if (pendingConnections >= maxRequests) { overactivity = TOO_MANY_SIM_REQS; }

// After — pause/terminate gets +1 headroom
final boolean isPauseOrTerminate = connection.isTerminate() || connection.getPause() != null;
final int maxSimultaneousRequests = isPauseOrTerminate ? maxRequests + 1 : maxRequests;
if (pendingConnections >= maxSimultaneousRequests) { overactivity = TOO_MANY_SIM_REQS; }

Parse client ack attribute from request bodies (§9.2)

Added HttpBindBody.getAck() to expose the client's acknowledged RID. When present, acknowledged entries are now pruned from the sentElements retransmission buffer rather than relying solely on a size cap of maxRequests.

Omit redundant ack in responses (§9.1)

"the connection manager SHOULD NOT include an 'ack' attribute in any response if the value would be the 'rid' of the request being responded to"

asBodyText() and createEmptyBody() now accept the responding-to RID and suppress the ack attribute when it would be identical—the common steady-state case.

Tests

Added HttpBindBodyTest covering getAck() (present/absent/invalid/initial-value), getRid(), and isPoll() variants.


💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

…ing, sentElements cleanup

Co-authored-by: akrherz <210858+akrherz@users.noreply.github.com>
Copilot AI changed the title [WIP] [OF-1109] Review and fix HTTP-Bind/BoSH support for XEP-0124 Fix XEP-0124 BOSH compliance: overactivity exception, ack handling, sentElements cleanup Mar 13, 2026
Copilot AI requested a review from akrherz March 13, 2026 19:02
@akrherz akrherz marked this pull request as ready for review March 13, 2026 19:05
@guusdk
Copy link
Copy Markdown
Member

guusdk commented Mar 25, 2026

@copilot I have raised these new JIRA tickets for the fixes that you created:

Evaluate them to see if they correctly describe the issues that you fixed.

To replace this PR, create three new PRs, each addressing a single JIRA issue. Use the issue number in the title of each commit. Also include a reference to the corresponding JIRA issue (using a javadoc @see tag) to the new, individual unit tests where applicable.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 25, 2026

@guusdk I've opened a new pull request, #3224, to work on those changes. Once the pull request is ready, I'll request review from you.

@guusdk
Copy link
Copy Markdown
Member

guusdk commented Mar 25, 2026

@copilot that is not what I wanted. Lets try something else. Create a new PR that only holds three commits, which includes each of the changes discussed here.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 25, 2026

@guusdk I've opened a new pull request, #3228, to work on those changes. Once the pull request is ready, I'll request review from you.

@guusdk
Copy link
Copy Markdown
Member

guusdk commented Mar 25, 2026

Copilot didn't exactly do what I wanted. I've manually created the desired PR in #3229 - that PR replaces this PR.

@guusdk guusdk closed this Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants