Skip to content

Commit

Permalink
Validation positions to prevent out of bounds panic
Browse files Browse the repository at this point in the history
  • Loading branch information
aswinkarthik committed Oct 8, 2019
1 parent b0ee231 commit 037f723
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 10 deletions.
59 changes: 59 additions & 0 deletions cmd/config.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package cmd

import (
"encoding/csv"
"fmt"
"github.com/spf13/afero"
"io"
"strings"

"github.com/aswinkarthik/csvdiff/pkg/digest"
Expand Down Expand Up @@ -94,9 +96,66 @@ func (c *Context) Validate(fs afero.Fs) error {
}
}

{
baseRecordCount, err := getColumnsCount(fs, c.BaseFilename)
if err != nil {
return err
}
deltaRecordCount, err := getColumnsCount(fs, c.DeltaFilename)
if err != nil {
return err
}

if baseRecordCount != deltaRecordCount {
return fmt.Errorf("base-file and delta-file columns count do not match")
}

comparator := func(element int) bool {
return element < baseRecordCount
}

if !assertAll(c.PrimaryKeyPositions, comparator) {
return fmt.Errorf("--primary-key positions are out of bounds")
}
if !assertAll(c.IncludeColumnPositions, comparator) {
return fmt.Errorf("--include positions are out of bounds")
}
if !assertAll(c.ValueColumnPositions, comparator) {
return fmt.Errorf("--columns positions are out of bounds")
}
}

return nil
}

func assertAll(elements []int, assertFn func(element int) bool) bool {
for _, el := range elements {
if !assertFn(el) {
return false
}
}
return true
}

func getColumnsCount(fs afero.Fs, filename string) (int, error) {
base, err := fs.Open(filename)
if err != nil {
return 0, err
}
defer base.Close()
csvReader := csv.NewReader(base)

record, err := csvReader.Read()
if err != nil {
if err == io.EOF {
return 0, fmt.Errorf("unable to process headers from csv file. EOF reached")
}
return 0, err
}

return len(record), nil
}

// BaseDigestConfig creates a digest.Context from cmd.Context
// that is needed to start the diff process
func (c *Context) BaseDigestConfig(fs afero.Fs) (digest.Config, error) {
Expand Down
83 changes: 74 additions & 9 deletions cmd/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,16 @@ func TestValueColumnPositions(t *testing.T) {

func TestConfigValidate(t *testing.T) {
validConfig := func(t *testing.T, fs afero.Fs) *cmd.Context {
_, err := fs.Create("/base.csv")
assert.NoError(t, err)
_, err = fs.Create("/delta.csv")
assert.NoError(t, err)
{
baseContent := []byte("id,name,age,desc")
err := afero.WriteFile(fs, "/base.csv", baseContent, os.ModePerm)
assert.NoError(t, err)
}
{
deltaContent := []byte("id,name,age,desc")
err := afero.WriteFile(fs, "/delta.csv", deltaContent, os.ModePerm)
assert.NoError(t, err)
}
return &cmd.Context{Format: "json", BaseFilename: "/base.csv", DeltaFilename: "/delta.csv"}
}

Expand Down Expand Up @@ -77,6 +83,7 @@ func TestConfigValidate(t *testing.T) {
assert.EqualError(t, err, "base-file /base.csv should be a file")

_, err = fs.Create("/valid-base.csv")
assert.NoError(t, err)
err = fs.Mkdir("/delta.csv", os.ModePerm)
assert.NoError(t, err)

Expand All @@ -87,15 +94,73 @@ func TestConfigValidate(t *testing.T) {

t.Run("should validate if both base and delta file exist", func(t *testing.T) {
fs := afero.NewMemMapFs()
_, err := fs.Create("/base.csv")
assert.NoError(t, err)
_, err = fs.Create("/delta.csv")
assert.NoError(t, err)
validConfig(t, fs)

config := &cmd.Context{Format: "json", BaseFilename: "/base.csv", DeltaFilename: "/delta.csv"}
err = config.Validate(fs)
err := config.Validate(fs)
assert.NoError(t, err)
})

t.Run("should validate if positions are within the limits of the csv file", func(t *testing.T) {
fs := afero.NewMemMapFs()
{
baseContent := []byte("id,name,age,desc")
err := afero.WriteFile(fs, "/base.csv", baseContent, os.ModePerm)
assert.NoError(t, err)
}
{
deltaContent := []byte("id,name,age,desc")
err := afero.WriteFile(fs, "/delta.csv", deltaContent, os.ModePerm)
assert.NoError(t, err)
}

t.Run("primary key positions", func(t *testing.T) {
ctx := &cmd.Context{
Format: "json",
BaseFilename: "/base.csv",
DeltaFilename: "/delta.csv",
PrimaryKeyPositions: digest.Positions{4},
}

assert.EqualError(t, ctx.Validate(fs), "--primary-key positions are out of bounds")
})

t.Run("include positions", func(t *testing.T) {
ctx := &cmd.Context{
Format: "json",
BaseFilename: "/base.csv",
DeltaFilename: "/delta.csv",
IncludeColumnPositions: digest.Positions{4},
}

assert.EqualError(t, ctx.Validate(fs), "--include positions are out of bounds")
})

t.Run("value positions", func(t *testing.T) {
ctx := &cmd.Context{
Format: "json",
BaseFilename: "/base.csv",
DeltaFilename: "/delta.csv",
ValueColumnPositions: digest.Positions{4},
}

assert.EqualError(t, ctx.Validate(fs), "--columns positions are out of bounds")
})

t.Run("inequal base and delta files", func(t *testing.T) {
deltaContent := []byte("id,name,age,desc,size")
err := afero.WriteFile(fs, "/delta.csv", deltaContent, os.ModePerm)
assert.NoError(t, err)

ctx := &cmd.Context{
Format: "json",
BaseFilename: "/base.csv",
DeltaFilename: "/delta.csv",
}

assert.EqualError(t, ctx.Validate(fs), "base-file and delta-file columns count do not match")
})
})
}

func TestConfig_DigestConfig(t *testing.T) {
Expand Down
1 change: 0 additions & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ func runContext(
return NewFormatter(outputStream, errorStream, ctx).Format(diff)
}


// Execute adds all child commands to the root command and sets flags appropriately.
// This is called by main.main(). It only needs to happen once to the rootCmd.
func Execute() {
Expand Down

0 comments on commit 037f723

Please sign in to comment.