Skip to content

Commit d40a1a6

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 ccfd649 commit d40a1a6

4 files changed

Lines changed: 53 additions & 12 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
})

go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ require (
7777
github.com/openzipkin/zipkin-go v0.4.3 // indirect
7878
github.com/spf13/pflag v1.0.9 // indirect
7979
github.com/x448/float16 v0.8.4 // indirect
80+
go.opentelemetry.io/otel v1.45.0 // indirect
81+
go.opentelemetry.io/otel/sdk/metric v1.45.0 // indirect
8082
go.yaml.in/yaml/v2 v2.4.3 // indirect
8183
go.yaml.in/yaml/v3 v3.0.5 // indirect
8284
golang.org/x/mod v0.38.0 // indirect

go.sum

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -277,16 +277,16 @@ github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9dec
277277
github.com/yuin/goldmark v1.4.1/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
278278
go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64=
279279
go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y=
280-
go.opentelemetry.io/otel v1.43.0 h1:mYIM03dnh5zfN7HautFE4ieIig9amkNANT+xcVxAj9I=
281-
go.opentelemetry.io/otel v1.43.0/go.mod h1:JuG+u74mvjvcm8vj8pI5XiHy1zDeoCS2LB1spIq7Ay0=
282-
go.opentelemetry.io/otel/metric v1.43.0 h1:d7638QeInOnuwOONPp4JAOGfbCEpYb+K6DVWvdxGzgM=
283-
go.opentelemetry.io/otel/metric v1.43.0/go.mod h1:RDnPtIxvqlgO8GRW18W6Z/4P462ldprJtfxHxyKd2PY=
284-
go.opentelemetry.io/otel/sdk v1.43.0 h1:pi5mE86i5rTeLXqoF/hhiBtUNcrAGHLKQdhg4h4V9Dg=
285-
go.opentelemetry.io/otel/sdk v1.43.0/go.mod h1:P+IkVU3iWukmiit/Yf9AWvpyRDlUeBaRg6Y+C58QHzg=
286-
go.opentelemetry.io/otel/sdk/metric v1.43.0 h1:S88dyqXjJkuBNLeMcVPRFXpRw2fuwdvfCGLEo89fDkw=
287-
go.opentelemetry.io/otel/sdk/metric v1.43.0/go.mod h1:C/RJtwSEJ5hzTiUz5pXF1kILHStzb9zFlIEe85bhj6A=
288-
go.opentelemetry.io/otel/trace v1.43.0 h1:BkNrHpup+4k4w+ZZ86CZoHHEkohws8AY+WTX09nk+3A=
289-
go.opentelemetry.io/otel/trace v1.43.0/go.mod h1:/QJhyVBUUswCphDVxq+8mld+AvhXZLhe+8WVFxiFff0=
280+
go.opentelemetry.io/otel v1.45.0 h1:pdrWmLHofpubmArBv1LgFSv1Z0Ie/ppdZzu+kUN5EeU=
281+
go.opentelemetry.io/otel v1.45.0/go.mod h1:XZxIqPapzEYnhNSScF5DIqXhm/rYi0FzCe2XddAwZfQ=
282+
go.opentelemetry.io/otel/metric v1.45.0 h1:7Eg1uH7CJ5cXv9is6tnBe1FI6rj1nwUdbFypRm3br/M=
283+
go.opentelemetry.io/otel/metric v1.45.0/go.mod h1:HAPbm1nd3p1PmFH7v2dR+6BjXxw+Lq4a2+pndMAm08s=
284+
go.opentelemetry.io/otel/sdk v1.45.0 h1:4VVSMgQ83dUgW2aoX5f6JgLvHwIvzcuLnF9lUdCSpCw=
285+
go.opentelemetry.io/otel/sdk v1.45.0/go.mod h1:Sr40LgXV7DsKMMJMKOhUWOgMWTfAaqvm2kF0g7ilwuA=
286+
go.opentelemetry.io/otel/sdk/metric v1.45.0 h1:oVFszMfyj1Am6s24Vtc7wBb8BKLcwepJjNEYILuiE3o=
287+
go.opentelemetry.io/otel/sdk/metric v1.45.0/go.mod h1:vUWUxDZvu1WVRj8JA8S0AdhsPrZoDpA2DdZauIh4mDA=
288+
go.opentelemetry.io/otel/trace v1.45.0 h1:l/mP6Uv7oNO7/TblbhpbgMidxhq1uO/rPsikOyVhxag=
289+
go.opentelemetry.io/otel/trace v1.45.0/go.mod h1:qoJJA2xNMnxRrdISU/kLtfUH2wNeQbiv+jhs/CxI8bc=
290290
go.step.sm/crypto v0.85.0 h1:h3U9gzCcxP1fWbseqy+CwsTxzHFNuoblefy7YP4jBHk=
291291
go.step.sm/crypto v0.85.0/go.mod h1:Rp/BdP0/ZJ7eYREhXcE4izCABLp0e8ziFqb0LBNG7Cc=
292292
go.yaml.in/yaml/v2 v2.4.3 h1:6gvOSjQoTB3vt1l+CU+tSyi/HOjfOjRLJ4YwYZGwRO0=

0 commit comments

Comments
 (0)