fix fieldset.New bug when prefix slice has len < cap #2851
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Accidentally inspected
gqlgen
source code and found a bug inFieldSet.New
implementation which can be reproduced ifprefix
slice is "not full" (length < capacity
). In this case internal functionparseUnnestedKeyFieldSet
will overwrite same slice element multiple times - which will lead to incorrectFieldSet
construction.Bug fix is pretty simple: we should set max capacity equal to length in
parseUnnestedKeyFieldSet
(using extended slicing operationx[start:end:cap]
)In order to cover this bug 1 test case were added:
TestWithPrefix/prefix with len<capacity
From the brief source code search it looks like no code were exposed to this bug because prefix argument always passed as
nil
. But, as this is public library interface, someone can use this logic.If it's possible - than removal of this argument can be considered (with potential breaking changes).
I have: