Skip to content

Commit

Permalink
fix: gorouter returns 400 when instance id does not exist
Browse files Browse the repository at this point in the history
[#170191495](https://www.pivotaltracker.com/story/show/170191495)

Co-authored-by: Aidan Obley <aobley@pivotal.io>
  • Loading branch information
Clay Kauzlaric and Aidan Obley committed Dec 13, 2019
1 parent 730a988 commit 02af2ca
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 10 deletions.
21 changes: 13 additions & 8 deletions handlers/lookup.go
Expand Up @@ -127,14 +127,17 @@ func (l *lookupHandler) handleMissingRoute(rw http.ResponseWriter, r *http.Reque
addNoCacheControlHeader(rw) addNoCacheControlHeader(rw)


errorMsg := fmt.Sprintf("Requested route ('%s') does not exist.", r.Host) errorMsg := fmt.Sprintf("Requested route ('%s') does not exist.", r.Host)
returnStatus := http.StatusNotFound

if appInstanceHeader := r.Header.Get(router_http.CfAppInstance); appInstanceHeader != "" { if appInstanceHeader := r.Header.Get(router_http.CfAppInstance); appInstanceHeader != "" {
guid, idx, _ := validateAndSplitInstanceHeader(appInstanceHeader) guid, idx := splitInstanceHeader(appInstanceHeader)
errorMsg = fmt.Sprintf("Requested instance ('%s') with guid ('%s') does not exist for route ('%s')", idx, guid, r.Host) errorMsg = fmt.Sprintf("Requested instance ('%s') with guid ('%s') does not exist for route ('%s')", idx, guid, r.Host)
returnStatus = http.StatusBadRequest
} }


writeStatus( writeStatus(
rw, rw,
http.StatusNotFound, returnStatus,
errorMsg, errorMsg,
l.logger, l.logger,
) )
Expand Down Expand Up @@ -173,27 +176,29 @@ func (l *lookupHandler) lookup(r *http.Request) (*route.EndpointPool, error) {
appInstanceHeader := r.Header.Get(router_http.CfAppInstance) appInstanceHeader := r.Header.Get(router_http.CfAppInstance)


if appInstanceHeader != "" { if appInstanceHeader != "" {
appID, appIndex, err := validateAndSplitInstanceHeader(appInstanceHeader) err := validateInstanceHeader(appInstanceHeader)

if err != nil { if err != nil {
l.logger.Error("invalid-app-instance-header", zap.Error(err)) l.logger.Error("invalid-app-instance-header", zap.Error(err))
return nil, InvalidInstanceHeaderError{headerValue: appInstanceHeader} return nil, InvalidInstanceHeaderError{headerValue: appInstanceHeader}
} }


appID, appIndex := splitInstanceHeader(appInstanceHeader)
return l.registry.LookupWithInstance(uri, appID, appIndex), nil return l.registry.LookupWithInstance(uri, appID, appIndex), nil
} }


return l.registry.Lookup(uri), nil return l.registry.Lookup(uri), nil
} }


func validateAndSplitInstanceHeader(appInstanceHeader string) (string, string, error) { func validateInstanceHeader(appInstanceHeader string) error {
// Regex to match format of `APP_GUID:INSTANCE_ID` // Regex to match format of `APP_GUID:INSTANCE_ID`
r := regexp.MustCompile(`^[\da-f]{8}-([\da-f]{4}-){3}[\da-f]{12}:\d+$`) r := regexp.MustCompile(`^[\da-f]{8}-([\da-f]{4}-){3}[\da-f]{12}:\d+$`)
if !r.MatchString(appInstanceHeader) { if !r.MatchString(appInstanceHeader) {
return "", "", fmt.Errorf("Incorrect %s header : %s", CfAppInstance, appInstanceHeader) return fmt.Errorf("Incorrect %s header : %s", CfAppInstance, appInstanceHeader)
} }
return nil
}


func splitInstanceHeader(appInstanceHeader string) (string, string) {
appDetails := strings.Split(appInstanceHeader, ":") appDetails := strings.Split(appInstanceHeader, ":")
return appDetails[0], appDetails[1], nil return appDetails[0], appDetails[1]

} }
4 changes: 2 additions & 2 deletions handlers/lookup_test.go
Expand Up @@ -154,9 +154,9 @@ var _ = Describe("Lookup", func() {
Expect(resp.Header().Get("Cache-Control")).To(Equal("no-cache, no-store")) Expect(resp.Header().Get("Cache-Control")).To(Equal("no-cache, no-store"))
}) })


It("returns a 404 NotFound and does not call next", func() { It("returns a 400 BadRequest and does not call next", func() {
Expect(nextCalled).To(BeFalse()) Expect(nextCalled).To(BeFalse())
Expect(resp.Code).To(Equal(http.StatusNotFound)) Expect(resp.Code).To(Equal(http.StatusBadRequest))
}) })


It("has a meaningful response", func() { It("has a meaningful response", func() {
Expand Down

0 comments on commit 02af2ca

Please sign in to comment.