Skip to content

Conversation

@yangbodong22011
Copy link
Contributor

@yangbodong22011 yangbodong22011 commented Jun 1, 2023

This PR contains the following changes (fixes #3438):

  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.

@yangbodong22011 yangbodong22011 force-pushed the fix-client-report-name branch 2 times, most recently from d5dbf87 to 160cb28 Compare June 1, 2023 04:35
@codecov-commenter
Copy link

codecov-commenter commented Jun 1, 2023

Codecov Report

Patch coverage: 78.37% and project coverage change: -0.05 ⚠️

Comparison is base (6822e33) 71.27% compared to head (6d584be) 71.23%.

❗ Current head 6d584be differs from pull request most recent head e6910e5. Consider uploading reports for the commit e6910e5 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              @@
##             master    #3449      +/-   ##
============================================
- Coverage     71.27%   71.23%   -0.05%     
- Complexity     4736     4740       +4     
============================================
  Files           274      275       +1     
  Lines         14697    14738      +41     
  Branches        991      989       -2     
============================================
+ Hits          10476    10499      +23     
- Misses         3789     3808      +19     
+ Partials        432      431       -1     
Impacted Files Coverage Δ
...rc/main/java/redis/clients/jedis/PipelineBase.java 26.93% <0.00%> (-0.10%) ⬇️
...main/java/redis/clients/jedis/TransactionBase.java 12.43% <0.00%> (-0.05%) ⬇️
...lients/jedis/commands/SortedSetBinaryCommands.java 100.00% <ø> (ø)
...edis/clients/jedis/commands/SortedSetCommands.java 100.00% <ø> (ø)
...edis/commands/SortedSetPipelineBinaryCommands.java 0.00% <ø> (ø)
...ents/jedis/commands/SortedSetPipelineCommands.java 0.00% <ø> (ø)
...a/redis/clients/jedis/commands/StreamCommands.java 100.00% <ø> (ø)
...clients/jedis/commands/StreamPipelineCommands.java 0.00% <ø> (ø)
.../main/java/redis/clients/jedis/CommandObjects.java 85.76% <60.00%> (-0.12%) ⬇️
...rc/main/java/redis/clients/jedis/UnifiedJedis.java 74.37% <66.66%> (-0.22%) ⬇️
... and 5 more

... and 3 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.

@yangbodong22011 yangbodong22011 force-pushed the fix-client-report-name branch 3 times, most recently from 2929574 to 3895341 Compare June 2, 2023 05:50
@sazzad16 sazzad16 added the bug label Jun 2, 2023
@yangbodong22011 yangbodong22011 force-pushed the fix-client-report-name branch from 3895341 to b4c9df4 Compare June 3, 2023 15:01
Check the character legality of clientName in advance through validateClientInfo,
so that character exceptions are ignored in initializeFromClientConfig. The remaining
errors, NOAUTH, UNKNOWN command, SYNTAX error, NOPREM should be ignored.
@yangbodong22011 yangbodong22011 force-pushed the fix-client-report-name branch from b4c9df4 to e6910e5 Compare June 5, 2023 01:50
@sazzad16 sazzad16 added this to the 5.0.0 milestone Jun 5, 2023
@sazzad16 sazzad16 merged commit 5c299e2 into redis:master Jun 5, 2023
sazzad16 pushed a commit to sazzad16/jedis that referenced this pull request 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.
sazzad16 added a commit that referenced this pull request Jun 6, 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.

---------

Co-authored-by: bodong.ybd <[email protected]>
sazzad16 added a commit that referenced this pull request Jun 6, 2023
@sazzad16 sazzad16 modified the milestones: 5.0.0, 4.4.x Oct 18, 2023
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.

Jedis 4.4.1 fails on CLIENT SETINFO for Redis 6.x if the user does not have all +CLIENT permissions

3 participants