1. docker
  2. Commit c4f9616c4a14852c02785eda0a9e0cbbd7af8ecb
Browse code

client: cleanup encoding body and add test-coverage

This code has various other issue, for which TODOs were added; this
commit only does some initial cleaning up, and improves docs and
test-coverage.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Sebastiaan van Stijn authored on 2025/07/15 16:50:20
Showing 5 changed files
client/hijack.go
History View file @ c4f9616
... ... 
@@ -17,11 +17,11 @@ import (
17 17
18 18 
 // postHijacked sends a POST request and hijacks the connection.
19 19 
 func (cli *Client) postHijacked(ctx context.Context, path string, query url.Values, body interface{}, headers map[string][]string) (types.HijackedResponse, error) {
20 
-	bodyEncoded, err := encodeData(body)
20 
+	jsonBody, err := jsonEncode(body)
21 21 
 	if err != nil {
22 22 
 		return types.HijackedResponse{}, err
23 23 
 	}
24 
-	req, err := cli.buildRequest(ctx, http.MethodPost, cli.getAPIPath(ctx, path, query), bodyEncoded, headers)
24 
+	req, err := cli.buildRequest(ctx, http.MethodPost, cli.getAPIPath(ctx, path, query), jsonBody, headers)
25 25 
 	if err != nil {
26 26 
 		return types.HijackedResponse{}, err
27 27 
 	}
client/request.go
History View file @ c4f9616
... ... 
@@ -27,25 +27,25 @@ func (cli *Client) get(ctx context.Context, path string, query url.Values, heade
27 27 
 	return cli.sendRequest(ctx, http.MethodGet, path, query, nil, headers)
28 28 
 }
29 29
30 
-// post sends an http request to the docker API using the method POST with a specific Go context.
31 
-func (cli *Client) post(ctx context.Context, path string, query url.Values, obj interface{}, headers http.Header) (*http.Response, error) {
32 
-	body, headers, err := encodeBody(obj, headers)
30 
+// post sends an http POST request to the API.
31 
+func (cli *Client) post(ctx context.Context, path string, query url.Values, body interface{}, headers http.Header) (*http.Response, error) {
32 
+	jsonBody, headers, err := prepareJSONRequest(body, headers)
33 33 
 	if err != nil {
34 34 
 		return nil, err
35 35 
 	}
36 
-	return cli.sendRequest(ctx, http.MethodPost, path, query, body, headers)
36 
+	return cli.sendRequest(ctx, http.MethodPost, path, query, jsonBody, headers)
37 37 
 }
38 38
39 39 
 func (cli *Client) postRaw(ctx context.Context, path string, query url.Values, body io.Reader, headers http.Header) (*http.Response, error) {
40 40 
 	return cli.sendRequest(ctx, http.MethodPost, path, query, body, headers)
41 41 
 }
42 42
43 
-func (cli *Client) put(ctx context.Context, path string, query url.Values, obj interface{}, headers http.Header) (*http.Response, error) {
44 
-	body, headers, err := encodeBody(obj, headers)
43 
+func (cli *Client) put(ctx context.Context, path string, query url.Values, body interface{}, headers http.Header) (*http.Response, error) {
44 
+	jsonBody, headers, err := prepareJSONRequest(body, headers)
45 45 
 	if err != nil {
46 46 
 		return nil, err
47 47 
 	}
48 
-	return cli.putRaw(ctx, path, query, body, headers)
48 
+	return cli.putRaw(ctx, path, query, jsonBody, headers)
49 49 
 }
50 50
51 51 
 // putRaw sends an http request to the docker API using the method PUT.
... ... 
@@ -64,26 +64,36 @@ func (cli *Client) delete(ctx context.Context, path string, query url.Values, he
64 64 
 	return cli.sendRequest(ctx, http.MethodDelete, path, query, nil, headers)
65 65 
 }
66 66
67 
-func encodeBody(obj interface{}, headers http.Header) (io.Reader, http.Header, error) {
68 
-	if obj == nil {
67 
+// prepareJSONRequest encodes the given body to JSON and returns it as an [io.Reader], and sets the Content-Type
68 
+// header. If body is nil, or a nil-interface, a "nil" body is returned without
69 
+// error.
70 
+//
71 
+// TODO(thaJeztah): should this return an error if a different Content-Type is already set?
72 
+// TODO(thaJeztah): is "nil" the appropriate approach for an empty body, or should we use [http.NoBody] (or similar)?
73 
+func prepareJSONRequest(body interface{}, headers http.Header) (io.Reader, http.Header, error) {
74 
+	if body == nil {
69 75 
 		return nil, headers, nil
70 76 
 	}
71 77 
 	// encoding/json encodes a nil pointer as the JSON document `null`,
72 78 
 	// irrespective of whether the type implements json.Marshaler or encoding.TextMarshaler.
73 79 
 	// That is almost certainly not what the caller intended as the request body.
74 
-	if reflect.TypeOf(obj).Kind() == reflect.Ptr && reflect.ValueOf(obj).IsNil() {
80 
+	//
81 
+	// TODO(thaJeztah): consider moving this to jsonEncode, which would also allow returning an (empty) reader instead of nil.
82 
+	if reflect.TypeOf(body).Kind() == reflect.Ptr && reflect.ValueOf(body).IsNil() {
75 83 
 		return nil, headers, nil
76 84 
 	}
77 85
78 
-	body, err := encodeData(obj)
86 
+	jsonBody, err := jsonEncode(body)
79 87 
 	if err != nil {
80 88 
 		return nil, headers, err
81 89 
 	}
82 
-	if headers == nil {
83 
-		headers = make(map[string][]string)
90 
+	hdr := http.Header{}
91 
+	if headers != nil {
92 
+		hdr = headers.Clone()
84 93 
 	}
85 
-	headers["Content-Type"] = []string{"application/json"}
86 
-	return body, headers, nil
94 
+
95 
+	hdr.Set("Content-Type", "application/json")
96 
+	return jsonBody, hdr, nil
87 97 
 }
88 98
89 99 
 func (cli *Client) buildRequest(ctx context.Context, method, path string, body io.Reader, headers http.Header) (*http.Request, error) {
... ... 
@@ -293,14 +303,14 @@ func (cli *Client) addHeaders(req *http.Request, headers http.Header) *http.Requ
293 293 
 	return req
294 294 
 }
295 295
296 
-func encodeData(data interface{}) (*bytes.Buffer, error) {
297 
-	params := bytes.NewBuffer(nil)
296 
+func jsonEncode(data interface{}) (io.Reader, error) {
297 
+	var params bytes.Buffer
298 298 
 	if data != nil {
299 
-		if err := json.NewEncoder(params).Encode(data); err != nil {
299 
+		if err := json.NewEncoder(&params).Encode(data); err != nil {
300 300 
 			return nil, err
301 301 
 		}
302 302 
 	}
303 
-	return params, nil
303 
+	return &params, nil
304 304 
 }
305 305
306 306 
 func ensureReaderClosed(response *http.Response) {
client/request_test.go
History View file @ c4f9616
... ... 
@@ -279,3 +279,83 @@ func TestDeadlineExceededContext(t *testing.T) {
279 279 
 	_, err := client.sendRequest(ctx, http.MethodGet, testEndpoint, nil, nil, nil)
280 280 
 	assert.Check(t, is.ErrorIs(err, context.DeadlineExceeded))
281 281 
 }
282 
+
283 
+func TestPrepareJSONRequest(t *testing.T) {
284 
+	tests := []struct {
285 
+		doc        string
286 
+		body       any
287 
+		headers    http.Header
288 
+		expBody    string
289 
+		expNilBody bool
290 
+		expHeaders http.Header
291 
+	}{
292 
+		{
293 
+			doc:        "nil body",
294 
+			body:       nil,
295 
+			headers:    http.Header{"Something": []string{"something"}},
296 
+			expNilBody: true,
297 
+			expHeaders: http.Header{
298 
+				// currently, no content-type is set on empty requests.
299 
+				"Something": []string{"something"},
300 
+			},
301 
+		},
302 
+		{
303 
+			doc:        "nil interface body",
304 
+			body:       (*struct{})(nil),
305 
+			headers:    http.Header{"Something": []string{"something"}},
306 
+			expNilBody: true,
307 
+			expHeaders: http.Header{
308 
+				// currently, no content-type is set on empty requests.
309 
+				"Something": []string{"something"},
310 
+			},
311 
+		},
312 
+		{
313 
+			doc:     "empty struct body",
314 
+			body:    &struct{}{},
315 
+			headers: http.Header{"Something": []string{"something"}},
316 
+			expBody: `{}`,
317 
+			expHeaders: http.Header{
318 
+				"Content-Type": []string{"application/json"},
319 
+				"Something":    []string{"something"},
320 
+			},
321 
+		},
322 
+		{
323 
+			doc:     "json raw message",
324 
+			body:    json.RawMessage("{}"),
325 
+			expBody: `{}`,
326 
+			expHeaders: http.Header{
327 
+				"Content-Type": []string{"application/json"},
328 
+			},
329 
+		},
330 
+		{
331 
+			doc:     "empty body",
332 
+			body:    http.NoBody,
333 
+			expBody: `{}`,
334 
+			expHeaders: http.Header{
335 
+				"Content-Type": []string{"application/json"},
336 
+			},
337 
+		},
338 
+	}
339 
+
340 
+	for _, tc := range tests {
341 
+		t.Run(tc.doc, func(t *testing.T) {
342 
+			req, hdr, err := prepareJSONRequest(tc.body, tc.headers)
343 
+			assert.NilError(t, err)
344 
+
345 
+			var body string
346 
+			if tc.expNilBody {
347 
+				assert.Check(t, is.Nil(req))
348 
+			} else {
349 
+				assert.Assert(t, req != nil)
350 
+
351 
+				resp, err := io.ReadAll(req)
352 
+				assert.NilError(t, err)
353 
+				body = strings.TrimSpace(string(resp))
354 
+			}
355 
+
356 
+			assert.Check(t, is.Equal(body, tc.expBody))
357 
+			assert.Check(t, is.DeepEqual(hdr, tc.expHeaders))
358 
+			assert.Check(t, is.Equal(tc.headers.Get("Content-Type"), ""), "Should not have mutated original headers")
359 
+		})
360 
+	}
361 
+}
vendor/github.com/moby/moby/client/hijack.go
History View file @ c4f9616
... ... 
@@ -17,11 +17,11 @@ import (
17 17
18 18 
 // postHijacked sends a POST request and hijacks the connection.
19 19 
 func (cli *Client) postHijacked(ctx context.Context, path string, query url.Values, body interface{}, headers map[string][]string) (types.HijackedResponse, error) {
20 
-	bodyEncoded, err := encodeData(body)
20 
+	jsonBody, err := jsonEncode(body)
21 21 
 	if err != nil {
22 22 
 		return types.HijackedResponse{}, err
23 23 
 	}
24 
-	req, err := cli.buildRequest(ctx, http.MethodPost, cli.getAPIPath(ctx, path, query), bodyEncoded, headers)
24 
+	req, err := cli.buildRequest(ctx, http.MethodPost, cli.getAPIPath(ctx, path, query), jsonBody, headers)
25 25 
 	if err != nil {
26 26 
 		return types.HijackedResponse{}, err
27 27 
 	}
vendor/github.com/moby/moby/client/request.go
History View file @ c4f9616
... ... 
@@ -27,25 +27,25 @@ func (cli *Client) get(ctx context.Context, path string, query url.Values, heade
27 27 
 	return cli.sendRequest(ctx, http.MethodGet, path, query, nil, headers)
28 28 
 }
29 29
30 
-// post sends an http request to the docker API using the method POST with a specific Go context.
31 
-func (cli *Client) post(ctx context.Context, path string, query url.Values, obj interface{}, headers http.Header) (*http.Response, error) {
32 
-	body, headers, err := encodeBody(obj, headers)
30 
+// post sends an http POST request to the API.
31 
+func (cli *Client) post(ctx context.Context, path string, query url.Values, body interface{}, headers http.Header) (*http.Response, error) {
32 
+	jsonBody, headers, err := prepareJSONRequest(body, headers)
33 33 
 	if err != nil {
34 34 
 		return nil, err
35 35 
 	}
36 
-	return cli.sendRequest(ctx, http.MethodPost, path, query, body, headers)
36 
+	return cli.sendRequest(ctx, http.MethodPost, path, query, jsonBody, headers)
37 37 
 }
38 38
39 39 
 func (cli *Client) postRaw(ctx context.Context, path string, query url.Values, body io.Reader, headers http.Header) (*http.Response, error) {
40 40 
 	return cli.sendRequest(ctx, http.MethodPost, path, query, body, headers)
41 41 
 }
42 42
43 
-func (cli *Client) put(ctx context.Context, path string, query url.Values, obj interface{}, headers http.Header) (*http.Response, error) {
44 
-	body, headers, err := encodeBody(obj, headers)
43 
+func (cli *Client) put(ctx context.Context, path string, query url.Values, body interface{}, headers http.Header) (*http.Response, error) {
44 
+	jsonBody, headers, err := prepareJSONRequest(body, headers)
45 45 
 	if err != nil {
46 46 
 		return nil, err
47 47 
 	}
48 
-	return cli.putRaw(ctx, path, query, body, headers)
48 
+	return cli.putRaw(ctx, path, query, jsonBody, headers)
49 49 
 }
50 50
51 51 
 // putRaw sends an http request to the docker API using the method PUT.
... ... 
@@ -64,26 +64,36 @@ func (cli *Client) delete(ctx context.Context, path string, query url.Values, he
64 64 
 	return cli.sendRequest(ctx, http.MethodDelete, path, query, nil, headers)
65 65 
 }
66 66
67 
-func encodeBody(obj interface{}, headers http.Header) (io.Reader, http.Header, error) {
68 
-	if obj == nil {
67 
+// prepareJSONRequest encodes the given body to JSON and returns it as an [io.Reader], and sets the Content-Type
68 
+// header. If body is nil, or a nil-interface, a "nil" body is returned without
69 
+// error.
70 
+//
71 
+// TODO(thaJeztah): should this return an error if a different Content-Type is already set?
72 
+// TODO(thaJeztah): is "nil" the appropriate approach for an empty body, or should we use [http.NoBody] (or similar)?
73 
+func prepareJSONRequest(body interface{}, headers http.Header) (io.Reader, http.Header, error) {
74 
+	if body == nil {
69 75 
 		return nil, headers, nil
70 76 
 	}
71 77 
 	// encoding/json encodes a nil pointer as the JSON document `null`,
72 78 
 	// irrespective of whether the type implements json.Marshaler or encoding.TextMarshaler.
73 79 
 	// That is almost certainly not what the caller intended as the request body.
74 
-	if reflect.TypeOf(obj).Kind() == reflect.Ptr && reflect.ValueOf(obj).IsNil() {
80 
+	//
81 
+	// TODO(thaJeztah): consider moving this to jsonEncode, which would also allow returning an (empty) reader instead of nil.
82 
+	if reflect.TypeOf(body).Kind() == reflect.Ptr && reflect.ValueOf(body).IsNil() {
75 83 
 		return nil, headers, nil
76 84 
 	}
77 85
78 
-	body, err := encodeData(obj)
86 
+	jsonBody, err := jsonEncode(body)
79 87 
 	if err != nil {
80 88 
 		return nil, headers, err
81 89 
 	}
82 
-	if headers == nil {
83 
-		headers = make(map[string][]string)
90 
+	hdr := http.Header{}
91 
+	if headers != nil {
92 
+		hdr = headers.Clone()
84 93 
 	}
85 
-	headers["Content-Type"] = []string{"application/json"}
86 
-	return body, headers, nil
94 
+
95 
+	hdr.Set("Content-Type", "application/json")
96 
+	return jsonBody, hdr, nil
87 97 
 }
88 98
89 99 
 func (cli *Client) buildRequest(ctx context.Context, method, path string, body io.Reader, headers http.Header) (*http.Request, error) {
... ... 
@@ -293,14 +303,14 @@ func (cli *Client) addHeaders(req *http.Request, headers http.Header) *http.Requ
293 293 
 	return req
294 294 
 }
295 295
296 
-func encodeData(data interface{}) (*bytes.Buffer, error) {
297 
-	params := bytes.NewBuffer(nil)
296 
+func jsonEncode(data interface{}) (io.Reader, error) {
297 
+	var params bytes.Buffer
298 298 
 	if data != nil {
299 
-		if err := json.NewEncoder(params).Encode(data); err != nil {
299 
+		if err := json.NewEncoder(&params).Encode(data); err != nil {
300 300 
 			return nil, err
301 301 
 		}
302 302 
 	}
303 
-	return params, nil
303 
+	return &params, nil
304 304 
 }
305 305
306 306 
 func ensureReaderClosed(response *http.Response) {