Skip to content

Commit 654fe9b

Browse files
authored
fix: prevent scalingengine panic on WriteHeader(0) with Go 1.25 (#1183)
MapCFClientError was creating CfError without setting StatusCode, causing WriteHeader(0) panic when the scaling handler forwarded it. Now maps CF error codes to proper HTTP status codes and adds a defensive guard in the handler.
1 parent 4cfdaed commit 654fe9b

3 files changed

Lines changed: 104 additions & 2 deletions

File tree

cf/errors.go

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"encoding/json"
55
"errors"
66
"fmt"
7+
"net/http"
78
"strings"
89

910
"github.com/cloudfoundry/go-cfclient/v3/resource"
@@ -126,9 +127,21 @@ func MapCFClientError(err error) error {
126127
return nil
127128
}
128129

130+
var cfHTTPErr resource.CloudFoundryHTTPError
131+
if errors.As(err, &cfHTTPErr) {
132+
return &CfError{
133+
StatusCode: cfHTTPErr.StatusCode,
134+
Errors: []CfErrorItem{{
135+
Title: cfHTTPErr.Status,
136+
Detail: string(cfHTTPErr.Body),
137+
}},
138+
}
139+
}
140+
129141
var cfClientErr resource.CloudFoundryError
130142
if errors.As(err, &cfClientErr) {
131143
return &CfError{
144+
StatusCode: httpStatusFromCFCode(cfClientErr.Code),
132145
Errors: []CfErrorItem{{
133146
Code: cfClientErr.Code,
134147
Title: cfClientErr.Title,
@@ -140,15 +153,32 @@ func MapCFClientError(err error) error {
140153
var cfClientErrs resource.CloudFoundryErrors
141154
if errors.As(err, &cfClientErrs) {
142155
items := make([]CfErrorItem, len(cfClientErrs.Errors))
156+
statusCode := 0
143157
for i, e := range cfClientErrs.Errors {
144158
items[i] = CfErrorItem{
145159
Code: e.Code,
146160
Title: e.Title,
147161
Detail: e.Detail,
148162
}
163+
if statusCode == 0 {
164+
statusCode = httpStatusFromCFCode(e.Code)
165+
}
149166
}
150-
return &CfError{Errors: items}
167+
return &CfError{StatusCode: statusCode, Errors: items}
151168
}
152169

153170
return err
154171
}
172+
173+
func httpStatusFromCFCode(cfCode int) int {
174+
switch cfCode {
175+
case cfResourceNotFound:
176+
return http.StatusNotFound
177+
case cfNotAuthenticated:
178+
return http.StatusUnauthorized
179+
case cfNotAuthorised:
180+
return http.StatusForbidden
181+
default:
182+
return http.StatusInternalServerError
183+
}
184+
}

cf/errors_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ package cf_test
33
import (
44
"encoding/json"
55
"errors"
6+
"net/http"
67

78
"code.cloudfoundry.org/app-autoscaler/src/autoscaler/cf"
9+
"github.com/cloudfoundry/go-cfclient/v3/resource"
810

911
. "github.com/onsi/ginkgo/v2"
1012
. "github.com/onsi/gomega"
@@ -124,4 +126,69 @@ var _ = Describe("Errors test", func() {
124126
})
125127
})
126128
})
129+
130+
Context("MapCFClientError", func() {
131+
It("returns nil for nil error", func() {
132+
Expect(cf.MapCFClientError(nil)).To(BeNil())
133+
})
134+
135+
It("maps CloudFoundryError with not-found code to 404", func() {
136+
err := resource.CloudFoundryError{Code: 10010, Title: "CF-ResourceNotFound", Detail: "App not found"}
137+
result := cf.MapCFClientError(err)
138+
var cfErr *cf.CfError
139+
Expect(errors.As(result, &cfErr)).To(BeTrue())
140+
Expect(cfErr.StatusCode).To(Equal(http.StatusNotFound))
141+
Expect(cfErr.Errors[0].Code).To(Equal(10010))
142+
})
143+
144+
It("maps CloudFoundryError with not-authenticated code to 401", func() {
145+
err := resource.CloudFoundryError{Code: 10002, Title: "CF-NotAuthenticated", Detail: "No auth token"}
146+
result := cf.MapCFClientError(err)
147+
var cfErr *cf.CfError
148+
Expect(errors.As(result, &cfErr)).To(BeTrue())
149+
Expect(cfErr.StatusCode).To(Equal(http.StatusUnauthorized))
150+
})
151+
152+
It("maps CloudFoundryError with not-authorized code to 403", func() {
153+
err := resource.CloudFoundryError{Code: 10003, Title: "CF-NotAuthorized", Detail: "No permission"}
154+
result := cf.MapCFClientError(err)
155+
var cfErr *cf.CfError
156+
Expect(errors.As(result, &cfErr)).To(BeTrue())
157+
Expect(cfErr.StatusCode).To(Equal(http.StatusForbidden))
158+
})
159+
160+
It("maps unknown CloudFoundryError code to 500", func() {
161+
err := resource.CloudFoundryError{Code: 99999, Title: "CF-Unknown", Detail: "Something"}
162+
result := cf.MapCFClientError(err)
163+
var cfErr *cf.CfError
164+
Expect(errors.As(result, &cfErr)).To(BeTrue())
165+
Expect(cfErr.StatusCode).To(Equal(http.StatusInternalServerError))
166+
})
167+
168+
It("maps CloudFoundryHTTPError preserving status code", func() {
169+
err := resource.CloudFoundryHTTPError{StatusCode: 503, Status: "Service Unavailable", Body: []byte("down")}
170+
result := cf.MapCFClientError(err)
171+
var cfErr *cf.CfError
172+
Expect(errors.As(result, &cfErr)).To(BeTrue())
173+
Expect(cfErr.StatusCode).To(Equal(503))
174+
})
175+
176+
It("maps CloudFoundryErrors taking status from first error", func() {
177+
err := resource.CloudFoundryErrors{Errors: []resource.CloudFoundryError{
178+
{Code: 10010, Title: "CF-ResourceNotFound", Detail: "Not found"},
179+
{Code: 10002, Title: "CF-NotAuthenticated", Detail: "No auth"},
180+
}}
181+
result := cf.MapCFClientError(err)
182+
var cfErr *cf.CfError
183+
Expect(errors.As(result, &cfErr)).To(BeTrue())
184+
Expect(cfErr.StatusCode).To(Equal(http.StatusNotFound))
185+
Expect(cfErr.Errors).To(HaveLen(2))
186+
})
187+
188+
It("returns original error if not a CF client error", func() {
189+
err := errors.New("some random error")
190+
result := cf.MapCFClientError(err)
191+
Expect(result).To(Equal(err))
192+
})
193+
})
127194
})

scalingengine/server/scaling_handler.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,17 @@ func (h *ScalingHandler) Scale(w http.ResponseWriter, r *http.Request, vars map[
5151

5252
var cfApiClientErrTypeProxy *cf.CfError
5353
if errors.As(err, &cfApiClientErrTypeProxy) {
54+
statusCode := cfApiClientErrTypeProxy.StatusCode
55+
if statusCode == 0 {
56+
statusCode = http.StatusInternalServerError
57+
}
58+
5459
errorDescription, err := json.Marshal(cfApiClientErrTypeProxy)
5560
if err != nil {
5661
logger.Error("failed-to-serialize cf-api-error", err)
5762
}
5863

59-
handlers.WriteJSONResponse(w, cfApiClientErrTypeProxy.StatusCode, models.ErrorResponse{
64+
handlers.WriteJSONResponse(w, statusCode, models.ErrorResponse{
6065
Code: "Error on request to the cloud-controller via a cf-client",
6166
Message: fmt.Sprintf("Error taking scaling action:\n%s", string(errorDescription))})
6267
} else {

0 commit comments

Comments
 (0)