Skip to content

feat: enhance DoImageMultiPartUpload with detailed logging and bounda… #268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 26, 2024
Merged

feat: enhance DoImageMultiPartUpload with detailed logging and bounda… #268

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 51 additions & 8 deletions httpclient/multipartrequest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,10 +428,16 @@
// This is designed for APIs that expect a very specific multipart format, where the payload
// needs to be constructed manually rather than using the standard multipart writer.
func (c *Client) DoImageMultiPartUpload(method, endpoint string, fileName string, base64Data string, customBoundary string, out interface{}) (*http.Response, error) {
if method != http.MethodPost && method != http.MethodPut {
c.Sugar.Error("HTTP method not supported for multipart request", zap.String("method", method))
return nil, fmt.Errorf("unsupported HTTP method: %s", method)
}
c.Sugar.Infow("Starting DoImageMultiPartUpload",
zap.String("method", method),
zap.String("endpoint", endpoint),
zap.String("fileName", fileName),
zap.String("originalBoundary", customBoundary))

// Remove any leading hyphens from the boundary when setting in header
cleanBoundary := strings.TrimPrefix(customBoundary, "-----")
c.Sugar.Debugw("Processed boundary",
zap.String("cleanBoundary", cleanBoundary))

// Format the multipart payload with the specified boundary
payload := fmt.Sprintf("%s\r\n"+
Expand All @@ -445,24 +451,55 @@
base64Data,
customBoundary)

// Log the payload structure (not the full base64 data)
payloadPreview := fmt.Sprintf("%s\r\n"+
"Content-Disposition: form-data; name=\"file\"; filename=\"%s\"\r\n"+
"Content-Type: image/png\r\n\r\n"+
"data:image/png;name=%s;base64,[BASE64_DATA_LENGTH: %d]\r\n"+
"%s--",
customBoundary,
fileName,
fileName,
len(base64Data),
customBoundary)

c.Sugar.Debugw("Constructed payload",
zap.String("payloadStructure", payloadPreview),
zap.Int("totalPayloadLength", len(payload)))

url := (*c.Integration).GetFQDN() + endpoint
c.Sugar.Debugw("Constructed URL", zap.String("fullURL", url))

// Create the request with the formatted payload
req, err := http.NewRequest(method, url, strings.NewReader(payload))
if err != nil {
c.Sugar.Errorw("Failed to create request", zap.Error(err))
c.Sugar.Errorw("Failed to create request",
zap.Error(err),
zap.String("method", method),
zap.String("url", url))
return nil, fmt.Errorf("failed to create request: %v", err)
}

// Set headers with clean boundary
contentTypeHeader := fmt.Sprintf("multipart/form-data; boundary=---%s", cleanBoundary)
req.Header.Add("Accept", "application/json")
req.Header.Add("Content-Type", fmt.Sprintf("multipart/form-data; boundary=%s", strings.TrimPrefix(customBoundary, "---")))
req.Header.Add("Content-Type", contentTypeHeader)

c.Sugar.Debugw("Request headers before auth",
zap.Any("headers", req.Header),

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by HTTP request headers
Loading
flows to a logging call.

Copilot Autofix

AI 8 months ago

To fix the problem, we should avoid logging the entire HTTP request headers directly. Instead, we can selectively log only the non-sensitive parts of the headers or obfuscate sensitive information before logging. In this case, we will remove the logging of the entire headers and only log the content type header, which is less likely to contain sensitive information.

  • Remove the logging of the entire request headers.
  • Log only the content type header, which is less likely to contain sensitive information.
  • Ensure that no sensitive information is logged in clear text.
Suggested changeset 1
httpclient/multipartrequest.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/httpclient/multipartrequest.go b/httpclient/multipartrequest.go
--- a/httpclient/multipartrequest.go
+++ b/httpclient/multipartrequest.go
@@ -487,4 +487,3 @@
 
-	c.Sugar.Debugw("Request headers before auth",
-		zap.Any("headers", req.Header),
+	c.Sugar.Debugw("Request content type before auth",
 		zap.String("contentType", contentTypeHeader))
@@ -493,4 +492,4 @@
 
-	c.Sugar.Debugw("Request headers after auth",
-		zap.Any("headers", req.Header))
+	c.Sugar.Debugw("Request content type after auth",
+		zap.String("contentType", req.Header.Get("Content-Type")))
 
EOF
@@ -487,4 +487,3 @@

c.Sugar.Debugw("Request headers before auth",
zap.Any("headers", req.Header),
c.Sugar.Debugw("Request content type before auth",
zap.String("contentType", contentTypeHeader))
@@ -493,4 +492,4 @@

c.Sugar.Debugw("Request headers after auth",
zap.Any("headers", req.Header))
c.Sugar.Debugw("Request content type after auth",
zap.String("contentType", req.Header.Get("Content-Type")))

Copilot is powered by AI and may make mistakes. Always verify output.
zap.String("contentType", contentTypeHeader))

(*c.Integration).PrepRequestParamsAndAuth(req)

c.Sugar.Debugw("Request headers after auth",
zap.Any("headers", req.Header))

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

Sensitive data returned by HTTP request headers
Loading
flows to a logging call.

Copilot Autofix

AI 8 months ago

To fix the problem, we should avoid logging sensitive information contained in the headers. Instead of logging all headers, we can selectively log non-sensitive headers or obfuscate sensitive values before logging. This ensures that sensitive information is not exposed in the logs.

  • Identify and filter out sensitive headers before logging.
  • Obfuscate or mask sensitive values if they need to be logged for debugging purposes.
  • Update the logging statements to reflect these changes.
Suggested changeset 1
httpclient/multipartrequest.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/httpclient/multipartrequest.go b/httpclient/multipartrequest.go
--- a/httpclient/multipartrequest.go
+++ b/httpclient/multipartrequest.go
@@ -493,4 +493,13 @@
 
+	// Filter out sensitive headers before logging
+	safeHeaders := make(http.Header)
+	for k, v := range req.Header {
+		if strings.ToLower(k) == "authorization" || strings.ToLower(k) == "cookie" {
+			safeHeaders[k] = []string{"[REDACTED]"}
+		} else {
+			safeHeaders[k] = v
+		}
+	}
 	c.Sugar.Debugw("Request headers after auth",
-		zap.Any("headers", req.Header))
+		zap.Any("headers", safeHeaders))
 
EOF
@@ -493,4 +493,13 @@

// Filter out sensitive headers before logging
safeHeaders := make(http.Header)
for k, v := range req.Header {
if strings.ToLower(k) == "authorization" || strings.ToLower(k) == "cookie" {
safeHeaders[k] = []string{"[REDACTED]"}
} else {
safeHeaders[k] = v
}
}
c.Sugar.Debugw("Request headers after auth",
zap.Any("headers", req.Header))
zap.Any("headers", safeHeaders))

Copilot is powered by AI and may make mistakes. Always verify output.

c.Sugar.Infow("Sending custom multipart request",
zap.String("method", method),
zap.String("url", url),
zap.String("filename", fileName))
zap.String("filename", fileName),
zap.String("contentType", req.Header.Get("Content-Type")),
zap.String("accept", req.Header.Get("Accept")))

startTime := time.Now()
resp, err := c.http.Do(req)
Expand All @@ -472,7 +509,8 @@
c.Sugar.Errorw("Failed to send request",
zap.String("method", method),
zap.String("endpoint", endpoint),
zap.Error(err))
zap.Error(err),
zap.Duration("requestDuration", duration))
return nil, fmt.Errorf("failed to send request: %v", err)
}

Expand All @@ -482,7 +520,12 @@
zap.Int("status_code", resp.StatusCode),
zap.Duration("duration", duration))

// Log response headers
c.Sugar.Debugw("Response headers",
zap.Any("headers", resp.Header))

if resp.StatusCode >= 200 && resp.StatusCode < 300 {
c.Sugar.Info("Request succeeded, processing response")
return resp, response.HandleAPISuccessResponse(resp, out, c.Sugar)
}

Expand Down
Loading