Skip to content
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.

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

JSON marshaling helpers will preserve Content-Type #22309

Merged
merged 2 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions sdk/azcore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Bugs Fixed

* `runtime.MarshalAsByteArray` and `runtime.MarshalAsJSON` will preserve the preexisting value of the `Content-Type` header.

### Other Changes

## 1.9.1 (2023-12-11)
Expand Down
86 changes: 48 additions & 38 deletions sdk/azcore/internal/exported/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,46 +125,11 @@ func (req *Request) OperationValue(value interface{}) bool {

// 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.
// Content-Type won't be set, and if it was set, will be deleted.
// Use streaming.NopCloser to turn an io.ReadSeeker into an io.ReadSeekCloser.
func (req *Request) SetBody(body io.ReadSeekCloser, contentType string) error {
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 {
// 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
}
}
// 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
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
// clobber the existing Content-Type to preserve behavior
return SetBody(req, body, contentType, true)
}

// RewindBody seeks the request's Body stream back to the beginning so it can be resent when retrying an operation.
Expand Down Expand Up @@ -211,3 +176,48 @@ type PolicyFunc func(*Request) (*http.Response, error)
func (pf PolicyFunc) Do(req *Request) (*http.Response, error) {
return pf(req)
}

// SetBody sets the specified ReadSeekCloser as the HTTP request body, and sets Content-Type and Content-Length accordingly.
// - req is the request to modify
// - body is the request body; if nil or empty, Content-Length won't be set
// - contentType is the value for the Content-Type header; if empty, Content-Length will be deleted
// - clobberContentType when true, will overwrite the existing value of Content-Type with contentType
func SetBody(req *Request, body io.ReadSeekCloser, contentType string, clobberContentType bool) error {
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 {
// 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
}
}
// 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
if contentType == "" {
// Del is a no-op when the header has no value
req.req.Header.Del(shared.HeaderContentType)
} else if req.req.Header.Get(shared.HeaderContentType) == "" || clobberContentType {
jhendrixMSFT marked this conversation as resolved.
Show resolved Hide resolved
req.req.Header.Set(shared.HeaderContentType, contentType)
}
return nil
}
19 changes: 19 additions & 0 deletions sdk/azcore/internal/exported/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,22 @@ func TestRequestWithContext(t *testing.T) {
req2.Raw().Header.Add("added-req2", "value")
require.EqualValues(t, "value", req1.Raw().Header.Get("added-req2"))
}

func TestSetBodyWithClobber(t *testing.T) {
req, err := NewRequest(context.Background(), http.MethodPatch, "https://contoso.com")
require.NoError(t, err)
require.NotNil(t, req)
req.req.Header.Set(shared.HeaderContentType, "clobber-me")
require.NoError(t, SetBody(req, NopCloser(strings.NewReader(`"json-string"`)), shared.ContentTypeAppJSON, true))
require.EqualValues(t, shared.ContentTypeAppJSON, req.req.Header.Get(shared.HeaderContentType))
}

func TestSetBodyWithNoClobber(t *testing.T) {
req, err := NewRequest(context.Background(), http.MethodPatch, "https://contoso.com")
require.NoError(t, err)
require.NotNil(t, req)
const mergePatch = "application/merge-patch+json"
req.req.Header.Set(shared.HeaderContentType, mergePatch)
require.NoError(t, SetBody(req, NopCloser(strings.NewReader(`"json-string"`)), shared.ContentTypeAppJSON, false))
require.EqualValues(t, mergePatch, req.req.Header.Get(shared.HeaderContentType))
}
6 changes: 4 additions & 2 deletions sdk/azcore/runtime/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ func EncodeByteArray(v []byte, format Base64Encoding) string {
func MarshalAsByteArray(req *policy.Request, v []byte, format Base64Encoding) error {
// send as a JSON string
encode := fmt.Sprintf("\"%s\"", EncodeByteArray(v, format))
return req.SetBody(exported.NopCloser(strings.NewReader(encode)), shared.ContentTypeAppJSON)
// tsp generated code can set Content-Type so we must prefer that
return exported.SetBody(req, exported.NopCloser(strings.NewReader(encode)), shared.ContentTypeAppJSON, false)
}

// MarshalAsJSON calls json.Marshal() to get the JSON encoding of v then calls SetBody.
Expand All @@ -106,7 +107,8 @@ func MarshalAsJSON(req *policy.Request, v interface{}) error {
if err != nil {
return fmt.Errorf("error marshalling type %T: %s", v, err)
}
return req.SetBody(exported.NopCloser(bytes.NewReader(b)), shared.ContentTypeAppJSON)
// tsp generated code can set Content-Type so we must prefer that
return exported.SetBody(req, exported.NopCloser(bytes.NewReader(b)), shared.ContentTypeAppJSON, false)
}

// MarshalAsXML calls xml.Marshal() to get the XML encoding of v then calls SetBody.
Expand Down