Skip to content

[ISSUE #10216] Fix potential NPE in EscapeBridge when topicPublishInfo is null#10507

Merged
ShannonDing merged 1 commit into
apache:developfrom
SGloria:fix/escape-bridge-npe-10216
Jun 15, 2026
Merged

[ISSUE #10216] Fix potential NPE in EscapeBridge when topicPublishInfo is null#10507
ShannonDing merged 1 commit into
apache:developfrom
SGloria:fix/escape-bridge-npe-10216

Conversation

@SGloria

@SGloria SGloria commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix NullPointerException in EscapeBridge when tryToFindTopicPublishInfo() returns null.

Motivation

In EscapeBridge.java, two methods dereference the result of tryToFindTopicPublishInfo() without null checks:

  1. asyncPutMessage() — calls topicPublishInfo.selectOneMessageQueue() directly
  2. asyncRemotePutMessageToSpecificQueue() — calls topicPublishInfo.getMessageQueueList() directly

When a slave broker acting as master attempts to escape a message for a topic whose route information is not yet available (e.g., during startup or after a nameserver disconnection), this causes NPE.

In contrast, putMessageToRemoteBroker() in the same class already handles this correctly:

if (null == topicPublishInfo || !topicPublishInfo.ok()) {
    LOG.warn("putMessageToRemoteBroker: no route info ...");
    return null;
}

Changes

  • Added null/validity checks (null == topicPublishInfo || !topicPublishInfo.ok()) in both methods
  • Return PutMessageStatus.PUT_TO_REMOTE_BROKER_FAIL with a warning log, consistent with the existing pattern
  • No behavioral change when topicPublishInfo is valid

Test Plan

  • Code compiles without errors (mvn compile -pl broker -am)
  • Fix follows the exact same pattern as putMessageToRemoteBroker() in the same file
  • No new dependencies or API changes introduced

Fixes #10216

…ishInfo is null

In asyncPutMessage() and asyncRemotePutMessageToSpecificQueue(), the
return value of tryToFindTopicPublishInfo() is dereferenced without a
null check, causing NullPointerException when topic route information
is unavailable (e.g., during startup or after nameserver disconnection).

This commit adds null/validity checks consistent with the existing
pattern in putMessageToRemoteBroker(), returning PUT_TO_REMOTE_BROKER_FAIL
with a warning log instead of crashing with NPE.

Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.02%. Comparing base (343daa9) to head (d2b953b).
⚠️ Report is 60 commits behind head on develop.

Files with missing lines Patch % Lines
.../apache/rocketmq/broker/failover/EscapeBridge.java 75.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop   #10507      +/-   ##
=============================================
- Coverage      49.03%   48.02%   -1.02%     
+ Complexity     13471    13322     -149     
=============================================
  Files           1375     1377       +2     
  Lines         100381   100715     +334     
  Branches       12965    13012      +47     
=============================================
- Hits           49223    48368     -855     
- Misses         45172    46395    +1223     
+ Partials        5986     5952      -34     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@RockteMQ-AI RockteMQ-AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review by github-manager-bot

Summary

Adds null checks for topicPublishInfo in EscapeBridge.asyncPutMessage() and asyncRemotePutMessageToSpecificQueue() to prevent NPE when topic route info is unavailable.

Findings

  • [Info] EscapeBridge.java:188-192 — The null check null == topicPublishInfo || !topicPublishInfo.ok() correctly handles both missing route info and invalid publish info. Returning PUT_TO_REMOTE_BROKER_FAIL with retryTopic=true allows the caller to retry, which is the right behavior.
  • [Info] EscapeBridge.java:258-262 — Same pattern applied consistently in the second method. Good.

Suggestions

  • The existing code at line ~260 already checks if (null == mqs || mqs.isEmpty()) after this new check. With the new guard ensuring topicPublishInfo.ok(), the mqs null check becomes partially redundant (since ok() implies messageQueueList is non-empty). Consider whether the existing check can be simplified, or add a comment explaining both guards serve different purposes.
  • Consider adding a unit test that mocks tryToFindTopicPublishInfo() returning null to verify the failover path.

Clean defensive fix. Fixes #10216. 👍


Automated review by github-manager-bot

@ShannonDing ShannonDing merged commit da58317 into apache:develop Jun 15, 2026
10 checks passed
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.

[Bug] Potential NPE in EscapeBridge when topicPublishInfo is null

5 participants