Skip to content
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: potential panic in validate #1831

Merged
merged 1 commit into from
Jul 5, 2023
Merged

fix: potential panic in validate #1831

merged 1 commit into from
Jul 5, 2023

Conversation

markphelps
Copy link
Collaborator

@markphelps markphelps commented Jul 5, 2023

  • fixes case where finding pos of error could panic
  • moves cue/fixtures to cue/testdata
  • adds fuzz tests for cue.Validate
  • fixed some linter warnings about deprecated ioutil package

Before Fix

--- FAIL: FuzzValidate (97.59s)
    --- FAIL: FuzzValidate (0.00s)
        testing.go:1485: panic: runtime error: index out of range [-1]
            goroutine 29131 [running]:
            runtime/debug.Stack()
            	/opt/homebrew/Cellar/go/1.20.5/libexec/src/runtime/debug/stack.go:24 +0xbc
            testing.tRunner.func1()
            	/opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1485 +0x264
            panic({0x102b97700, 0x1400a1d8918})
            	/opt/homebrew/Cellar/go/1.20.5/libexec/src/runtime/panic.go:884 +0x204
            go.flipt.io/flipt/internal/cue.FeaturesValidator.Validate({0x1400a8024a0?, {0x1400a8024a0?, 0x1400a80f3b0?, 0x0?}}, {0x1029f30ca, 0x3}, {0x14009640050?, 0x0?, 0x4?})
            	/Users/markphelps/workspace/flipt/internal/cue/validate.go:76 +0x484
            go.flipt.io/flipt/internal/cue.FuzzValidate.func1(0x1400a7f8340, {0x14009640050, 0x6, 0x10})
            	/Users/markphelps/workspace/flipt/internal/cue/validate_fuzz_test.go:26 +0x10c
            reflect.Value.call({0x102b24a00?, 0x102bcad28?, 0x1400c914e38?}, {0x1029f334d, 0x4}, {0x1400a783080, 0x2, 0x0?})
            	/opt/homebrew/Cellar/go/1.20.5/libexec/src/reflect/value.go:586 +0x87c
            reflect.Value.Call({0x102b24a00?, 0x102bcad28?, 0x14000000000?}, {0x1400a783080?, 0x102bc9680?, 0x102e46ab0?})
            	/opt/homebrew/Cellar/go/1.20.5/libexec/src/reflect/value.go:370 +0x90
            testing.(*F).Fuzz.func1.1(0x1400c90a000?)
            	/opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/fuzz.go:335 +0x360
            testing.tRunner(0x1400a7f8340, 0x1400a80f290)
            	/opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1576 +0x10c
            created by testing.(*F).Fuzz.func1
            	/opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/fuzz.go:322 +0x4c4


    Failing input written to testdata/fuzz/FuzzValidate/9d39dbf6febda3de

@markphelps markphelps requested a review from a team as a code owner July 5, 2023 17:57
@markphelps markphelps enabled auto-merge (squash) July 5, 2023 18:02
@codecov
Copy link

codecov bot commented Jul 5, 2023

Codecov Report

Merging #1831 (f9f702d) into main (c846fc3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1831   +/-   ##
=======================================
  Coverage   71.63%   71.64%           
=======================================
  Files          58       58           
  Lines        5500     5501    +1     
=======================================
+ Hits         3940     3941    +1     
  Misses       1335     1335           
  Partials      225      225           
Impacted Files Coverage Δ
internal/cue/validate.go 84.61% <100.00%> (+0.40%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@yquansah yquansah left a comment

Choose a reason for hiding this comment

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

LGTM

@markphelps markphelps merged commit e2faa04 into main Jul 5, 2023
@markphelps markphelps deleted the mp/fix-validate-panic branch July 5, 2023 19:16
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.

2 participants