-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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: check store keys length before accessing #9639
Conversation
…khita/check-store-keys-length
Codecov Report
@@ Coverage Diff @@
## master #9639 +/- ##
===========================================
+ Coverage 35.48% 63.41% +27.93%
===========================================
Files 332 571 +239
Lines 32620 37542 +4922
===========================================
+ Hits 11575 23808 +12233
+ Misses 19819 11876 -7943
- Partials 1226 1858 +632
|
…khita/check-store-keys-length
…khita/check-store-keys-length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First couple of comments
…khita/check-store-keys-length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, all the numbers seem to add up! Just a couple of small nits, but pre-approving.
…khita/check-store-keys-length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…khita/check-store-keys-length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also checked that the lengths add up.
I didn't check whether or not we added kv.Assert*
everywhere where key slices are accessed.
…khita/check-store-keys-length
|
…khita/check-store-keys-length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
last small nit
With Ru's approval, we got 2, so I'm merging. Thanks @likhita-809 ! |
* check store keys length before accessing * check store keys length in all modules * add changelog * refactoring * address review comments * remove commented lines * small fixes * address review comments * fix failing tests * add godocs
Description
Closes: #8451
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change