Skip to content

Commit ab121d7

Browse files
committed
address review comments
1 parent 89e673b commit ab121d7

File tree

1 file changed

+12
-8
lines changed

1 file changed

+12
-8
lines changed

utils/http/proxy/proxy.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ func ProxyRequest(r *http.Request, proxyMethod, endpoint string) (proxiedRequest
3434
return
3535
}
3636
ctx := r.Context()
37+
// Note: It is important to know that an 0 or -1 content length does not mean there is no body. This is likely the case but it could also be because the body was never read and its size never assessed.
3738
contentLength := determineRequestContentLength(r)
3839
h := httpheaders.FromRequest(r).AllowList(headers.Authorization)
3940
if reflection.IsEmpty(proxyMethod) {
@@ -48,25 +49,28 @@ func ProxyRequest(r *http.Request, proxyMethod, endpoint string) (proxiedRequest
4849
if proxiedRequest.ContentLength <= 0 {
4950
if proxiedRequest.Body == nil || proxiedRequest.Body == http.NoBody {
5051
if contentLength > 0 {
52+
// In this case, NewRequestWithContext does not understand/expect the request body type (not a string/byte buffer as it may be wrapped into a bigger structure) and so, the body of the proxied request is set to nil
53+
// This makes sure this does not happen without performing a copy of the body and the use of unnecessary memory.
5154
proxiedRequest.Body = r.Body
5255
proxiedRequest.GetBody = r.GetBody
5356
} else {
57+
// In this case, it will attempt a copy of the request body which should not be costly as the request is unlikely to have a body. Although it may still do as contentlength may not have actually been evaluated. However, we want to make sure it is set to the same type as the original request.
5458
proxiedRequest, err = http.NewRequestWithContext(ctx, proxyMethod, endpoint, convertBody(ctx, r.Body))
5559
if err != nil {
5660
err = commonerrors.WrapError(commonerrors.ErrUnexpected, err, "could not create a proxied request")
5761
return
5862
}
5963
}
60-
}
61-
62-
if contentLength <= 0 {
63-
proxiedRequest, err = http.NewRequestWithContext(ctx, proxyMethod, endpoint, convertBody(ctx, r.Body))
64-
if err != nil {
65-
err = commonerrors.WrapError(commonerrors.ErrUnexpected, err, "could not create a proxied request")
66-
return
64+
} else {
65+
// In this case, the original request is unlikely to have a body but we want to make sure that the body is of the same type.
66+
if contentLength <= 0 {
67+
proxiedRequest, err = http.NewRequestWithContext(ctx, proxyMethod, endpoint, convertBody(ctx, r.Body))
68+
if err != nil {
69+
err = commonerrors.WrapError(commonerrors.ErrUnexpected, err, "could not create a proxied request")
70+
return
71+
}
6772
}
6873
}
69-
7074
if contentLength > 0 && proxiedRequest.ContentLength <= 0 {
7175
proxiedRequest.ContentLength = contentLength
7276
h.AppendHeader(headers.ContentLength, strconv.FormatInt(contentLength, 10))

0 commit comments

Comments
 (0)