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

add Request.CloneWithBody and Request.SetBody #105

Merged
merged 1 commit into from
Mar 26, 2024
Merged
Changes from all commits
Commits
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
60 changes: 40 additions & 20 deletions fsthttp/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,9 @@ type Request struct {
// reads may return immediately with EOF. For outgoing requests, the body is
// optional. A body may only be read once.
//
// It is possible to assign the unread body of the incoming client request
// to the body field of a different request. When that second request is
// sent, the body will be efficiently streamed from the incoming request.
//
// It is also possible to assign the unread body of a received response to
// the body field of a request, with the same results.
// Prefer using the SetBody method over assigning to this value directly,
// as it enables optimizations when sending outgoing requests. See the
// SetBody documentation for more information.
Body io.ReadCloser

// Host is the hostname parsed from the incoming request URL.
Expand Down Expand Up @@ -116,11 +113,16 @@ func NewRequest(method string, uri string, body io.Reader) (*Request, error) {
return nil, err
}

rc, ok := body.(io.ReadCloser)
if !ok && body != nil {
rc = nopCloser{body}
}

return &Request{
Method: method,
URL: u,
Header: NewHeader(),
Body: makeBodyFor(body),
Body: rc,
Host: u.Host,
}, nil
}
Expand Down Expand Up @@ -209,8 +211,30 @@ func newClientRequest() (*Request, error) {
}, nil
}

// SetBody sets the [Request]'s body to the provided [io.Reader]. Prefer
// using this method over setting the Body field directly, as it enables
// optimizations in the runtime.
//
// If an unread body from an incoming client request is set on an
// outgoing upstream request, the body will be efficiently streamed from
// the incoming request. It is also possible to set the unread body of
// a received response to the body of a request, with the same results.
//
// If the body is set from an in-memory reader such as [bytes.Buffer],
// [bytes.Reader], or [strings.Reader], the runtime will send the
// request with a Content-Length header instead of Transfer-Encoding:
// chunked.
func (req *Request) SetBody(body io.Reader) {
Comment on lines +223 to +227
Copy link
Collaborator

@cee-dub cee-dub Mar 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit of a learning question here, but is it possible to have the case of an incoming request with a content-length brought to this body assignment as well?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realized this isn't super clear, but I think Clone already handles this by copying the header if present.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but the runtime will remove Content-Length and Transfer-Encoding before sending the request unless ManualFramingMode is true.

rc, ok := body.(io.ReadCloser)
if !ok && body != nil {
rc = nopCloser{body}
}

req.Body = rc
}

// Clone returns a copy of the request. The returned copy will have a nil Body
// field, and it's URL will have a nil User field.
// field, and its URL will have a nil User field.
func (req *Request) Clone() *Request {
return &Request{
Method: req.Method,
Expand All @@ -230,6 +254,14 @@ func (req *Request) Clone() *Request {
}
}

// CloneWithBody returns a copy of the request, with the Body field set
// to the provided io.Reader. Its URL will have a nil User field.
func (req *Request) CloneWithBody(body io.Reader) *Request {
Comment on lines +257 to +259
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this could be removed now, perhaps by including the two lines of code into the doc of SetBody.

r := req.Clone()
r.SetBody(body)
return r
}

func cloneURL(u *url.URL) *url.URL {
return &url.URL{
Scheme: u.Scheme,
Expand Down Expand Up @@ -555,18 +587,6 @@ func abiBodyFrom(rc io.ReadCloser) (*fastly.HTTPBody, error) {
return b, nil
}

func makeBodyFor(r io.Reader) io.ReadCloser {
if r == nil {
return nil
}

if b, ok := r.(*fastly.HTTPBody); ok {
return b
}

return nopCloser{r}
}

func safePollInterval(d time.Duration) time.Duration {
const (
min = 1 * time.Millisecond
Expand Down
Loading