fix(security): fixed the possible overflow related issue#167
Merged
SantiagoDePolonia merged 1 commit intomainfrom Mar 23, 2026
Merged
fix(security): fixed the possible overflow related issue#167SantiagoDePolonia merged 1 commit intomainfrom
SantiagoDePolonia merged 1 commit intomainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughRefactored the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/streaming/observed_sse_stream_test.go`:
- Around line 293-337: Add more edge-case table-driven tests for joinedSuffix to
cover negative n, nil/empty prefix and data, n larger than sum of prefix+data,
and cases where prefix length equals n (no data used) — e.g., add test rows
invoking joinedSuffix(prefix=nil, data=nil, n<0), joinedSuffix(prefix=nil,
data="abc", n=2), joinedSuffix(prefix="ab", data=nil, n=3),
joinedSuffix(prefix="xyz", data="123", n=6) and joinedSuffix(prefix="xyz",
data="123", n=3) to ensure correct behavior; update the TestJoinedSuffix cases
to include these inputs and expected outputs so joinedSuffix is validated
against boundary conditions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: da0f7dda-e54c-4cda-8c14-86ae01892d10
📒 Files selected for processing (2)
internal/streaming/observed_sse_stream.gointernal/streaming/observed_sse_stream_test.go
Comment on lines
+293
to
+337
| func TestJoinedSuffix(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| prefix []byte | ||
| data []byte | ||
| n int | ||
| want []byte | ||
| }{ | ||
| { | ||
| name: "returns nil for non-positive length", | ||
| data: []byte("abc"), | ||
| n: 0, | ||
| want: nil, | ||
| }, | ||
| { | ||
| name: "returns suffix from data when data is long enough", | ||
| data: []byte("abcdef"), | ||
| n: 3, | ||
| want: []byte("def"), | ||
| }, | ||
| { | ||
| name: "combines prefix tail and data", | ||
| prefix: []byte("abcd"), | ||
| data: []byte("ef"), | ||
| n: 4, | ||
| want: []byte("cdef"), | ||
| }, | ||
| { | ||
| name: "uses available prefix bytes only", | ||
| prefix: []byte("ab"), | ||
| data: []byte("cd"), | ||
| n: 5, | ||
| want: []byte("abcd"), | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| got := joinedSuffix(tt.prefix, tt.data, tt.n) | ||
| if !bytes.Equal(got, tt.want) { | ||
| t.Fatalf("joinedSuffix() = %q, want %q", got, tt.want) | ||
| } | ||
| }) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Good test coverage for the core scenarios.
Consider adding edge cases for more thorough coverage:
💡 Suggested additional test cases
{
name: "returns nil for non-positive length",
data: []byte("abc"),
n: 0,
want: nil,
},
+ {
+ name: "returns nil for negative length",
+ data: []byte("abc"),
+ n: -1,
+ want: nil,
+ },
+ {
+ name: "handles nil prefix",
+ prefix: nil,
+ data: []byte("abc"),
+ n: 2,
+ want: []byte("bc"),
+ },
+ {
+ name: "handles empty data with prefix",
+ prefix: []byte("abcd"),
+ data: []byte{},
+ n: 2,
+ want: []byte("cd"),
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func TestJoinedSuffix(t *testing.T) { | |
| tests := []struct { | |
| name string | |
| prefix []byte | |
| data []byte | |
| n int | |
| want []byte | |
| }{ | |
| { | |
| name: "returns nil for non-positive length", | |
| data: []byte("abc"), | |
| n: 0, | |
| want: nil, | |
| }, | |
| { | |
| name: "returns suffix from data when data is long enough", | |
| data: []byte("abcdef"), | |
| n: 3, | |
| want: []byte("def"), | |
| }, | |
| { | |
| name: "combines prefix tail and data", | |
| prefix: []byte("abcd"), | |
| data: []byte("ef"), | |
| n: 4, | |
| want: []byte("cdef"), | |
| }, | |
| { | |
| name: "uses available prefix bytes only", | |
| prefix: []byte("ab"), | |
| data: []byte("cd"), | |
| n: 5, | |
| want: []byte("abcd"), | |
| }, | |
| } | |
| for _, tt := range tests { | |
| t.Run(tt.name, func(t *testing.T) { | |
| got := joinedSuffix(tt.prefix, tt.data, tt.n) | |
| if !bytes.Equal(got, tt.want) { | |
| t.Fatalf("joinedSuffix() = %q, want %q", got, tt.want) | |
| } | |
| }) | |
| } | |
| } | |
| func TestJoinedSuffix(t *testing.T) { | |
| tests := []struct { | |
| name string | |
| prefix []byte | |
| data []byte | |
| n int | |
| want []byte | |
| }{ | |
| { | |
| name: "returns nil for non-positive length", | |
| data: []byte("abc"), | |
| n: 0, | |
| want: nil, | |
| }, | |
| { | |
| name: "returns nil for negative length", | |
| data: []byte("abc"), | |
| n: -1, | |
| want: nil, | |
| }, | |
| { | |
| name: "handles nil prefix", | |
| prefix: nil, | |
| data: []byte("abc"), | |
| n: 2, | |
| want: []byte("bc"), | |
| }, | |
| { | |
| name: "handles empty data with prefix", | |
| prefix: []byte("abcd"), | |
| data: []byte{}, | |
| n: 2, | |
| want: []byte("cd"), | |
| }, | |
| { | |
| name: "returns suffix from data when data is long enough", | |
| data: []byte("abcdef"), | |
| n: 3, | |
| want: []byte("def"), | |
| }, | |
| { | |
| name: "combines prefix tail and data", | |
| prefix: []byte("abcd"), | |
| data: []byte("ef"), | |
| n: 4, | |
| want: []byte("cdef"), | |
| }, | |
| { | |
| name: "uses available prefix bytes only", | |
| prefix: []byte("ab"), | |
| data: []byte("cd"), | |
| n: 5, | |
| want: []byte("abcd"), | |
| }, | |
| } | |
| for _, tt := range tests { | |
| t.Run(tt.name, func(t *testing.T) { | |
| got := joinedSuffix(tt.prefix, tt.data, tt.n) | |
| if !bytes.Equal(got, tt.want) { | |
| t.Fatalf("joinedSuffix() = %q, want %q", got, tt.want) | |
| } | |
| }) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/streaming/observed_sse_stream_test.go` around lines 293 - 337, Add
more edge-case table-driven tests for joinedSuffix to cover negative n,
nil/empty prefix and data, n larger than sum of prefix+data, and cases where
prefix length equals n (no data used) — e.g., add test rows invoking
joinedSuffix(prefix=nil, data=nil, n<0), joinedSuffix(prefix=nil, data="abc",
n=2), joinedSuffix(prefix="ab", data=nil, n=3), joinedSuffix(prefix="xyz",
data="123", n=6) and joinedSuffix(prefix="xyz", data="123", n=3) to ensure
correct behavior; update the TestJoinedSuffix cases to include these inputs and
expected outputs so joinedSuffix is validated against boundary conditions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit
Refactor
Tests