Skip to content

Fix deferred HTTP reload race - #1712

Merged
BenCodez merged 1 commit into
masterfrom
fix/deferred-http-reload-regression
Oct 14, 2026
Merged

BenCodez merged 1 commit into
masterfrom
fix/deferred-http-reload-regression

Conversation

@BenCodez

@BenCodez BenCodez commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner

A queued HTTP reconciliation could request a full runtime replacement when a cancelled transport change restored the unchanged live HTTP configuration before the manual reload entered its coordination path. Keep the existing listener and its pending delivery queue when no replacement is needed.

Adds a deterministic regression for that interleaving. Changed endpoints and genuine transport changes retain their existing drain/reconciliation behavior. No new configuration, commands, protocol changes, dependencies or database migrations.

Validation:

  • New regression fails on unchanged master and passes with the fix.
  • All 176 VotingPluginProxyTest cases passed, including the existing concurrent manual-reload case.
  • Full clean package: 2,435 passed, one existing optional skip, zero failures/errors (2,436 total); bounded JVM heaps, all tests enabled.
  • Packaging checks, ZIP integrity, required proxy class and absence of bundled test libraries verified. Regular JAR: 10,632,837 bytes; existing advisory 10 MiB size warning remains.
  • Fresh independent source review: No findings. Diff/whitespace and full-range disclosure checks passed.

BungeeCord and Velocity adapters compile in the normal build. No live proxy deployment verification is claimed. Hosted CI is pending; do not merge automatically.

Summary by CodeRabbit

  • Bug Fixes
    • Prevented unchanged HTTP listeners from being replaced during deferred reconciliation or a manual reload, avoiding unnecessary interruptions to ongoing activity.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-14T01:22:44.972793Z 11fc55c PR opened
🔒 Security Review ✅ Completed 2026-10-14T01:26:22.989001Z 11fc55c PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 671adc26-ee0d-4661-8b41-852ec4d5be90
📥 Commits

Reviewing files that changed from the base of the PR and between 4724d57 and 11fc55c.

📒 Files selected for processing: 2
  • VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/VotingPluginProxy.java
  • VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/VotingPluginProxyTest.java

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout: 3
  • GitHub Check: build
  • GitHub Check: Analyze (actions)
  • GitHub Check: Analyze (java-kotlin)
🔇 Additional comments: 2
VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/VotingPluginProxy.java: 1

4987-4991: LGTM!

VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/VotingPluginProxyTest.java: 1

2649-2679: LGTM!

📝 Walkthrough

The deferred HTTP reconciliation callback now preserves the runtime when pending deliveries have cleared and the existing listener matches the current HTTP configuration. A regression test covers restoring the HTTP configuration before the callback and then performing a manual reload.

Changes: HTTP reconciliation

Layer / File(s) Summary
Preserve unchanged HTTP runtime
VotingPlugin/src/main/java/com/bencodez/votingplugin/proxy/VotingPluginProxy.java, VotingPlugin/src/test/java/com/bencodez/votingplugin/tests/VotingPluginProxyTest.java
The callback skips runtime replacement when HTTP remains configured and the live listener matches its configuration. The regression test checks that the listener is not replaced or closed, polling is canceled, and a subsequent manual reload does not trigger a core reload.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 11fc5

No actionable merge-blocking issue is identified for the deferred HTTP reload fix. Normal checks and the pending hosted CI result remain appropriate before merging.

Security architecture risk: 🔵 Low · up to 11fc5

The fix preserves the existing listener and its pending deliveries during a cancelled transport change. A narrow compatibility risk remains if message-encryption settings or keys change during the same sequence: the preserved listener may retain its earlier policy. Existing TLS and backend-identity routing are not removed.

Retained concerns

  • Low · security · inferred: The new preservation path equates unchanged host, port and public endpoint with an unchanged HTTP runtime. If a cancelled transport switch also accompanies an encryption-policy or key update, it can leave the listener using its earlier immutable codec instead of attempting the full replacement performed by the base callback. This is a conditional security-policy consistency concern: configuration-change privileges are required, TLS remains in place, and the supported reload-versus-restart contract is unresolved.
Security review details

Security Blast Radius

  • inferred — The conditional stale-policy exposure concerns traffic handled by the retained proxy HTTP listener and its enrolled backends. Triggering the policy mismatch requires configuration or key modification, not merely an arbitrary inbound message. Anonymous-network exploitation was not established.

Security Findings and Attack Paths

  • inferred — If the preserved codec still permits unencrypted application envelopes or uses an earlier key after an administrative update, authenticated traffic can continue to be evaluated under that earlier policy. The newly skipped replacement can prolong this state in the cancellation interleaving. This is not evidence that HTTPS becomes plaintext or that backend authentication is bypassed.

Trust Boundaries and Controls

  • observed — Listener construction supplies TLS identity and enrollment authority. Received envelopes retain authenticated backend identity; routing checks the stamped server against that identity and applies additional player-presence authority checks. These controls are unchanged by the guard, although the external transport's enforcement internals were not inspected.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: fixing a race in deferred HTTP reload reconciliation.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@BenCodez
BenCodez merged commit 4cf01f5 into master Oct 14, 2026
6 checks passed
@BenCodez
BenCodez deleted the fix/deferred-http-reload-regression branch October 14, 2026 01:27
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.

1 participant