Request.SetBody() allows overwriting with an empty body (#19754)

Author: chlowell

sdk/azcore/CHANGELOG.md

@@ -13,6 +13,7 @@
 
 ### Other Changes
 * Updated `internal` module to latest version.
+* `policy/Request.SetBody()` allows replacing a request's body with an empty one
 
 ## 1.2.0 (2022-11-04)
 

sdk/azcore/internal/exported/request.go

@@ -100,32 +100,46 @@ func (req *Request) OperationValue(value interface{}) bool {
 	return req.values.get(value)
 }
 
-// SetBody sets the specified ReadSeekCloser as the HTTP request body.
+// SetBody sets the specified ReadSeekCloser as the HTTP request body, and sets Content-Type and Content-Length
+// accordingly. If the ReadSeekCloser is nil or empty, Content-Length won't be set. If contentType is "",
+// Content-Type won't be set.
 func (req *Request) SetBody(body io.ReadSeekCloser, contentType string) error {
-	// Set the body and content length.
-	size, err := body.Seek(0, io.SeekEnd) // Seek to the end to get the stream's size
-	if err != nil {
-		return err
+	var err error
+	var size int64
+	if body != nil {
+		size, err = body.Seek(0, io.SeekEnd) // Seek to the end to get the stream's size
+		if err != nil {
+			return err
+		}
 	}
 	if size == 0 {
-		body.Close()
-		return nil
-	}
-	_, err = body.Seek(0, io.SeekStart)
-	if err != nil {
-		return err
+		// treat an empty stream the same as a nil one: assign req a nil body
+		body = nil
+		// RFC 9110 specifies a client shouldn't set Content-Length on a request containing no content
+		// (Del is a no-op when the header has no value)
+		req.req.Header.Del(shared.HeaderContentLength)
+	} else {
+		_, err = body.Seek(0, io.SeekStart)
+		if err != nil {
+			return err
+		}
+		req.req.Header.Set(shared.HeaderContentLength, strconv.FormatInt(size, 10))
+		req.Raw().GetBody = func() (io.ReadCloser, error) {
+			_, err := body.Seek(0, io.SeekStart) // Seek back to the beginning of the stream
+			return body, err
+		}
 	}
-	req.Raw().GetBody = func() (io.ReadCloser, error) {
-		_, err := body.Seek(0, io.SeekStart) // Seek back to the beginning of the stream
-		return body, err
-	}
-	// keep a copy of the original body.  this is to handle cases
+	// keep a copy of the body argument.  this is to handle cases
 	// where req.Body is replaced, e.g. httputil.DumpRequest and friends.
 	req.body = body
 	req.req.Body = body
 	req.req.ContentLength = size
-	req.req.Header.Set(shared.HeaderContentType, contentType)
-	req.req.Header.Set(shared.HeaderContentLength, strconv.FormatInt(size, 10))
+	if contentType == "" {
+		// Del is a no-op when the header has no value
+		req.req.Header.Del(shared.HeaderContentType)
+	} else {
+		req.req.Header.Set(shared.HeaderContentType, contentType)
+	}
 	return nil
 }
 

sdk/azcore/internal/exported/request_test.go

@@ -13,6 +13,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/shared"
 	"github.com/stretchr/testify/require"
 )
 
@@ -123,6 +124,25 @@ func TestRequestEmptyBody(t *testing.T) {
 	require.NoError(t, err)
 	require.NoError(t, req.SetBody(NopCloser(strings.NewReader("")), "application/text"))
 	require.Nil(t, req.Body())
+	require.NotContains(t, req.Raw().Header, shared.HeaderContentLength)
+	require.Equal(t, []string{"application/text"}, req.Raw().Header[shared.HeaderContentType])
+
+	// SetBody should treat a nil ReadSeekCloser the same as one having no content
+	req, err = NewRequest(context.Background(), http.MethodPost, testURL)
+	require.NoError(t, err)
+	require.NoError(t, req.SetBody(nil, ""))
+	require.Nil(t, req.Body())
+	require.NotContains(t, req.Raw().Header, shared.HeaderContentLength)
+	require.NotContains(t, req.Raw().Header, shared.HeaderContentType)
+
+	// SetBody should allow replacing a previously set body with an empty one
+	req, err = NewRequest(context.Background(), http.MethodPost, testURL)
+	require.NoError(t, err)
+	require.NoError(t, req.SetBody(NopCloser(strings.NewReader("content")), "application/text"))
+	require.NoError(t, req.SetBody(nil, "application/json"))
+	require.Nil(t, req.Body())
+	require.NotContains(t, req.Raw().Header, shared.HeaderContentLength)
+	require.Equal(t, []string{"application/json"}, req.Raw().Header[shared.HeaderContentType])
 }
 
 func TestRequestClone(t *testing.T) {