Skip to content
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

fix RequestCode overflowed issues:8868 #8871

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

zzjcool
Copy link
Contributor

@zzjcool zzjcool commented Oct 29, 2024

Which Issue(s) This PR Fixes

Fixes #8868

Brief Description

Description:
The following message request codes exceed the maximum value for a short integer (32767), resulting in an overflow. The original values are as follows:

public static final int POP_MESSAGE = 200050;
public static final int ACK_MESSAGE = 200051;
public static final int BATCH_ACK_MESSAGE = 200151;
public static final int PEEK_MESSAGE = 200052;
public static final int CHANGE_MESSAGE_INVISIBLETIME = 200053;
public static final int NOTIFICATION = 200054;
public static final int POLLING_INFO = 200055;

Actual Values After Overflow:
Due to the overflow, the actual values of these constants are:

public static final int POP_MESSAGE = 3442;
public static final int ACK_MESSAGE = 3443;
public static final int BATCH_ACK_MESSAGE = 3543;
public static final int PEEK_MESSAGE = 3444;
public static final int CHANGE_MESSAGE_INVISIBLETIME = 3445;
public static final int NOTIFICATION = 3446;
public static final int POLLING_INFO = 3447;

How Did You Test This Change?

unit test

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.15%. Comparing base (ecb45bb) to head (f8d7bc6).

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #8871      +/-   ##
=============================================
- Coverage      47.19%   47.15%   -0.04%     
+ Complexity     11676    11668       -8     
=============================================
  Files           1304     1304              
  Lines          90984    90984              
  Branches       11668    11668              
=============================================
- Hits           42936    42906      -30     
- Misses         42765    42789      +24     
- Partials        5283     5289       +6     

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

@universefeeler
Copy link
Contributor

universefeeler commented Oct 29, 2024 via email

@lizhanhui lizhanhui merged commit 327abe5 into apache:develop Oct 29, 2024
11 checks passed
@yuz10
Copy link
Member

yuz10 commented Oct 29, 2024

I think the change breaks compatibility, In fact, the request code is int when use serializeTypeCurrentRPC=json. which is default. so old clients will fail to connect new server, and new clients fail to connet old server. the fix is included in #5909

@lizhanhui
Copy link
Contributor

I think the change breaks compatibility, In fact, the request code is int when use serializeTypeCurrentRPC=json. which is default. so old clients will fail to connect new server, and new clients fail to connet old server. the fix is included in #5909

Sounds reasonable, let me double check.

@lizhanhui
Copy link
Contributor

There is no easy way to fix the overflow issue. We need to revert this change @zzjcool

lizhanhui added a commit that referenced this pull request Oct 30, 2024
yuz10 pushed a commit that referenced this pull request Oct 30, 2024
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] Request Code overflow
5 participants