Skip to content

Commit 571989b

Browse files
committed
chore: Refactor MarshalMultipartRequest to improve error handling and logging
1 parent 3c31b1e commit 571989b

File tree

2 files changed

+59
-9
lines changed

2 files changed

+59
-9
lines changed

apiintegrations/jamfpro/jamfpro_api_request.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ func (j *JamfAPIHandler) MarshalRequest(body interface{}, method string, endpoin
5656
return data, nil
5757
}
5858

59+
// MarshalMultipartRequest creates a multipart request body for sending files and form fields in a single request.
5960
func (j *JamfAPIHandler) MarshalMultipartRequest(fields map[string]string, files map[string]string, log logger.Logger) ([]byte, string, string, error) {
6061
body := &bytes.Buffer{}
6162
writer := multipart.NewWriter(body)
@@ -68,7 +69,7 @@ func (j *JamfAPIHandler) MarshalMultipartRequest(fields map[string]string, files
6869
}
6970
}
7071

71-
// Add the files to the form data, using safeOpenFile to ensure secure file access
72+
// Add the files to the form data
7273
for formField, filePath := range files {
7374
file, err := helpers.SafeOpenFile(filePath)
7475
if err != nil {

httpclient/multipartrequest.go

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,14 @@ package httpclient
33

44
import (
55
"bytes"
6+
"io"
7+
"mime/multipart"
68
"net/http"
9+
"path/filepath"
710

811
"github.com/deploymenttheory/go-api-http-client/authenticationhandler"
912
"github.com/deploymenttheory/go-api-http-client/headers"
13+
"github.com/deploymenttheory/go-api-http-client/helpers"
1014
"github.com/deploymenttheory/go-api-http-client/response"
1115
"go.uber.org/zap"
1216
)
@@ -54,17 +58,65 @@ func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]s
5458
return nil, err
5559
}
5660

57-
// Marshal the multipart form data
58-
requestData, contentType, logBody, err := c.APIHandler.MarshalMultipartRequest(fields, files, log)
59-
if err != nil {
61+
// Create a buffer to hold the multipart form data
62+
body := &bytes.Buffer{}
63+
writer := multipart.NewWriter(body)
64+
65+
// Add the simple fields to the form data
66+
for field, value := range fields {
67+
log.Debug("Adding field to multipart request", zap.String("Field", field), zap.String("Value", value))
68+
if err := writer.WriteField(field, value); err != nil {
69+
return nil, err
70+
}
71+
}
72+
73+
// Add the files to the form data
74+
for formField, filePath := range files {
75+
file, err := helpers.SafeOpenFile(filePath)
76+
if err != nil {
77+
log.Error("Failed to open file securely", zap.String("file", filePath), zap.Error(err))
78+
return nil, err
79+
}
80+
defer file.Close()
81+
82+
part, err := writer.CreateFormFile(formField, filepath.Base(filePath))
83+
if err != nil {
84+
return nil, err
85+
}
86+
log.Debug("Adding file to multipart request", zap.String("FormField", formField), zap.String("FilePath", filePath))
87+
if _, err := io.Copy(part, file); err != nil {
88+
return nil, err
89+
}
90+
}
91+
92+
// Close the writer to finish writing the multipart message
93+
if err := writer.Close(); err != nil {
6094
return nil, err
6195
}
6296

97+
contentType := writer.FormDataContentType()
98+
bodyBytes := body.Bytes()
99+
100+
// Extract the first and last parts of the body for logging
101+
const logSegmentSize = 1024 // 1 KB
102+
bodyLen := len(bodyBytes)
103+
var logBody string
104+
if bodyLen <= 2*logSegmentSize {
105+
logBody = string(bodyBytes)
106+
} else {
107+
logBody = string(bodyBytes[:logSegmentSize]) + "..." + string(bodyBytes[bodyLen-logSegmentSize:])
108+
}
109+
110+
// Log the boundary and a partial body for debugging
111+
boundary := writer.Boundary()
112+
log.Debug("Multipart boundary", zap.String("Boundary", boundary))
113+
log.Debug("Multipart request body (partial)", zap.String("Body", logBody))
114+
63115
// Construct URL using the ConstructAPIResourceEndpoint function
64116
url := c.APIHandler.ConstructAPIResourceEndpoint(endpoint, log)
65117

66118
// Create the request
67-
req, err := http.NewRequest(method, url, bytes.NewBuffer(requestData))
119+
req, err := http.NewRequest(method, url, bytes.NewBuffer(bodyBytes))
68120
if err != nil {
69121
return nil, err
70122
}
@@ -73,13 +125,10 @@ func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]s
73125
headerHandler := headers.NewHeaderHandler(req, c.Logger, c.APIHandler, c.AuthTokenHandler)
74126

75127
// Use HeaderManager to set headers
76-
headerHandler.SetContentType(contentType) // Set correct content type with boundary
128+
headerHandler.SetContentType(contentType)
77129
headerHandler.SetRequestHeaders(endpoint)
78130
headerHandler.LogHeaders(c.clientConfig.ClientOptions.Logging.HideSensitiveData)
79131

80-
// Log request details with the partial body
81-
log.Debug("HTTP Request Details (partial body)", zap.String("Method", method), zap.String("URL", url), zap.String("Content-Type", contentType), zap.String("Body", logBody))
82-
83132
// Execute the request
84133
resp, err := c.httpClient.Do(req)
85134
if err != nil {

0 commit comments

Comments
 (0)