Skip to content

fix: panic in extractKey #25

Merged
slayerjain merged 3 commits intomainfrom
fix/panic
Aug 26, 2025
Merged

fix: panic in extractKey #25
slayerjain merged 3 commits intomainfrom
fix/panic

Conversation

@officialasishkumar
Copy link
Copy Markdown
Member

@officialasishkumar officialasishkumar commented Aug 26, 2025

Fixes a panic in the extractKey function by adding proper bounds checking and improving key extraction logic. The fix addresses the issue where accessing string indices without length validation could cause out-of-bounds panics.

  • Adds bounds checking before accessing string indices to prevent panics
  • Refactors key extraction logic to be more robust with edge cases
  • Introduces a helper function unquoteKey for consistent key cleaning

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Copilot AI review requested due to automatic review settings August 26, 2025 08:13
@keploy
Copy link
Copy Markdown

keploy Bot commented Aug 26, 2025

To generate Unit Tests for this PR, please click here.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Fixes a panic in the extractKey function by adding proper bounds checking and improving key extraction logic. The fix addresses the issue where accessing string indices without length validation could cause out-of-bounds panics.

  • Adds bounds checking before accessing string indices to prevent panics
  • Refactors key extraction logic to be more robust with edge cases
  • Introduces a helper function unquoteKey for consistent key cleaning

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread jsondiff.go
}

// Remove a single leading +/- if present.
if line[0] == '-' || line[0] == '+' {
Copy link

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line can still cause a panic if line is an empty string after trimming. The empty string check should be performed before accessing line[0].

Suggested change
if line[0] == '-' || line[0] == '+' {
if len(line) > 0 && (line[0] == '-' || line[0] == '+') {

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good to have but this will never happen since line 223 handles that

Comment thread jsondiff.go
Comment on lines +533 to +534
expectMap = map[string]interface{}{cleanExpectKey: jsonObj}

Copy link

Copilot AI Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] There's an unnecessary empty line that breaks the consistency of the switch statement formatting.

Suggested change
expectMap = map[string]interface{}{cleanExpectKey: jsonObj}

Copilot uses AI. Check for mistakes.
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
@slayerjain slayerjain merged commit 38e5b95 into main Aug 26, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants