From f8ceb77e745b8f6d4a12bbd87158cb9474df8117 Mon Sep 17 00:00:00 2001 From: Varun Malhotra Date: Wed, 5 Apr 2023 12:14:18 -0700 Subject: [PATCH] invoke cancel function after response has been fully read Context: We were using v2@2.11 version of okta sdk and using ListUsers and few other APIs. However, those APIs started returning no response after upgrade to v2@2.14 version of SDK. Summary of the issue: When caller passes timeout parameter for these APIs, sdk invokes context cancel function before response has been fully read causing truncation of response. This was not getting caught as io.ReadAll error return was getting ignored. context cancel function needs to be called after all io.ReadAll invocations on http response body. ref: https://groups.google.com/g/golang-nuts/c/2FKwG6oEvos --- okta/requestExecutor.go | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/okta/requestExecutor.go b/okta/requestExecutor.go index 230566b0a..dba4c567f 100644 --- a/okta/requestExecutor.go +++ b/okta/requestExecutor.go @@ -456,6 +456,14 @@ func (re *RequestExecutor) Do(ctx context.Context, req *http.Request, v interfac re.freshCache = false } if !inCache { + if re.config.Okta.Client.RequestTimeout > 0 { + // ref: https://groups.google.com/g/golang-nuts/c/2FKwG6oEvos + // this cancel function needs be called after response has been fully + // read (io.ReadAll -- in buildResponse) + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, time.Second*time.Duration(re.config.Okta.Client.RequestTimeout)) + defer cancel() + } resp, err := re.doWithRetries(ctx, req) if err != nil { return nil, err @@ -507,11 +515,6 @@ func (re *RequestExecutor) doWithRetries(ctx context.Context, req *http.Request) resp *http.Response err error ) - if re.config.Okta.Client.RequestTimeout > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, time.Second*time.Duration(re.config.Okta.Client.RequestTimeout)) - defer cancel() - } bOff := &oktaBackoff{ ctx: ctx, maxRetries: re.config.Okta.Client.RateLimit.MaxRetries, @@ -649,7 +652,10 @@ func CheckResponseForError(resp *http.Response) error { } } } - bodyBytes, _ := io.ReadAll(resp.Body) + bodyBytes, err := io.ReadAll(resp.Body) + if err != nil { + return err + } copyBodyBytes := make([]byte, len(bodyBytes)) copy(copyBodyBytes, bodyBytes) _ = resp.Body.Close() @@ -668,7 +674,10 @@ func buildResponse(resp *http.Response, re *RequestExecutor, v interface{}) (*Re if err != nil { return response, err } - bodyBytes, _ := io.ReadAll(resp.Body) + bodyBytes, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } copyBodyBytes := make([]byte, len(bodyBytes)) copy(copyBodyBytes, bodyBytes) _ = resp.Body.Close() // close it to avoid memory leaks