Merge pull request #16374 from chlins/fix/codeql-alerts-2

fix: trim linebreaks for log parameters

src/jobservice/api/router.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/goharbor/harbor/src/jobservice/errs"
 	"github.com/goharbor/harbor/src/jobservice/logger"
+	"github.com/goharbor/harbor/src/lib"
 	"github.com/goharbor/harbor/src/lib/errors"
 	tracelib "github.com/goharbor/harbor/src/lib/trace"
 )
@@ -78,7 +79,7 @@ func (br *BaseRouter) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 			if authErr == nil {
 				authErr = errors.Errorf("unauthorized: %s", err)
 			}
-			logger.Errorf("Serve http request '%q %q' failed with error: %s", req.Method, req.URL.String(), authErr.Error())
+			logger.Errorf("Serve http request '%s %s' failed with error: %s", lib.TrimLineBreaks(req.Method), req.URL.String(), authErr.Error())
 			w.WriteHeader(http.StatusUnauthorized)
 			writeDate(w, []byte(authErr.Error()))
 			return

src/jobservice/period/basic_scheduler.go

@@ -20,6 +20,7 @@ import (
 	"time"
 
 	"github.com/goharbor/harbor/src/jobservice/errs"
+	"github.com/goharbor/harbor/src/lib"
 
 	"github.com/gocraft/work"
 	"github.com/goharbor/harbor/src/jobservice/common/rds"
@@ -119,10 +120,10 @@ func (bs *basicScheduler) UnSchedule(policyID string) error {
 	// Failure errors will be only logged here
 	eKey := rds.KeyUpstreamJobAndExecutions(bs.namespace, policyID)
 	if eIDs, err := getPeriodicExecutions(conn, eKey); err != nil {
-		logger.Errorf("Get executions for periodic job %q error: %s", policyID, err)
+		logger.Errorf("Get executions for periodic job %s error: %s", lib.TrimLineBreaks(policyID), err)
 	} else {
 		if len(eIDs) == 0 {
-			logger.Debugf("no stopped executions: %q", policyID)
+			logger.Debugf("no stopped executions: %s", lib.TrimLineBreaks(policyID))
 		}
 
 		for _, eID := range eIDs {
@@ -164,7 +165,7 @@ func (bs *basicScheduler) UnSchedule(policyID string) error {
 	}
 
 	if removed == 0 {
-		logger.Warningf("No periodic job with ID=%q and numeric ID=%d removed from the periodic job policy set", policyID, numericID)
+		logger.Warningf("No periodic job with ID=%s and numeric ID=%d removed from the periodic job policy set", lib.TrimLineBreaks(policyID), numericID)
 	}
 
 	return nil
@@ -199,7 +200,7 @@ func (bs *basicScheduler) locatePolicy(policyID string, conn redis.Conn) (int64,
 	// Switch the job stats to stopped if the job stats existing
 	// Should not block the next clear action
 	if err := tracker.Stop(); err != nil {
-		logger.Errorf("Stop periodic job %q failed with error: %s", policyID, err)
+		logger.Errorf("Stop periodic job %s failed with error: %s", lib.TrimLineBreaks(policyID), err)
 	}
 
 	return tracker.NumericID()

src/jobservice/period/enqueuer.go

@@ -26,6 +26,7 @@ import (
 	"github.com/goharbor/harbor/src/jobservice/job"
 	"github.com/goharbor/harbor/src/jobservice/lcm"
 	"github.com/goharbor/harbor/src/jobservice/logger"
+	"github.com/goharbor/harbor/src/lib"
 	"github.com/gomodule/redigo/redis"
 	"github.com/robfig/cron"
 )
@@ -163,7 +164,7 @@ func (e *enqueuer) scheduleNextJobs(p *Policy, conn redis.Conn) {
 		// The cron spec should be already checked at upper layers.
 		// Just in cases, if error occurred, ignore it
 		e.lastEnqueueErr = err
-		logger.Errorf("Invalid corn spec in periodic policy %q %s: %s", p.JobName, p.ID, err)
+		logger.Errorf("Invalid corn spec in periodic policy %s %s: %s", lib.TrimLineBreaks(p.JobName), p.ID, err)
 	} else {
 		for t := schedule.Next(nowTime); t.Before(horizon); t = schedule.Next(t) {
 			epoch := t.Unix()
@@ -222,7 +223,7 @@ func (e *enqueuer) scheduleNextJobs(p *Policy, conn redis.Conn) {
 				break // Probably redis connection is broken
 			}
 
-			logger.Debugf("Scheduled execution for periodic job %q:%s at %d", j.Name, p.ID, epoch)
+			logger.Debugf("Scheduled execution for periodic job %s:%s at %d", lib.TrimLineBreaks(j.Name), p.ID, epoch)
 		}
 	}
 }

src/jobservice/worker/cworker/c_worker.go

@@ -21,6 +21,7 @@ import (
 	"time"
 
 	"github.com/goharbor/harbor/src/jobservice/errs"
+	"github.com/goharbor/harbor/src/lib"
 
 	"github.com/gocraft/work"
 	"github.com/goharbor/harbor/src/jobservice/common/utils"
@@ -360,7 +361,7 @@ func (w *basicWorker) StopJob(jobID string) error {
 		// We need to delete the scheduled job in the queue if it is not running yet
 		if err := w.client.DeleteScheduledJob(t.Job().Info.RunAt, jobID); err != nil {
 			// Job is already running?
-			logger.Warningf("scheduled job %q (run at = %d) is not found in the queue, is it running?", jobID, t.Job().Info.RunAt)
+			logger.Warningf("scheduled job %s (run at = %d) is not found in the queue, is it running?", lib.TrimLineBreaks(jobID), t.Job().Info.RunAt)
 		}
 	}
 

src/lib/strings.go

@@ -0,0 +1,24 @@
+// Copyright Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package lib
+
+import "strings"
+
+// TrimsLineBreaks trims line breaks in string.
+func TrimLineBreaks(s string) string {
+	escaped := strings.ReplaceAll(s, "\n", "")
+	escaped = strings.ReplaceAll(escaped, "\r", "")
+	return escaped
+}

src/lib/strings_test.go

@@ -0,0 +1,34 @@
+// Copyright Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package lib
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestTrimLineBreaks(t *testing.T) {
+	assert := assert.New(t)
+
+	s := `
+abc
+def
+
+`
+	expect := "abcdef"
+	actual := TrimLineBreaks(s)
+	assert.Equal(expect, actual, "should trim line breaks")
+}