fix #1531 and introduce the 'Configuration.ResetOnFireErrorCode' (read HISTORY.md)

Former-commit-id: 84f1e894378a6dfd94e0bf057f4037e35aee0c4f

core/router/api_builder.go

@@ -984,109 +984,11 @@ func (api *APIBuilder) OnErrorCode(statusCode int, handlers ...context.Handler)
 	return
 }
 
-// ClientErrorCodes holds the 4xx Client errors.
-var (
-	ClientErrorCodes = []int{
-		http.StatusBadRequest,
-		http.StatusUnauthorized,
-		http.StatusPaymentRequired,
-		http.StatusForbidden,
-		http.StatusNotFound,
-		http.StatusMethodNotAllowed,
-		http.StatusNotAcceptable,
-		http.StatusProxyAuthRequired,
-		http.StatusRequestTimeout,
-		http.StatusConflict,
-		http.StatusGone,
-		http.StatusLengthRequired,
-		http.StatusPreconditionFailed,
-		http.StatusRequestEntityTooLarge,
-		http.StatusRequestURITooLong,
-		http.StatusUnsupportedMediaType,
-		http.StatusRequestedRangeNotSatisfiable,
-		http.StatusExpectationFailed,
-		http.StatusTeapot,
-		http.StatusMisdirectedRequest,
-		http.StatusUnprocessableEntity,
-		http.StatusLocked,
-		http.StatusFailedDependency,
-		http.StatusTooEarly,
-		http.StatusUpgradeRequired,
-		http.StatusPreconditionRequired,
-		http.StatusTooManyRequests,
-		http.StatusRequestHeaderFieldsTooLarge,
-		http.StatusUnavailableForLegalReasons,
-		// Unofficial.
-		StatusPageExpired,
-		StatusBlockedByWindowsParentalControls,
-		StatusInvalidToken,
-		StatusTokenRequired,
-	}
-	// ServerErrorCodes holds the 5xx Server errors.
-	ServerErrorCodes = []int{
-		http.StatusInternalServerError,
-		http.StatusNotImplemented,
-		http.StatusBadGateway,
-		http.StatusServiceUnavailable,
-		http.StatusGatewayTimeout,
-		http.StatusHTTPVersionNotSupported,
-		http.StatusVariantAlsoNegotiates,
-		http.StatusInsufficientStorage,
-		http.StatusLoopDetected,
-		http.StatusNotExtended,
-		http.StatusNetworkAuthenticationRequired,
-		// Unofficial.
-		StatusBandwidthLimitExceeded,
-		StatusInvalidSSLCertificate,
-		StatusSiteOverloaded,
-		StatusSiteFrozen,
-		StatusNetworkReadTimeout,
-	}
-)
-
-// Unofficial status error codes.
-const (
-	// 4xx
-	StatusPageExpired                      = 419
-	StatusBlockedByWindowsParentalControls = 450
-	StatusInvalidToken                     = 498
-	StatusTokenRequired                    = 499
-	// 5xx
-	StatusBandwidthLimitExceeded = 509
-	StatusInvalidSSLCertificate  = 526
-	StatusSiteOverloaded         = 529
-	StatusSiteFrozen             = 530
-	StatusNetworkReadTimeout     = 598
-)
-
-var unofficialStatusText = map[int]string{
-	StatusPageExpired:                      "Page Expired",
-	StatusBlockedByWindowsParentalControls: "Blocked by Windows Parental Controls",
-	StatusInvalidToken:                     "Invalid Token",
-	StatusTokenRequired:                    "Token Required",
-	StatusBandwidthLimitExceeded:           "Bandwidth Limit Exceeded",
-	StatusInvalidSSLCertificate:            "Invalid SSL Certificate",
-	StatusSiteOverloaded:                   "Site is overloaded",
-	StatusSiteFrozen:                       "Site is frozen",
-	StatusNetworkReadTimeout:               "Network read timeout error",
-}
-
-// StatusText returns a text for the HTTP status code. It returns the empty
-// string if the code is unknown.
-func StatusText(code int) string {
-	text := http.StatusText(code)
-	if text == "" {
-		text = unofficialStatusText[code]
-	}
-
-	return text
-}
-
 // OnAnyErrorCode registers a handlers chain for all error codes
