Skip to content

[ISSUE #9439] Add escape for win in the method returning broker configuration #9440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 5, 2025

Conversation

Crazylychee
Copy link
Contributor

@Crazylychee Crazylychee commented May 30, 2025

Which Issue(s) This PR Fixes

Fixes #9439

Brief Description

utilize MixAll.isWindows for platform-specific escaping

Affected files:

  • MixAll.java: utilize MixAll.isWindows for platform-specific escaping

How Did You Test This Change?

image
The above is before the modification, you can see the bug. The following is after the modification, you can see the program returns normally.
image

Copy link
Contributor

@RongtongJin RongtongJin left a comment

Choose a reason for hiding this comment

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

It is recommended to perform escaping on the client side (at the admin tools code), and handle it according to the platform type (can be done using MixAll.isWindows).

@Crazylychee
Copy link
Contributor Author

It is recommended to perform escaping on the client side (at the admin tools code), and handle it according to the platform type (can be done using MixAll.isWindows).

Thanks for the feedback! I've implemented the client-side escaping in the admin tools code as suggested. The logic now uses MixAll.isWindows to handle the escaping differently based on the platform type.

@Crazylychee Crazylychee changed the title [ISSUE #9439] Add backslash escaping to MixAll properties2String [ISSUE #9439] utilize MixAll.isWindows for platform-specific escaping Jun 1, 2025
@Crazylychee Crazylychee requested a review from RongtongJin June 2, 2025 05:42
@codecov-commenter
Copy link

codecov-commenter commented Jun 3, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 48.04%. Comparing base (577371e) to head (2648c70).
Report is 3 commits behind head on develop.

Files with missing lines Patch % Lines
...c/main/java/org/apache/rocketmq/common/MixAll.java 50.00% 1 Missing and 1 partial ⚠️
...e/rocketmq/container/BrokerContainerProcessor.java 0.00% 1 Missing ⚠️
...ntroller/processor/ControllerRequestProcessor.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #9440      +/-   ##
=============================================
+ Coverage      48.00%   48.04%   +0.03%     
- Complexity     11924    11984      +60     
=============================================
  Files           1307     1308       +1     
  Lines          92021    92174     +153     
  Branches       11775    11790      +15     
=============================================
+ Hits           44177    44287     +110     
- Misses         42368    42386      +18     
- Partials        5476     5501      +25     

☔ View full report in Codecov by Sentry.
📢 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.

@Crazylychee Crazylychee changed the title [ISSUE #9439] utilize MixAll.isWindows for platform-specific escaping [ISSUE #9439] Add escape for win in the method returning broker configuration Jun 3, 2025
@Crazylychee Crazylychee force-pushed the develop branch 2 times, most recently from 934f67f to 9c0c436 Compare June 4, 2025 06:34
Copy link
Contributor

@GenerousMan GenerousMan left a comment

Choose a reason for hiding this comment

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

LGTM

@RongtongJin RongtongJin merged commit 4f9d292 into apache:develop Jun 5, 2025
10 of 11 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] getBrokerConfig return null and log displayed "Malformed \uxxxx encoding."
5 participants