Skip to content

Commit 44d0f78

Browse files
committed
Fix(curl): surface auth errors and avoid panic on nil HTTP response
MakeCurlRequest assumed that a non-nil error always came with an HTTP response (e.g. a 4xx/5xx from the API). That assumption breaks when the request fails before reaching the API — for example a token refresh / client-credentials authentication failure in the UAA request wrapper, which returns an error with a nil *http.Response. In that case: - Without --fail, the error was swallowed and cf curl printed empty output, giving the user no indication anything went wrong. - With --fail, the code dereferenced httpResponse.StatusCode on the nil response and panicked. Only surface CurlExit22Error when an HTTP response is actually present; when the response is nil, return the underlying error directly so the user sees a real message (e.g. "Bad credentials") and cf curl exits cleanly instead of panicking. Existing behavior for real HTTP error responses (print body by default, fail only with --fail) is unchanged. Signed-off-by: Prem Kumar Kalle <prem.kalle@broadcom.com>
1 parent 742387c commit 44d0f78

2 files changed

Lines changed: 41 additions & 2 deletions

File tree

actor/v7action/curl.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,19 @@ func (actor Actor) MakeCurlRequest(
4747
requestBodyBytes,
4848
)
4949

50-
if err != nil && failOnHTTPError {
51-
return nil, nil, translatableerror.CurlExit22Error{StatusCode: httpResponse.StatusCode}
50+
if err != nil {
51+
// A nil HTTP response means the request never reached the API (e.g. a token
52+
// refresh or other authentication failure in the request wrapper). There is no
53+
// status code to read and no response body to print, so surface the error
54+
// directly. This also avoids a nil-pointer dereference on httpResponse below
55+
// when the fail-on-http-error flag is set.
56+
if httpResponse == nil {
57+
return nil, nil, err
58+
}
59+
60+
if failOnHTTPError {
61+
return nil, nil, translatableerror.CurlExit22Error{StatusCode: httpResponse.StatusCode}
62+
}
5263
}
5364

5465
return responseBody, httpResponse, nil

actor/v7action/curl_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,5 +247,33 @@ var _ = Describe("Curl Actions", func() {
247247
})
248248
})
249249
})
250+
251+
When("the request fails before an HTTP response is received", func() {
252+
// e.g. a token refresh / authentication failure in the request wrapper, where
253+
// no request reaches the API and there is no HTTP response.
254+
BeforeEach(func() {
255+
mockErr = errors.New("Bad credentials")
256+
mockResponseBody = nil
257+
mockHTTPResponse = nil
258+
})
259+
260+
It("surfaces the error instead of returning empty output", func() {
261+
Expect(executeErr).To(MatchError("Bad credentials"))
262+
Expect(responseBody).To(BeNil())
263+
Expect(httpResponse).To(BeNil())
264+
})
265+
266+
When("the fail-on-http-errors flag is set", func() {
267+
BeforeEach(func() {
268+
failOnHTTPError = true
269+
})
270+
271+
It("surfaces the error without panicking on the nil response", func() {
272+
Expect(executeErr).To(MatchError("Bad credentials"))
273+
Expect(responseBody).To(BeNil())
274+
Expect(httpResponse).To(BeNil())
275+
})
276+
})
277+
})
250278
})
251279
})

0 commit comments

Comments
 (0)