Skip to content

Commit e5be29f

Browse files
committed
Fix bugs flagged in routine code review
- fixes lack of nil checks in retry - fixes possibility of nil body on PATCH/PUT/POST on retry - added couple of missing nil checks in http.go - fixed handling of malformed input
1 parent c6f548f commit e5be29f

3 files changed

Lines changed: 28 additions & 8 deletions

File tree

internal/cmd/http.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func (c *HttpCommand) http() error {
4444
url,
4545
getData(result.body))
4646
if err != nil {
47-
log.Errorf("Request error: %v\n", err)
47+
log.Fatalf("Request error: %v\n", err)
4848
}
4949
req.Header = result.headers
5050
if len(*result.urlValues) > 0 {

internal/env/run.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,10 @@ func call(m *parser.Method, err error) error {
7777
var any common.Input
7878
for err == nil {
7979
err = input.Decode(&any)
80-
if err == io.EOF {
80+
if err != nil {
81+
if err == io.EOF {
82+
err = nil
83+
}
8184
break
8285
}
8386
err = env.Exec(m, &any)

internal/utils/retry_cli.go

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,20 @@ func (c *Client) RetriableDo(req *http.Request) (*http.Response, error) {
8080
logger.HttpRequest(req)
8181
var i int64
8282
for {
83+
if i > 0 && req.GetBody != nil {
84+
body, err := req.GetBody()
85+
if err != nil {
86+
return nil, err
87+
}
88+
req.Body = body
89+
}
8390
// #nosec G704 -- request target is intentionally provided by CLI input.
8491
resp, err := c.HttpClient.Do(req)
8592
if c.RetryWithBackoff(logger, i, resp, err) {
8693
logger.Debugf("%d/%d\n", i, c.MaxRetry)
94+
if resp != nil && resp.Body != nil {
95+
_ = resp.Body.Close()
96+
}
8797
i++
8898
} else {
8999
return resp, err
@@ -97,23 +107,27 @@ func (c *Client) RetriableDo(req *http.Request) (*http.Response, error) {
97107
func (c *Client) RetryWithBackoff(logger *config.Log,
98108
i int64, resp *http.Response, err error) bool {
99109
doRetry := false
110+
statusCode := 0
100111
retrySeconds := time.Second * time.Duration(i)
112+
if resp != nil {
113+
statusCode = resp.StatusCode
114+
}
101115
if err != nil {
102116
logger.Errorf("error = %v, retriable.\n", err)
103117
doRetry = true
104-
} else if resp.StatusCode == 0 || resp.StatusCode > 500 {
105-
logger.Debugf("status = %d, retriable.\n", resp.StatusCode)
118+
} else if statusCode == 0 || statusCode >= 500 {
119+
logger.Debugf("status = %d, retriable.\n", statusCode)
106120
doRetry = true
107-
} else if resp.StatusCode == 429 {
121+
} else if statusCode == 429 {
108122
doRetry = true
109123
retrySeconds = time.Duration(getRetryAfterHeaderValue(resp))
110124
logger.Debugf(
111125
"status = %d, retry_after = %d seconds, retriable.\n",
112-
resp.StatusCode, retrySeconds)
113-
} else if c.StatusCode > 0 && resp.StatusCode != c.StatusCode {
126+
statusCode, retrySeconds)
127+
} else if c.StatusCode > 0 && statusCode != c.StatusCode {
114128
// allow for an override to wait for a specific code
115129
doRetry = true
116-
logger.Debugf("status = %d, retrying till %d.\n", resp.StatusCode, c.StatusCode)
130+
logger.Debugf("status = %d, retrying till %d.\n", statusCode, c.StatusCode)
117131
}
118132
if doRetry && i < c.MaxRetry {
119133
time.Sleep(retrySeconds)
@@ -126,6 +140,9 @@ func (c *Client) RetryWithBackoff(logger *config.Log,
126140
// default to DefaultRetryAfterSeconds if not present or invalid
127141
func getRetryAfterHeaderValue(resp *http.Response) time.Duration {
128142
retryAfter := DefaultRetryAfterSeconds
143+
if resp == nil {
144+
return retryAfter
145+
}
129146
var err error
130147
var i int
131148
if val, ok := resp.Header[RetryAfter]; ok {

0 commit comments

Comments
 (0)