-// (4xxx and 5xxx, change the `ClientErrorCodes` and `ServerErrorCodes` variables to modify those)
+// (4xxx and 5xxx, change the `context.ClientErrorCodes` and `context.ServerErrorCodes` variables to modify those)
 // Look `OnErrorCode` too.
 func (api *APIBuilder) OnAnyErrorCode(handlers ...context.Handler) (routes []*Route) {
-	for _, statusCode := range append(ClientErrorCodes, ServerErrorCodes...) {
+	for _, statusCode := range context.ClientAndServerErrorCodes {
 		routes = append(routes, api.OnErrorCode(statusCode, handlers...)...)
 	}
 

core/router/handler.go

@@ -35,6 +35,8 @@ type (
 	// HTTPErrorHandler should contain a method `FireErrorCode` which
 	// handles http unsuccessful status codes.
 	HTTPErrorHandler interface {
+		// FireErrorCode should send an error response to the client based
+		// on the given context's response status code.
 		FireErrorCode(ctx context.Context)
 	}
 )
@@ -437,27 +439,55 @@ func (h *routerHandler) HandleRequest(ctx context.Context) {
 	ctx.StatusCode(http.StatusNotFound)
 }
 
+func statusCodeSuccessful(statusCode int) bool {
+	return !context.StatusCodeNotSuccessful(statusCode)
+}
+
+// FireErrorCode handles the response's error response.
+// If `Configuration.ResetOnFireErrorCode()` is true
+// and the response writer was a recorder or a gzip writer one
+// then it will try to reset the headers and the body before calling the
+// registered (or default) error handler for that error code set by
+// `ctx.StatusCode` method.
 func (h *routerHandler) FireErrorCode(ctx context.Context) {
+	// On common response writer, always check
+	// if we can't reset the body and the body has been filled
+	// which means that the status code already sent,
+	// then do not fire this custom error code,
+	// rel: context/context.go#EndRequest.
+	//
+	// Note that, this is set to 0 on recorder and gzip writer because they cache the response,
+	// so we check their len(Body()) instead, look below.
+	if ctx.ResponseWriter().Written() > 0 {
+		return
+	}
+
 	statusCode := ctx.GetStatusCode() // the response's cached one.
 
-	// if we can reset the body
-	if w, ok := ctx.IsRecording(); ok {
-		if statusCodeSuccessful(w.StatusCode()) { // if not an error status code
-			w.WriteHeader(statusCode) // then set it manually here, otherwise it should be set via ctx.StatusCode(...)
+	if ctx.Application().ConfigurationReadOnly().GetResetOnFireErrorCode() /* could be an argument too but we must not break the method */ {
+		// if we can reset the body, probably manual call of `Application.FireErrorCode`.
+		if w, ok := ctx.IsRecording(); ok {
+			if statusCodeSuccessful(w.StatusCode()) { // if not an error status code
+				w.WriteHeader(statusCode) // then set it manually here, otherwise it should be set via ctx.StatusCode(...)
+			}
+			// reset if previous content and it's recorder, keep the status code.
+			w.ClearHeaders()
+			w.ResetBody()
+		} else if w, ok := ctx.ResponseWriter().(*context.GzipResponseWriter); ok {
+			// reset and disable the gzip in order to be an expected form of http error result
+			w.ResetBody()
+			w.Disable()
 		}
-		// reset if previous content and it's recorder, keep the status code.
-		w.ClearHeaders()
-		w.ResetBody()
-	} else if w, ok := ctx.ResponseWriter().(*context.GzipResponseWriter); ok {
-		// reset and disable the gzip in order to be an expected form of http error result
-		w.ResetBody()
-		w.Disable()
 	} else {
-		// if we can't reset the body and the body has been filled
-		// which means that the status code already sent,
-		// then do not fire this custom error code.
-		if ctx.ResponseWriter().Written() > 0 { // != -1, rel: context/context.go#EndRequest
-			return
+		// check if a body already set (the error response is handled by the handler itself, see `Context.EndRequest`)
+		if w, ok := ctx.IsRecording(); ok {
+			if len(w.Body()) > 0 {
+				return
+			}
+		} else if w, ok := ctx.ResponseWriter().(*context.GzipResponseWriter); ok {
+			if len(w.Body()) > 0 {
+				return
+			}
 		}
 	}
 
@@ -523,11 +553,7 @@ func (h *routerHandler) FireErrorCode(ctx context.Context) {
 	}
 
 	// not error handler found, write a default message.
-	ctx.WriteString(StatusText(statusCode))
-}
-
-func statusCodeSuccessful(statusCode int) bool {
-	return !context.StatusCodeNotSuccessful(statusCode)
+	ctx.WriteString(context.StatusText(statusCode))
 }
 
 func (h *routerHandler) subdomainAndPathAndMethodExists(ctx context.Context, t *trie, method, path string) bool {

core/router/status_test.go

@@ -35,11 +35,11 @@ func TestOnAnyErrorCode(t *testing.T) {
 		ctx.WriteString(expectedFoundResponse)
 	})
 
-	app.Get("/406", func(ctx context.Context) {
+	expected407 := "this should be sent, we manage the response response by ourselves"
+	app.Get("/407", func(ctx context.Context) {
 		ctx.Record()
-		ctx.WriteString("this should not be sent, only status text will be sent")
-		ctx.WriteString("the handler can handle 'rollback' of the text when error code fired because of the recorder")
-		ctx.StatusCode(iris.StatusNotAcceptable)
+		ctx.WriteString(expected407)
+		ctx.StatusCode(iris.StatusProxyAuthRequired)
 	})
 
 	e := httptest.New(t, app)
@@ -57,7 +57,26 @@ func TestOnAnyErrorCode(t *testing.T) {
 
 	checkAndClearBuf(t, buff, expectedPrintBeforeExecuteErr)
 
-	e.GET("/406").Expect().Status(iris.StatusNotAcceptable).
+	e.GET("/407").Expect().Status(iris.StatusProxyAuthRequired).
+		Body().Equal(expected407)
+
+	// Test Configuration.ResetOnFireErrorCode.
+	app2 := iris.New()
+	app2.Configure(iris.WithResetOnFireErrorCode)
+
+	app2.OnAnyErrorCode(func(ctx context.Context) {
+		buff.WriteString(expectedPrintBeforeExecuteErr)
+		ctx.Next()
+	}, defaultErrHandler)
+
+	app2.Get("/406", func(ctx context.Context) {
+		ctx.Record()
+		ctx.WriteString("this should not be sent, only status text will be sent")
+		ctx.WriteString("the handler can handle 'rollback' of the text when error code fired because of the recorder")
+		ctx.StatusCode(iris.StatusNotAcceptable)
+	})
+
+	httptest.New(t, app2).GET("/406").Expect().Status(iris.StatusNotAcceptable).
 		Body().Equal(http.StatusText(iris.StatusNotAcceptable))
 
 	checkAndClearBuf(t, buff, expectedPrintBeforeExecuteErr)