Fix semgrep dgryski.semgrep-go issues (#3511)

* fix semgrep dgryski.semgrep-go issues Fix most of the semgrep issues with the http://semgrep.dev/r/dgryski.semgrep-go ruleset (`semgrep --config http://semgrep.dev/r/dgryski.semgrep-go`). Left the issue with Content-Type text/plain on json.Encode in endpoints/openrtb2/amp_auction.go since changing to application/json breaks the AMP unit tests, and issues with the pointer receiver for MarshalJSON in usersync/cookie.go. Fix #3509. Signed-off-by: Dmitry S <dsavints@gmail.com> * add comment about legacy text/plain content type Signed-off-by: Dmitry S <dsavints@gmail.com> * fix semgrep dgryski issue with w.Write, add nosemgrep Signed-off-by: Dmitry S <dsavints@gmail.com> * add nosemgrep ignore for marshal-json-pointer-receiver --------- Signed-off-by: Dmitry S <dsavints@gmail.com>
prebid · Mar 25, 2024 · 94148bf · 94148bf
1 parent 54874d0
commit 94148bf
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 16 deletions.
diff --git a/endpoints/events/event.go b/endpoints/events/event.go
@@ -70,7 +70,7 @@ func (e *eventEndpoint) Handle(w http.ResponseWriter, r *http.Request, _ httprou
 		w.WriteHeader(http.StatusBadRequest)
 
 		for _, err := range errs {
-			w.Write([]byte(fmt.Sprintf("invalid request: %s\n", err.Error())))
+			fmt.Fprintf(w, "invalid request: %s\n", err.Error())
 		}
 
 		return
@@ -81,7 +81,7 @@ func (e *eventEndpoint) Handle(w http.ResponseWriter, r *http.Request, _ httprou
 
 	if err != nil {
 		w.WriteHeader(http.StatusUnauthorized)
-		w.Write([]byte(fmt.Sprintf("Account '%s' is required query parameter and can't be empty", AccountIdParameter)))
+		fmt.Fprintf(w, "Account '%s' is required query parameter and can't be empty", AccountIdParameter)
 		return
 	}
 	eventRequest.AccountID = accountId
@@ -105,15 +105,15 @@ func (e *eventEndpoint) Handle(w http.ResponseWriter, r *http.Request, _ httprou
 		w.WriteHeader(status)
 
 		for _, message := range messages {
-			w.Write([]byte(fmt.Sprintf("Invalid request: %s\n", message)))
+			fmt.Fprintf(w, "Invalid request: %s\n", message)
 		}
 		return
 	}
 
 	// Check if events are enabled for the account
 	if !account.Events.Enabled {
 		w.WriteHeader(http.StatusUnauthorized)
-		w.Write([]byte(fmt.Sprintf("Account '%s' doesn't support events", eventRequest.AccountID)))
+		fmt.Fprintf(w, "Account '%s' doesn't support events", eventRequest.AccountID)
 		return
 	}
 

diff --git a/endpoints/events/vtrack.go b/endpoints/events/vtrack.go
@@ -74,15 +74,15 @@ func (v *vtrackEndpoint) Handle(w http.ResponseWriter, r *http.Request, _ httpro
 	// account id is required
 	if accountId == "" {
 		w.WriteHeader(http.StatusBadRequest)
-		w.Write([]byte(fmt.Sprintf("Account '%s' is required query parameter and can't be empty", AccountParameter)))
+		fmt.Fprintf(w, "Account '%s' is required query parameter and can't be empty", AccountParameter)
 		return
 	}
 
 	// get integration value from request parameter
 	integrationType, err := getIntegrationType(r)
 	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
-		w.Write([]byte(fmt.Sprintf("Invalid integration type: %s\n", err.Error())))
+		fmt.Fprintf(w, "Invalid integration type: %s\n", err.Error())
 		return
 	}
 
@@ -92,7 +92,7 @@ func (v *vtrackEndpoint) Handle(w http.ResponseWriter, r *http.Request, _ httpro
 	// check if there was any error while parsing puts request
 	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
-		w.Write([]byte(fmt.Sprintf("Invalid request: %s\n", err.Error())))
+		fmt.Fprintf(w, "Invalid request: %s\n", err.Error())
 		return
 	}
 
@@ -106,7 +106,7 @@ func (v *vtrackEndpoint) Handle(w http.ResponseWriter, r *http.Request, _ httpro
 		w.WriteHeader(status)
 
 		for _, message := range messages {
-			w.Write([]byte(fmt.Sprintf("Invalid request: %s\n", message)))
+			fmt.Fprintf(w, "Invalid request: %s\n", message)
 		}
 		return
 	}
@@ -118,7 +118,7 @@ func (v *vtrackEndpoint) Handle(w http.ResponseWriter, r *http.Request, _ httpro
 		if len(errs) > 0 {
 			w.WriteHeader(http.StatusInternalServerError)
 			for _, err := range errs {
-				w.Write([]byte(fmt.Sprintf("Error(s) updating vast: %s\n", err.Error())))
+				fmt.Fprintf(w, "Error(s) updating vast: %s\n", err.Error())
 
 				return
 			}
@@ -128,7 +128,7 @@ func (v *vtrackEndpoint) Handle(w http.ResponseWriter, r *http.Request, _ httpro
 
 		if err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
-			w.Write([]byte(fmt.Sprintf("Error serializing pbs cache response: %s\n", err.Error())))
+			fmt.Fprintf(w, "Error serializing pbs cache response: %s\n", err.Error())
 
 			return
 		}

diff --git a/endpoints/openrtb2/amp_auction.go b/endpoints/openrtb2/amp_auction.go
@@ -171,7 +171,7 @@ func (deps *endpointDeps) AmpAuction(w http.ResponseWriter, r *http.Request, _ h
 	if errortypes.ContainsFatalError(errL) {
 		w.WriteHeader(http.StatusBadRequest)
 		for _, err := range errortypes.FatalOnly(errL) {
-			w.Write([]byte(fmt.Sprintf("Invalid request: %s\n", err.Error())))
+			fmt.Fprintf(w, "Invalid request: %s\n", err.Error())
 		}
 		labels.RequestStatus = metrics.RequestStatusBadInput
 		return
@@ -224,7 +224,7 @@ func (deps *endpointDeps) AmpAuction(w http.ResponseWriter, r *http.Request, _ h
 		w.WriteHeader(httpStatus)
 		labels.RequestStatus = metricsStatus
 		for _, err := range errortypes.FatalOnly(errL) {
-			w.Write([]byte(fmt.Sprintf("Invalid request: %s\n", err.Error())))
+			fmt.Fprintf(w, "Invalid request: %s\n", err.Error())
 		}
 		ao.Errors = append(ao.Errors, acctIDErrs...)
 		return
@@ -237,7 +237,7 @@ func (deps *endpointDeps) AmpAuction(w http.ResponseWriter, r *http.Request, _ h
 	if errortypes.ContainsFatalError(errs) {
 		w.WriteHeader(http.StatusBadRequest)
 		for _, err := range errortypes.FatalOnly(errs) {
-			w.Write([]byte(fmt.Sprintf("Invalid request: %s\n", err.Error())))
+			fmt.Fprintf(w, "Invalid request: %s\n", err.Error())
 		}
 		labels.RequestStatus = metrics.RequestStatusBadInput
 		return
@@ -398,8 +398,13 @@ func sendAmpResponse(
 	ao.AmpTargetingValues = targets
 
 	// Fixes #231
-	enc := json.NewEncoder(w)
+	enc := json.NewEncoder(w) // nosemgrep: json-encoder-needs-type
 	enc.SetEscapeHTML(false)
+	// Explicitly set content type to text/plain, which had previously been
+	// the implied behavior from the time the project was launched.
+	// It's unclear why text/plain was chosen or if it was an oversight,
+	// nevertheless we will keep it as such for compatibility reasons.
+	w.Header().Set("Content-Type", "text/plain; charset=utf-8")
 
 	// If an error happens when encoding the response, there isn't much we can do.
 	// If we've sent _any_ bytes, then Go would have sent the 200 status code first.

diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go
@@ -2409,7 +2409,7 @@ func writeError(errs []error, w http.ResponseWriter, labels *metrics.Labels) boo
 		w.WriteHeader(httpStatus)
 		labels.RequestStatus = metricsStatus
 		for _, err := range errs {
-			w.Write([]byte(fmt.Sprintf("Invalid request: %s\n", err.Error())))
+			fmt.Fprintf(w, "Invalid request: %s\n", err.Error())
 		}
 		rc = true
 	}

diff --git a/usersync/cookie.go b/usersync/cookie.go
@@ -225,7 +225,7 @@ type cookieJson struct {
 	OptOut bool                `json:"optout,omitempty"`
 }
 
-func (cookie *Cookie) MarshalJSON() ([]byte, error) {
+func (cookie *Cookie) MarshalJSON() ([]byte, error) { // nosemgrep: marshal-json-pointer-receiver
 	return jsonutil.Marshal(cookieJson{
 		UIDs:   cookie.uids,
 		OptOut: cookie.optOut,