Skip to content

Commit

Permalink
Fix incorrect conversion between integer types (#3688)
Browse files Browse the repository at this point in the history
Signed-off-by: ZhangXiaozheng <[email protected]>
  • Loading branch information
zhang-x-z authored Jan 12, 2024
1 parent 453b086 commit 5206a2b
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 13 deletions.
15 changes: 10 additions & 5 deletions pkg/ddc/alluxio/operations/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ func (a AlluxioFileUtils) Count(alluxioPath string) (fileCount int64, folderCoun
command = []string{"alluxio", "fs", "count", alluxioPath}
stdout string
stderr string
ufileCount, ufolderCount, utotal uint64
ufileCount, ufolderCount, utotal int64
)

stdout, stderr, err = a.execWithoutTimeout(command, false)
Expand All @@ -412,22 +412,27 @@ func (a AlluxioFileUtils) Count(alluxioPath string) (fileCount int64, folderCoun
return
}

ufileCount, err = strconv.ParseUint(data[0], 10, 64)
ufileCount, err = strconv.ParseInt(data[0], 10, 64)
if err != nil {
return
}

ufolderCount, err = strconv.ParseUint(data[1], 10, 64)
ufolderCount, err = strconv.ParseInt(data[1], 10, 64)
if err != nil {
return
}

utotal, err = strconv.ParseUint(data[2], 10, 64)
utotal, err = strconv.ParseInt(data[2], 10, 64)
if err != nil {
return
}

return int64(ufileCount), int64(ufolderCount), int64(utotal), err
if ufileCount < 0 || ufolderCount < 0 || utotal < 0 {
err = fmt.Errorf("the return value of Count method is negative")
return
}

return ufileCount, ufolderCount, utotal, err
}

