diff --git a/src/jobservice/core/controller.go b/src/jobservice/core/controller.go index ea018e53a..bbc2f1220 100644 --- a/src/jobservice/core/controller.go +++ b/src/jobservice/core/controller.go @@ -16,17 +16,17 @@ package core import ( "fmt" - "github.com/goharbor/harbor/src/jobservice/mgt" - "github.com/pkg/errors" - "github.com/goharbor/harbor/src/jobservice/logger" + "github.com/pkg/errors" + "github.com/robfig/cron" "github.com/goharbor/harbor/src/jobservice/common/query" "github.com/goharbor/harbor/src/jobservice/common/utils" "github.com/goharbor/harbor/src/jobservice/errs" "github.com/goharbor/harbor/src/jobservice/job" + "github.com/goharbor/harbor/src/jobservice/logger" + "github.com/goharbor/harbor/src/jobservice/mgt" "github.com/goharbor/harbor/src/jobservice/worker" - "github.com/robfig/cron" ) // basicController implement the core interface and provides related job handle methods. diff --git a/src/jobservice/logger/base_test.go b/src/jobservice/logger/base_test.go index 7ce373dba..45c638d58 100644 --- a/src/jobservice/logger/base_test.go +++ b/src/jobservice/logger/base_test.go @@ -12,6 +12,13 @@ import ( "github.com/goharbor/harbor/src/jobservice/logger/backend" ) +const ( + fakeLogFile = "f00000000000000000000000.log" + fakeLogID = "f00000000000000000000000" + fakeJobID = "f00000000000000000000001" + fakeJobID2 = "f00000000000000000000002" +) + // Test one single std logger func TestGetLoggerSingleStd(t *testing.T) { l, err := GetLogger(BackendOption("STD_OUTPUT", "DEBUG", nil)) @@ -48,7 +55,7 @@ func TestGetLoggerSingleFile(t *testing.T) { lSettings := map[string]interface{}{} lSettings["base_dir"] = os.TempDir() - lSettings["filename"] = fmt.Sprintf("%s.log", "fake_job_ID") + lSettings["filename"] = fmt.Sprintf("%s.log", fakeJobID) defer func() { if err := os.Remove(path.Join(os.TempDir(), lSettings["filename"].(string))); err != nil { t.Error(err) @@ -67,7 +74,7 @@ func TestGetLoggerSingleFile(t *testing.T) { func TestGetLoggersMulti(t *testing.T) { lSettings := map[string]interface{}{} lSettings["base_dir"] = os.TempDir() - lSettings["filename"] = fmt.Sprintf("%s.log", "fake_job_ID2") + lSettings["filename"] = fmt.Sprintf("%s.log", fakeJobID2) defer func() { if err := os.Remove(path.Join(os.TempDir(), lSettings["filename"].(string))); err != nil { t.Error(err) @@ -142,7 +149,7 @@ func TestGetGetter(t *testing.T) { t.Fatal(err) } - logFile := path.Join(os.TempDir(), "fake_log_file.log") + logFile := path.Join(os.TempDir(), fakeLogFile) if err := ioutil.WriteFile(logFile, []byte("hello log getter"), 0644); err != nil { t.Fatal(err) } @@ -152,7 +159,7 @@ func TestGetGetter(t *testing.T) { } }() - data, err := g.Retrieve("fake_log_file") + data, err := g.Retrieve(fakeLogID) if err != nil { t.Error(err) } diff --git a/src/jobservice/logger/getter/file_getter.go b/src/jobservice/logger/getter/file_getter.go index cc7faffad..3838964b4 100644 --- a/src/jobservice/logger/getter/file_getter.go +++ b/src/jobservice/logger/getter/file_getter.go @@ -1,6 +1,7 @@ package getter import ( + "encoding/hex" "errors" "fmt" "io/ioutil" @@ -23,8 +24,12 @@ func NewFileGetter(baseDir string) *FileGetter { // Retrieve implements @Interface.Retrieve func (fg *FileGetter) Retrieve(logID string) ([]byte, error) { - if len(logID) == 0 { - return nil, errors.New("empty log identify") + if len(logID) != 24 { + return nil, errors.New("invalid length of log identify") + } + + if _, err := hex.DecodeString(logID); err != nil { + return nil, errors.New("invalid log identify") } fPath := path.Join(fg.baseDir, fmt.Sprintf("%s.log", logID)) diff --git a/src/jobservice/logger/getter/file_getter_test.go b/src/jobservice/logger/getter/file_getter_test.go index 9053819a3..3685d9874 100644 --- a/src/jobservice/logger/getter/file_getter_test.go +++ b/src/jobservice/logger/getter/file_getter_test.go @@ -9,10 +9,16 @@ import ( "github.com/goharbor/harbor/src/jobservice/errs" ) +const ( + newLogFileName = "30dbf28152f361ba57f95f84.log" + newLogFileID = "30dbf28152f361ba57f95f84" + nonExistFileID = "f00000000000000000000000" +) + // Test the log data getter func TestLogDataGetter(t *testing.T) { - fakeLog := path.Join(os.TempDir(), "TestLogDataGetter.log") - if err := ioutil.WriteFile(fakeLog, []byte("hello"), 0644); err != nil { + fakeLog := path.Join(os.TempDir(), newLogFileName) + if err := ioutil.WriteFile(fakeLog, []byte("hello"), 0600); err != nil { t.Fatal(err) } defer func() { @@ -22,7 +28,7 @@ func TestLogDataGetter(t *testing.T) { }() fg := NewFileGetter(os.TempDir()) - if _, err := fg.Retrieve("not-existing"); err != nil { + if _, err := fg.Retrieve(nonExistFileID); err != nil { if !errs.IsObjectNotFoundError(err) { t.Error("expect object not found error but got other error") } @@ -30,7 +36,7 @@ func TestLogDataGetter(t *testing.T) { t.Error("expect non nil error but got nil") } - data, err := fg.Retrieve("TestLogDataGetter") + data, err := fg.Retrieve(newLogFileID) if err != nil { t.Error(err) }