Merge pull request #9245 from ninjadq/check_logid

Add checking for logid

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.

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)
 	}

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))

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)
 	}