// file count of the Alluxio Filesystem (except folder)
Expand Down
4 changes: 4 additions & 0 deletions pkg/ddc/alluxio/operations/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const (
OTHER_ERR = "other-err"
FINE = "fine"
EXEC_ERR = "exec-err"
NEGATIVE_RES = "negative-res"
TOO_MANY_LINES = "too many lines"
DATA_NUM = "data nums not match"
PARSE_ERR = "parse err"
Expand Down Expand Up @@ -626,6 +627,8 @@ func TestAlluxioFileUtils_Count(t *testing.T) {

if strings.Contains(p4[3], EXEC_ERR) {
return "does not exist", "", errors.New("exec-error")
} else if strings.Contains(p4[3], NEGATIVE_RES) {
return "12324\t45463\t-9223372036854775808", "", nil
} else if strings.Contains(p4[3], TOO_MANY_LINES) {
return "1\n2\n3\n4\n", "1\n2\n3\n4\n", nil
} else if strings.Contains(p4[3], DATA_NUM) {
Expand Down Expand Up @@ -654,6 +657,7 @@ func TestAlluxioFileUtils_Count(t *testing.T) {
noErr bool
}{
{EXEC_ERR, 0, 0, 0, false},
{NEGATIVE_RES, 0, 0, 0, false},
{TOO_MANY_LINES, 0, 0, 0, false},
{DATA_NUM, 0, 0, 0, false},
{PARSE_ERR, 0, 0, 0, false},
Expand Down
15 changes: 10 additions & 5 deletions pkg/ddc/goosefs/operations/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ func (a GooseFSFileUtils) Count(goosefsPath string) (fileCount int64, folderCoun
command = []string{"goosefs", "fs", "count", goosefsPath}
stdout string
stderr string
ufileCount, ufolderCount, utotal uint64
ufileCount, ufolderCount, utotal int64
)

stdout, stderr, err = a.execWithoutTimeout(command, false)
Expand All @@ -379,22 +379,27 @@ func (a GooseFSFileUtils) Count(goosefsPath string) (fileCount int64, folderCoun
return
}

ufileCount, err = strconv.ParseUint(data[0], 10, 64)
ufileCount, err = strconv.ParseInt(data[0], 10, 64)
if err != nil {
return
}

ufolderCount, err = strconv.ParseUint(data[1], 10, 64)
ufolderCount, err = strconv.ParseInt(data[1], 10, 64)
if err != nil {
return
}

utotal, err = strconv.ParseUint(data[2], 10, 64)
utotal, err = strconv.ParseInt(data[2], 10, 64)
if err != nil {
return
}

return int64(ufileCount), int64(ufolderCount), int64(utotal), err
if ufileCount < 0 || ufolderCount < 0 || utotal < 0 {
err = fmt.Errorf("the return value of Count method is negative")
return
}

return ufileCount, ufolderCount, utotal, err
}

// file count of the GooseFS Filesystem (except folder)
Expand Down
4 changes: 4 additions & 0 deletions pkg/ddc/goosefs/operations/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const (
OTHER_ERR = "other-err"
FINE = "fine"
EXEC_ERR = "exec-err"
NEGATIVE_RES = "negative-res"
TOO_MANY_LINES = "too many lines"
DATA_NUM = "data nums not match"
PARSE_ERR = "parse err"
Expand Down Expand Up @@ -553,6 +554,8 @@ func TestCount(t *testing.T) {

if strings.Contains(p4[3], EXEC_ERR) {
return "does not exist", "", errors.New("exec-error")
} else if strings.Contains(p4[3], NEGATIVE_RES) {
return "12324\t45463\t-9223372036854775808", "", nil
} else if strings.Contains(p4[3], TOO_MANY_LINES) {
return "1\n2\n3\n4\n", "1\n2\n3\n4\n", nil
} else if strings.Contains(p4[3], DATA_NUM) {
Expand Down Expand Up @@ -581,6 +584,7 @@ func TestCount(t *testing.T) {
noErr bool
}{
{EXEC_ERR, 0, 0, 0, false},
{NEGATIVE_RES, 0, 0, 0, false},
{TOO_MANY_LINES, 0, 0, 0, false},
{DATA_NUM, 0, 0, 0, false},
{PARSE_ERR, 0, 0, 0, false},
Expand Down
10 changes: 7 additions & 3 deletions pkg/ddc/juicefs/operations/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (j JuiceFileUtils) Count(juiceSubPath string) (total int64, err error) {
command = []string{"du", "-sb", juiceSubPath}
stdout string
stderr string
utotal uint64
utotal int64
)

stdout, stderr, err = j.exec(command)
Expand All @@ -117,12 +117,16 @@ func (j JuiceFileUtils) Count(juiceSubPath string) (total int64, err error) {
return
}

utotal, err = strconv.ParseUint(data[0], 10, 64)
utotal, err = strconv.ParseInt(data[0], 10, 64)
if err != nil {
return
}
if utotal < 0 {
err = fmt.Errorf("the return value of Count method is negative")
return
}

return int64(utotal), err
return utotal, err
}

// file count of the JuiceFS Filesystem (except folder)
Expand Down
13 changes: 13 additions & 0 deletions pkg/ddc/juicefs/operations/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,9 @@ func TestJuiceFileUtils_Count(t *testing.T) {
ExecWithoutTimeoutErr := func(a JuiceFileUtils, command []string) (stdout string, stderr string, err error) {
return "", "", errors.New("fail to run the command")
}
ExecWithoutTimeoutNegative := func(a JuiceFileUtils, command []string) (stdout string, stderr string, err error) {
return "-9223372036854775808 /tmp", "", nil
}
wrappedUnhookExec := func() {
err := gohook.UnHook(JuiceFileUtils.execWithoutTimeout)
if err != nil {
Expand All @@ -407,6 +410,16 @@ func TestJuiceFileUtils_Count(t *testing.T) {
}
wrappedUnhookExec()

err = gohook.Hook(JuiceFileUtils.execWithoutTimeout, ExecWithoutTimeoutNegative, nil)
if err != nil {
t.Fatal(err.Error())
}
_, err = a.Count("/tmp")
if err == nil {
t.Error("check failure, want err, got nil")
}
wrappedUnhookExec()

err = gohook.Hook(JuiceFileUtils.execWithoutTimeout, ExecWithoutTimeoutCommon, nil)
if err != nil {
t.Fatal(err.Error())
Expand Down

0 comments on commit 5206a2b

Please sign in to comment.