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

Client setinfo ignore all exception (#3449) #3458

Merged
merged 3 commits into from
Jun 6, 2023

Conversation

sazzad16
Copy link
Collaborator

@sazzad16 sazzad16 commented Jun 5, 2023

This PR contains the following changes:

  1. Check the character legality of clientName in advance through validateClientInfo, so that character exceptions are ignored in initializeFromClientConfig.
  2. Move the select command out of the fire-and-forget message (we care about its return value).
  3. For the test of RedisCredentials, we gave it "invalidPass" for the first time to make it fail the verification instead of skipping the verification.

This PR contains the following changes:

1. Check the character legality of clientName in advance through `validateClientInfo`, so that character exceptions are ignored in `initializeFromClientConfig`.
2. Move the `select` command out of the fire-and-forget message (we care about its return value).
3. For the test of RedisCredentials, we gave it "invalidPass" for the first time to make it fail the verification instead of skipping the verification.
@sazzad16 sazzad16 added this to the 4.4.x milestone Jun 5, 2023
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 73.33% and project coverage change: +0.01 🎉

Comparison is base (ec91bfa) 67.24% compared to head (73baf2d) 67.25%.

❗ Current head 73baf2d differs from pull request most recent head 5b4ecf3. Consider uploading reports for the commit 5b4ecf3 to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@             Coverage Diff              @@
##                4.x    #3458      +/-   ##
============================================
+ Coverage     67.24%   67.25%   +0.01%     
- Complexity     4734     4738       +4     
============================================
  Files           269      269              
  Lines         15308    15304       -4     
  Branches        966      962       -4     
============================================
- Hits          10294    10293       -1     
+ Misses         4591     4590       -1     
+ Partials        423      421       -2     
Impacted Files Coverage Δ
src/main/java/redis/clients/jedis/Connection.java 82.35% <73.33%> (+0.13%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sazzad16 sazzad16 added the bug label Jun 6, 2023
@sazzad16 sazzad16 merged commit 40b098e into redis:4.x Jun 6, 2023
@sazzad16 sazzad16 deleted the client-setinfo-ignore-4x branch June 6, 2023 05:11
@sazzad16 sazzad16 modified the milestones: 4.4.x, 4.4.2 Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants