From 14de978cab7687c4e8719d2b50dea26ff8a97273 Mon Sep 17 00:00:00 2001 From: Jeevanandam M Date: Sun, 6 Aug 2017 10:11:33 -0700 Subject: [PATCH 01/20] version bump to v0.8-dev [ci skip] --- aah.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aah.go b/aah.go index a8148167..ede39c87 100644 --- a/aah.go +++ b/aah.go @@ -21,7 +21,7 @@ import ( ) // Version no. of aah framework -const Version = "0.7" +const Version = "0.8-dev" // aah application variables var ( From 04f695f69098456d01d095b841a51cb006717d83 Mon Sep 17 00:00:00 2001 From: Jeevanandam M Date: Mon, 7 Aug 2017 00:34:24 -0700 Subject: [PATCH 02/20] Content negotiation offered and accepted implementation #75 (#91) --- param.go | 31 +++++++++++++++++++++++++------ param_test.go | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/param.go b/param.go index 135cb91d..2616acc2 100644 --- a/param.go +++ b/param.go @@ -22,8 +22,12 @@ const ( ) var ( - keyQueryParamName = keyOverrideI18nName - keyPathParamName = keyOverrideI18nName + keyQueryParamName = keyOverrideI18nName + keyPathParamName = keyOverrideI18nName + isAcceptedExists bool + acceptedContentTypes []string + isOfferedExists bool + offeredContentTypes []string ) //‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ @@ -34,13 +38,25 @@ var ( // parameters (Payload, Form, Query, Multi-part) stores into context. Request // params are made available in View via template functions. func (e *engine) parseRequestParams(ctx *Context) flowResult { - req := ctx.Req.Unwrap() + // Content Negotitaion - Accepted, refer to Github #75 + if isAcceptedExists && !ess.IsSliceContainsString(acceptedContentTypes, ctx.Req.ContentType.Mime) { + log.Warnf("Content type '%v' not accepted by server", ctx.Req.ContentType.Mime) + writeErrorInfo(ctx, http.StatusUnsupportedMediaType, "Unsupported Media Type") + return flowStop + } + // Content Negotitaion - Offered, refer to Github #75 + if isOfferedExists && !ess.IsSliceContainsString(offeredContentTypes, ctx.Req.AcceptContentType.Mime) { + log.Warnf("Content type '%v' not offered by server", ctx.Req.AcceptContentType.Mime) + writeErrorInfo(ctx, http.StatusUnsupportedMediaType, "Not Acceptable") + return flowStop + } + + req := ctx.Req.Unwrap() if ctx.Req.Method != ahttp.MethodGet { contentType := ctx.Req.ContentType.Mime log.Debugf("Request content type: %s", contentType) - // TODO add support for content-type restriction and return 415 status for that // TODO HTML sanitizer for Form and Multipart Form switch contentType { @@ -117,8 +133,11 @@ func tmplQueryParam(viewArgs map[string]interface{}, key string) interface{} { } func paramInitialize(e *Event) { - keyPathParamName = AppConfig().StringDefault("i18n.param_name.path", keyOverrideI18nName) - keyQueryParamName = AppConfig().StringDefault("i18n.param_name.query", keyOverrideI18nName) + cfg := AppConfig() + keyPathParamName = cfg.StringDefault("i18n.param_name.path", keyOverrideI18nName) + keyQueryParamName = cfg.StringDefault("i18n.param_name.query", keyOverrideI18nName) + acceptedContentTypes, isAcceptedExists = cfg.StringList("request.content_negotiation.accepted") + offeredContentTypes, isOfferedExists = cfg.StringList("request.content_negotiation.offered") } func init() { diff --git a/param_test.go b/param_test.go index 9e40e1ee..17ac3196 100644 --- a/param_test.go +++ b/param_test.go @@ -98,7 +98,7 @@ func TestParamParseLocaleFromAppConfiguration(t *testing.T) { r := httptest.NewRequest("GET", "http://localhost:8080/index.html?language=en-CA", nil) ctx1 := &Context{ - Req: ahttp.ParseRequest(r, &ahttp.Request{}), + Req: ahttp.AcquireRequest(r), viewArgs: make(map[string]interface{}), } @@ -111,3 +111,35 @@ func TestParamParseLocaleFromAppConfiguration(t *testing.T) { assert.Equal(t, "CA", ctx1.Req.Locale.Region) assert.Equal(t, "en-CA", ctx1.Req.Locale.String()) } + +func TestParamContentNegotiation(t *testing.T) { + defer ess.DeleteFiles("testapp.pid") + + e := engine{} + + // Accepted + isAcceptedExists = true + acceptedContentTypes = []string{"application/json"} + r1 := httptest.NewRequest("POST", "http://localhost:8080/v1/userinfo", nil) + r1.Header.Set(ahttp.HeaderContentType, "application/xml") + ctx1 := &Context{ + Req: ahttp.AcquireRequest(r1), + reply: acquireReply(), + } + result1 := e.parseRequestParams(ctx1) + assert.True(t, result1 == 1) + isAcceptedExists = false + + // Offered + isOfferedExists = true + offeredContentTypes = []string{"application/json"} + r2 := httptest.NewRequest("POST", "http://localhost:8080/v1/userinfo", nil) + r2.Header.Set(ahttp.HeaderAccept, "application/xml") + ctx2 := &Context{ + Req: ahttp.AcquireRequest(r2), + reply: acquireReply(), + } + result2 := e.parseRequestParams(ctx2) + assert.True(t, result2 == 1) + isOfferedExists = false +} From bea13262a7567b2b65d29755781eea59ce7bb0b6 Mon Sep 17 00:00:00 2001 From: Jeevanandam M Date: Mon, 7 Aug 2017 11:03:21 -0700 Subject: [PATCH 03/20] #79 adding option of registering external JSON library (#93) --- render.go | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/render.go b/render.go index ffde4e7d..a56ae7a9 100644 --- a/render.go +++ b/render.go @@ -21,6 +21,15 @@ import ( ) var ( + // JSONMarshal is used to register external JSON library for Marshalling. + JSONMarshal func(v interface{}) ([]byte, error) + + // JSONUnmarshal is used to register external JSON library for Unmarshalling. + JSONUnmarshal func(data []byte, v interface{}) error + + // JSONMarshalIndent is used to register external JSON library for Marshal indent. + JSONMarshalIndent func(v interface{}, prefix, indent string) ([]byte, error) + xmlHeaderBytes = []byte(xml.Header) rdrHTMLPool = &sync.Pool{New: func() interface{} { return &HTML{} }} rdrJSONPool = &sync.Pool{New: func() interface{} { return &JSON{} }} @@ -96,9 +105,9 @@ func (j *JSON) Render(w io.Writer) error { ) if appConfig.BoolDefault("render.pretty", false) { - bytes, err = json.MarshalIndent(j.Data, "", " ") + bytes, err = JSONMarshalIndent(j.Data, "", " ") } else { - bytes, err = json.Marshal(j.Data) + bytes, err = JSONMarshal(j.Data) } if err != nil { @@ -248,6 +257,10 @@ func (h *HTML) reset() { h.ViewArgs = make(Data) } +//‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ +// Render Unexported methods +//___________________________________ + // doRender method renders and detects the errors earlier. Writes the // error info if any. func (e *engine) doRender(ctx *Context) { @@ -296,3 +309,10 @@ func releaseRender(r Render) { } } } + +func init() { + // Registering default standard JSON library + JSONMarshal = json.Marshal + JSONUnmarshal = json.Unmarshal + JSONMarshalIndent = json.MarshalIndent +} From afe68335ff462643ca48a5f400d14983583a5df2 Mon Sep 17 00:00:00 2001 From: Jeevanandam M Date: Sun, 6 Aug 2017 22:44:26 -0700 Subject: [PATCH 04/20] request max body size limit implementation #83 --- aah.go | 6 ++++++ context.go | 2 +- context_test.go | 2 +- controller.go | 2 +- controller_test.go | 2 +- param.go | 7 +++++++ param_test.go | 9 +++++++-- router.go | 2 +- security_test.go | 2 +- static_test.go | 2 +- util.go | 11 +++++++++++ view_test.go | 5 +++++ 12 files changed, 43 insertions(+), 9 deletions(-) diff --git a/aah.go b/aah.go index a8148167..00a6b3b2 100644 --- a/aah.go +++ b/aah.go @@ -40,6 +40,7 @@ var ( appIsLetsEncrypt bool appIsProfileProd bool appMultipartMaxMemory int64 + appMaxBodyBytesSize int64 appPID int appInitialized bool appBuildInfo *BuildInfo @@ -293,6 +294,11 @@ func initAppVariables() error { return err } + maxBodySizeStr := cfg.StringDefault("request.max_body_size", "5mb") + if appMaxBodyBytesSize, err = ess.StrToBytes(maxBodySizeStr); err != nil { + return errors.New("'request.max_body_size' value is not a valid size unit") + } + multipartMemoryStr := cfg.StringDefault("request.multipart_size", "32mb") if appMultipartMaxMemory, err = ess.StrToBytes(multipartMemoryStr); err != nil { return errors.New("'request.multipart_size' value is not a valid size unit") diff --git a/context.go b/context.go index 48de7deb..61b6ed61 100644 --- a/context.go +++ b/context.go @@ -13,7 +13,7 @@ import ( "aahframework.org/ahttp.v0" "aahframework.org/essentials.v0" "aahframework.org/log.v0" - "aahframework.org/router.v0" + "aahframework.org/router.v0-unstable" "aahframework.org/security.v0" "aahframework.org/security.v0/session" ) diff --git a/context_test.go b/context_test.go index 405999e6..59cb8b34 100644 --- a/context_test.go +++ b/context_test.go @@ -13,7 +13,7 @@ import ( "aahframework.org/ahttp.v0" "aahframework.org/config.v0" - "aahframework.org/router.v0" + "aahframework.org/router.v0-unstable" "aahframework.org/security.v0" "aahframework.org/test.v0/assert" ) diff --git a/controller.go b/controller.go index eccb5737..cfa1638e 100644 --- a/controller.go +++ b/controller.go @@ -10,7 +10,7 @@ import ( "strings" "aahframework.org/essentials.v0" - "aahframework.org/router.v0" + "aahframework.org/router.v0-unstable" ) const ( diff --git a/controller_test.go b/controller_test.go index d84c33eb..451a030c 100644 --- a/controller_test.go +++ b/controller_test.go @@ -7,7 +7,7 @@ package aah import ( "testing" - "aahframework.org/router.v0" + "aahframework.org/router.v0-unstable" "aahframework.org/test.v0/assert" ) diff --git a/param.go b/param.go index 135cb91d..bf8b6c41 100644 --- a/param.go +++ b/param.go @@ -40,6 +40,13 @@ func (e *engine) parseRequestParams(ctx *Context) flowResult { contentType := ctx.Req.ContentType.Mime log.Debugf("Request content type: %s", contentType) + // Prevent DDoS attacks by large HTTP request bodies by enforcing + // configured hard limit, Github #83. + if contentType != ahttp.ContentTypeMultipartForm.Mime { + req.Body = http.MaxBytesReader(ctx.Res, req.Body, + firstNonZeroInt64(ctx.route.MaxBodySize, appMaxBodyBytesSize)) + } + // TODO add support for content-type restriction and return 415 status for that // TODO HTML sanitizer for Form and Multipart Form diff --git a/param_test.go b/param_test.go index 9e40e1ee..4481dba3 100644 --- a/param_test.go +++ b/param_test.go @@ -14,6 +14,7 @@ import ( "aahframework.org/ahttp.v0" "aahframework.org/config.v0" "aahframework.org/essentials.v0" + "aahframework.org/router.v0-unstable" "aahframework.org/test.v0/assert" ) @@ -51,7 +52,9 @@ func TestParamParse(t *testing.T) { // Request Query String r1 := httptest.NewRequest("GET", "http://localhost:8080/index.html?lang=en-CA", nil) ctx1 := &Context{ - Req: ahttp.ParseRequest(r1, &ahttp.Request{}), + Req: ahttp.AcquireRequest(r1), + Res: ahttp.AcquireResponseWriter(httptest.NewRecorder()), + route: &router.Route{MaxBodySize: 5 << 20}, viewArgs: make(map[string]interface{}), } @@ -73,7 +76,9 @@ func TestParamParse(t *testing.T) { r2, _ := http.NewRequest("POST", "http://localhost:8080/user/registration", strings.NewReader(form.Encode())) r2.Header.Add(ahttp.HeaderContentType, ahttp.ContentTypeForm.String()) ctx2 := &Context{ - Req: ahttp.ParseRequest(r2, &ahttp.Request{}), + Req: ahttp.AcquireRequest(r2), + Res: ahttp.AcquireResponseWriter(httptest.NewRecorder()), + route: &router.Route{MaxBodySize: 5 << 20}, viewArgs: make(map[string]interface{}), } diff --git a/router.go b/router.go index 280ed188..4feb9487 100644 --- a/router.go +++ b/router.go @@ -17,7 +17,7 @@ import ( "aahframework.org/config.v0" "aahframework.org/essentials.v0" "aahframework.org/log.v0" - "aahframework.org/router.v0" + "aahframework.org/router.v0-unstable" ) var appRouter *router.Router diff --git a/security_test.go b/security_test.go index f659025c..7d46ce82 100644 --- a/security_test.go +++ b/security_test.go @@ -11,7 +11,7 @@ import ( "aahframework.org/ahttp.v0" "aahframework.org/config.v0" - "aahframework.org/router.v0" + "aahframework.org/router.v0-unstable" "aahframework.org/security.v0" "aahframework.org/security.v0/authc" "aahframework.org/security.v0/authz" diff --git a/static_test.go b/static_test.go index 84efb10c..35a972d4 100644 --- a/static_test.go +++ b/static_test.go @@ -15,7 +15,7 @@ import ( "testing" "aahframework.org/config.v0" - "aahframework.org/router.v0" + "aahframework.org/router.v0-unstable" "aahframework.org/test.v0/assert" ) diff --git a/util.go b/util.go index b8f8d1c2..fbb2c006 100644 --- a/util.go +++ b/util.go @@ -110,6 +110,17 @@ func firstNonEmpty(values ...string) string { return "" } +// TODO this method is candidate for essentials library +// move it when you get a time +func firstNonZeroInt64(values ...int64) int64 { + for _, v := range values { + if v != 0 { + return v + } + } + return 0 +} + func identifyContentType(ctx *Context) *ahttp.ContentType { // based on 'Accept' Header if !ess.IsStrEmpty(ctx.Req.AcceptContentType.Mime) && diff --git a/view_test.go b/view_test.go index 6967f4ad..0d17c899 100644 --- a/view_test.go +++ b/view_test.go @@ -155,8 +155,13 @@ func TestViewResolveView(t *testing.T) { assert.Equal(t, "index.html", htmlRdr.Filename) assert.Equal(t, "views/pages/frontend/app/index.html", htmlRdr.ViewArgs["ViewNotFound"]) + ctx.Req.AcceptContentType.Mime = "" + appConfig = appCfg + assert.Nil(t, identifyContentType(ctx)) + // cleanup appViewEngine = nil + appConfig = nil } func TestViewResolveViewNotFound(t *testing.T) { From b4bc8290b0d1d5591aa746fec2f861d2f064cdbb Mon Sep 17 00:00:00 2001 From: Jeevanandam M Date: Fri, 11 Aug 2017 11:46:17 -0700 Subject: [PATCH 05/20] Centralized error handling #57 (#95) --- access_log_test.go | 39 -------------- engine.go | 69 ++++++++++++++---------- engine_test.go | 22 +++++--- error.go | 127 ++++++++++++++++++++++++++++++++++++++++----- error_test.go | 75 ++++++++++++++++++++++---- param.go | 15 ++++-- param_test.go | 4 ++ reply.go | 11 +++- router.go | 45 +++++----------- security.go | 10 +++- view.go | 77 ++++++++++++++------------- view_test.go | 2 +- 12 files changed, 326 insertions(+), 170 deletions(-) diff --git a/access_log_test.go b/access_log_test.go index 96cb1fad..b33fb812 100644 --- a/access_log_test.go +++ b/access_log_test.go @@ -98,45 +98,6 @@ func TestAccessLogInitDefault(t *testing.T) { `) } -func TestEngineAccessLog(t *testing.T) { - // App Config - cfgDir := filepath.Join(getTestdataPath(), appConfigDir()) - err := initConfig(cfgDir) - assert.Nil(t, err) - assert.NotNil(t, AppConfig()) - - AppConfig().SetString("server.port", "8080") - - // Router - err = initRoutes(cfgDir, AppConfig()) - assert.Nil(t, err) - assert.NotNil(t, AppRouter()) - - // Security - err = initSecurity(AppConfig()) - assert.Nil(t, err) - assert.True(t, AppSessionManager().IsStateful()) - - // Controllers - cRegistry = controllerRegistry{} - - AddController((*Site)(nil), []*MethodInfo{ - { - Name: "GetInvolved", - Parameters: []*ParameterInfo{}, - }, - }) - - AppConfig().SetBool("server.access_log.enable", true) - - e := newEngine(AppConfig()) - req := httptest.NewRequest("GET", "localhost:8080/get-involved.html", nil) - res := httptest.NewRecorder() - e.ServeHTTP(res, req) - - assert.True(t, e.isAccessLogEnabled) -} - func testFormatter(t *testing.T, al *accessLog, pattern, expected string) { var err error appAccessLogFmtFlags, err = ess.ParseFmtFlag(pattern, accessLogFmtFlags) diff --git a/engine.go b/engine.go index a4db4d3c..688ae8df 100644 --- a/engine.go +++ b/engine.go @@ -100,9 +100,6 @@ func (e *engine) ServeHTTP(w http.ResponseWriter, r *http.Request) { goto wReply } - // Set defaults when actual value not found - e.setDefaults(ctx) - // Middlewares, interceptors, targeted controller e.executeMiddlewares(ctx) @@ -124,7 +121,12 @@ func (e *engine) handleRecovery(ctx *Context) { st.Print(buf) log.Error(buf.String()) - writeErrorInfo(ctx, http.StatusInternalServerError, "Internal Server Error") + ctx.Reply().Error(&Error{ + Code: http.StatusInternalServerError, + Message: http.StatusText(http.StatusInternalServerError), + Data: r, + }) + e.writeReply(ctx) } } @@ -168,7 +170,11 @@ func (e *engine) prepareContext(w http.ResponseWriter, r *http.Request) *Context func (e *engine) handleRoute(ctx *Context) flowResult { domain := AppRouter().FindDomain(ctx.Req) if domain == nil { - writeErrorInfo(ctx, http.StatusNotFound, "Not Found") + log.Warnf("Domain not found, Host: %s, Path: %s", ctx.Req.Host, ctx.Req.Path) + ctx.Reply().Error(&Error{ + Code: http.StatusNotFound, + Message: http.StatusText(http.StatusNotFound), + }) return flowStop } @@ -178,8 +184,11 @@ func (e *engine) handleRoute(ctx *Context) flowResult { return flowStop } - ctx.route = domain.NotFoundRoute - handleRouteNotFound(ctx, domain, domain.NotFoundRoute) + log.Warnf("Route not found, Host: %s, Path: %s", ctx.Req.Host, ctx.Req.Path) + ctx.Reply().Error(&Error{ + Code: http.StatusNotFound, + Message: http.StatusText(http.StatusNotFound), + }) return flowStop } @@ -202,15 +211,20 @@ func (e *engine) handleRoute(ctx *Context) flowResult { // Serving static file if route.IsStatic { if err := e.serveStatic(ctx); err == errFileNotFound { - handleRouteNotFound(ctx, domain, route) - ctx.Reply().done = false // override + log.Warnf("Static file not found, Host: %s, Path: %s", ctx.Req.Host, ctx.Req.Path) + ctx.Reply().done = false + ctx.Reply().NotFound().body = acquireBuffer() } return flowStop } // No controller or action found for the route if err := ctx.setTarget(route); err == errTargetNotFound { - handleRouteNotFound(ctx, domain, route) + log.Warnf("Target not found, Controller: %s, Action: %s", route.Controller, route.Action) + ctx.Reply().Error(&Error{ + Code: http.StatusNotFound, + Message: http.StatusText(http.StatusNotFound), + }) return flowStop } @@ -224,14 +238,6 @@ func (e *engine) loadSession(ctx *Context) { } } -// setDefaults method sets default value based on aah app configuration -// when actual value is not found. -func (e *engine) setDefaults(ctx *Context) { - if ctx.Req.Locale == nil { - ctx.Req.Locale = ahttp.NewLocale(AppConfig().StringDefault("i18n.default", "en")) - } -} - // executeMiddlewares method executes the configured middlewares. func (e *engine) executeMiddlewares(ctx *Context) { mwChain[0].Next(ctx) @@ -239,6 +245,10 @@ func (e *engine) executeMiddlewares(ctx *Context) { // writeReply method writes the response on the wire based on `Reply` instance. func (e *engine) writeReply(ctx *Context) { + if ctx.Reply().err != nil { + handleError(ctx, ctx.Reply().err) + } + // Response already written on the wire, don't go forward. // refer to `Reply().Done()` method. if ctx.Reply().done { @@ -276,7 +286,7 @@ func (e *engine) writeReply(ctx *Context) { e.doRender(ctx) isBodyAllowed := isResponseBodyAllowed(reply.Code) - // Gzip, 1kb above TODO make it configurable for bytes size value + // Gzip, 1kb above TODO make it configurable from aah.conf if isBodyAllowed && reply.body.Len() > 1024 { e.wrapGzipWriter(ctx) } @@ -289,16 +299,9 @@ func (e *engine) writeReply(ctx *Context) { // HTTP status ctx.Res.WriteHeader(reply.Code) + // Write response on the wire if isBodyAllowed { - // Write response on the wire - var err error - if minifier == nil || !appIsProfileProd || !ctHTML.IsEqual(reply.ContType) { - if _, err = reply.body.WriteTo(ctx.Res); err != nil { - log.Error(err) - } - } else if err = minifier(reply.ContType, ctx.Res, reply.body); err != nil { - log.Errorf("Minifier error: %s", err.Error()) - } + e.writeBody(ctx) } // 'OnAfterReply' server extension point @@ -352,6 +355,16 @@ func (e *engine) setCookies(ctx *Context) { } } +func (e *engine) writeBody(ctx *Context) { + if minifier == nil || !appIsProfileProd || !ctHTML.IsEqual(ctx.Reply().ContType) { + if _, err := ctx.Reply().body.WriteTo(ctx.Res); err != nil { + log.Error(err) + } + } else if err := minifier(ctx.Reply().ContType, ctx.Res, ctx.Reply().body); err != nil { + log.Errorf("Minifier error: %s", err.Error()) + } +} + //‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ // Unexported methods //___________________________________ diff --git a/engine_test.go b/engine_test.go index 67ddeac7..896718a2 100644 --- a/engine_test.go +++ b/engine_test.go @@ -6,7 +6,6 @@ package aah import ( "compress/gzip" - "fmt" "io/ioutil" "net/http" "net/http/httptest" @@ -90,10 +89,10 @@ func TestEngineNew(t *testing.T) { assert.NotNil(t, buf) releaseBuffer(buf) - appLogFatal = func(v ...interface{}) { fmt.Println(v) } + appLogFatal = func(v ...interface{}) { t.Log(v) } AppConfig().SetInt("render.gzip.level", 10) e = newEngine(AppConfig()) - fmt.Println(e) + assert.NotNil(t, e) } func TestEngineServeHTTP(t *testing.T) { @@ -115,6 +114,13 @@ func TestEngineServeHTTP(t *testing.T) { assert.Nil(t, err) assert.True(t, AppSessionManager().IsStateful()) + err = initLogs(getTestdataPath(), AppConfig()) + assert.Nil(t, err) + + err = initAccessLog(getTestdataPath(), AppConfig()) + assert.Nil(t, err) + appAccessLog.SetWriter(os.Stdout) + // Controllers cRegistry = controllerRegistry{} @@ -136,6 +142,8 @@ func TestEngineServeHTTP(t *testing.T) { // Middlewares Middlewares(ToMiddleware(testEngineMiddleware)) + AppConfig().SetBool("server.access_log.enable", true) + // Engine e := newEngine(AppConfig()) @@ -180,7 +188,6 @@ func TestEngineServeHTTP(t *testing.T) { resp4 := w4.Result() assert.Equal(t, 404, resp4.StatusCode) - assert.True(t, strings.Contains(resp4.Status, "Not Found")) // Request 5 RedirectTrailingSlash - 302 status wd, _ := os.Getwd() @@ -219,13 +226,15 @@ func TestEngineServeHTTP(t *testing.T) { r8 := httptest.NewRequest("POST", "http://localhost:8080/credits", nil) r8.Header.Add(ahttp.HeaderAcceptEncoding, "gzip, deflate, sdch, br") + r8.Header.Add(ahttp.HeaderAccept, ahttp.ContentTypeJSON.String()) w8 := httptest.NewRecorder() e.ServeHTTP(w8, r8) // Method Not Allowed 405 response resp8 := w8.Result() body8 := getResponseBody(resp8) - assert.Equal(t, "405 Method Not Allowed", body8) + assert.Equal(t, 405, resp8.StatusCode) + assert.Equal(t, `{"code":405,"message":"Method Not Allowed"}`, body8) assert.Equal(t, "GET, OPTIONS", resp8.Header.Get("Allow")) // Auto Options @@ -235,8 +244,7 @@ func TestEngineServeHTTP(t *testing.T) { e.ServeHTTP(w9, r9) resp9 := w9.Result() - body9 := getResponseBody(resp9) - assert.Equal(t, "200 'OPTIONS' allowed HTTP Methods", body9) + assert.Equal(t, 200, resp9.StatusCode) assert.Equal(t, "GET, OPTIONS", resp9.Header.Get("Allow")) appBaseDir = "" diff --git a/error.go b/error.go index 1de8ce34..1ef571ea 100644 --- a/error.go +++ b/error.go @@ -5,15 +5,104 @@ package aah import ( + "fmt" + "html/template" "strings" "aahframework.org/ahttp.v0" "aahframework.org/essentials.v0" + "aahframework.org/log.v0" ) -// writeError method writes the server error response based content type. -// It's handy internal method. -func writeErrorInfo(ctx *Context, code int, msg string) { +var errorHandler ErrorHandler + +var defaultErrorHTMLTemplate = template.Must(template.New("error_template").Parse(` + + + + + + {{ .Error.Code }} - {{ .Error.Message }} + + + + +
{{ with .Error }} +
+
+ {{ .Code }} - {{ .Message }} +
+
{{ end }} +
+ + +`)) + +type ( + // Error structure used to represent the error details in the aah framework. + Error struct { + Code int `json:"code,omitempty" xml:"code,omitempty"` + Message string `json:"message,omitempty" xml:"message,omitempty"` + Data interface{} `json:"data,omitempty" xml:"data,omitempty"` + } + + // ErrorHandler is function type used to register centralized error handling + // in aah framework. + ErrorHandler func(ctx *Context, err *Error) bool +) + +// SetErrorHandler method is used to register centralized application error +// handling. If custom handler is not then default error handler is used. +func SetErrorHandler(handler ErrorHandler) { + if handler != nil { + log.Infof("Custom centralized application error handler registered: %v", funcName(handler)) + errorHandler = handler + } +} + +//‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ +// Unexported package methods +//___________________________________ + +// handleError method is aah centralized error handler. +func handleError(ctx *Context, err *Error) { + if errorHandler == nil { + defaultErrorHandler(ctx, err) + } else { + if !errorHandler(ctx, err) { + defaultErrorHandler(ctx, err) + } + } +} + +// defaultErrorHandler method is used when custom error handler is not register +// in the aah. It writes the response based on HTTP Content-Type. +func defaultErrorHandler(ctx *Context, err *Error) bool { ct := ctx.Reply().ContType if ess.IsStrEmpty(ct) { if ict := identifyContentType(ctx); ict != nil { @@ -23,18 +112,32 @@ func writeErrorInfo(ctx *Context, code int, msg string) { ct = ct[:idx] } + // Set HTTP response code + ctx.Reply().Status(err.Code) + switch ct { case ahttp.ContentTypeJSON.Mime, ahttp.ContentTypeJSONText.Mime: - ctx.Reply().Status(code).JSON(Data{ - "code": code, - "message": msg, - }) + ctx.Reply().JSON(err) case ahttp.ContentTypeXML.Mime, ahttp.ContentTypeXMLText.Mime: - ctx.Reply().Status(code).XML(Data{ - "code": code, - "message": msg, - }) + ctx.Reply().XML(err) + case ahttp.ContentTypeHTML.Mime: + html := acquireHTML() + html.Filename = fmt.Sprintf("%d%s", err.Code, appViewExt) + if appViewEngine != nil { + tmpl, er := appViewEngine.Get("", "errors", html.Filename) + if tmpl == nil || er != nil { + html.Template = defaultErrorHTMLTemplate + } else { + html.Template = tmpl + } + } else { + html.Template = defaultErrorHTMLTemplate + } + html.ViewArgs = Data{"Error": err} + addFrameworkValuesIntoViewArgs(ctx, html) + ctx.Reply().Rdr = html default: - ctx.Reply().Status(code).Text("%d %s", code, msg) + ctx.Reply().Text("%d - %s", err.Code, err.Message) } + return true } diff --git a/error_test.go b/error_test.go index 21db6a65..598da159 100644 --- a/error_test.go +++ b/error_test.go @@ -5,31 +5,88 @@ package aah import ( + "fmt" + "net/http" + "net/http/httptest" + "path/filepath" "testing" + "aahframework.org/ahttp.v0" + "aahframework.org/config.v0" "aahframework.org/test.v0/assert" ) -func TestErrorWriteInfo(t *testing.T) { +func TestErrorHandler(t *testing.T) { + // 400 ctx1 := &Context{reply: acquireReply()} ctx1.Reply().ContentType("application/json") - writeErrorInfo(ctx1, 400, "Bad Request") - + handleError(ctx1, &Error{ + Code: http.StatusBadRequest, + Message: http.StatusText(http.StatusBadRequest), + }) assert.NotNil(t, ctx1.Reply().Rdr) jsonr := ctx1.Reply().Rdr.(*JSON) assert.NotNil(t, jsonr) assert.NotNil(t, jsonr.Data) - assert.Equal(t, 400, jsonr.Data.(Data)["code"]) - assert.Equal(t, "Bad Request", jsonr.Data.(Data)["message"]) + assert.Equal(t, 400, jsonr.Data.(*Error).Code) + assert.Equal(t, "Bad Request", jsonr.Data.(*Error).Message) + // 500 ctx2 := &Context{reply: acquireReply()} ctx2.Reply().ContentType("application/xml") - writeErrorInfo(ctx2, 500, "Internal Server Error") - + handleError(ctx2, &Error{ + Code: http.StatusInternalServerError, + Message: http.StatusText(http.StatusInternalServerError), + }) assert.NotNil(t, ctx2.Reply().Rdr) xmlr := ctx2.Reply().Rdr.(*XML) assert.NotNil(t, xmlr) assert.NotNil(t, xmlr.Data) - assert.Equal(t, 500, xmlr.Data.(Data)["code"]) - assert.Equal(t, "Internal Server Error", xmlr.Data.(Data)["message"]) + assert.Equal(t, 500, xmlr.Data.(*Error).Code) + assert.Equal(t, "Internal Server Error", xmlr.Data.(*Error).Message) + + SetErrorHandler(func(ctx *Context, err *Error) bool { + t.Log(ctx, err) + return false + }) + + // 403 + ctx3 := &Context{reply: acquireReply()} + ctx3.Reply().ContentType("text/plain") + handleError(ctx3, &Error{ + Code: http.StatusForbidden, + Message: http.StatusText(http.StatusForbidden), + }) + assert.NotNil(t, ctx3.Reply().Rdr) + plain := ctx3.Reply().Rdr.(*Text) + assert.NotNil(t, plain) + assert.Equal(t, "[403 Forbidden]", fmt.Sprint(plain.Values)) +} + +func TestErrorDefaultHandler(t *testing.T) { + appCfg, _ := config.ParseString("") + viewDir := filepath.Join(getTestdataPath(), appViewsDir()) + err := initViewEngine(viewDir, appCfg) + assert.Nil(t, err) + assert.NotNil(t, AppViewEngine()) + + // 400 + r1 := httptest.NewRequest("GET", "http://localhost:8080/get-involved.html", nil) + ctx1 := &Context{Req: ahttp.AcquireRequest(r1), reply: acquireReply()} + ctx1.Reply().ContentType(ahttp.ContentTypeHTML.String()) + defaultErrorHandler(ctx1, &Error{Code: http.StatusNotFound, Message: "Test message"}) + html := ctx1.Reply().Rdr.(*HTML) + t.Logf("%+v\n", html) + assert.True(t, defaultErrorHTMLTemplate == html.Template) + assert.Equal(t, "404.html", html.Filename) + + // 500 + r2 := httptest.NewRequest("GET", "http://localhost:8080/get-involved.html", nil) + ctx2 := &Context{Req: ahttp.AcquireRequest(r2), reply: acquireReply()} + ctx2.Reply().ContentType(ahttp.ContentTypeHTML.String()) + defaultErrorHandler(ctx2, &Error{Code: http.StatusInternalServerError, Message: "Test message"}) + html = ctx2.Reply().Rdr.(*HTML) + t.Logf("%+v\n", html) + assert.True(t, defaultErrorHTMLTemplate == html.Template) + assert.Equal(t, "500.html", html.Filename) } diff --git a/param.go b/param.go index 2fce2e69..8725f896 100644 --- a/param.go +++ b/param.go @@ -41,14 +41,20 @@ func (e *engine) parseRequestParams(ctx *Context) flowResult { // Content Negotitaion - Accepted, refer to Github #75 if isAcceptedExists && !ess.IsSliceContainsString(acceptedContentTypes, ctx.Req.ContentType.Mime) { log.Warnf("Content type '%v' not accepted by server", ctx.Req.ContentType.Mime) - writeErrorInfo(ctx, http.StatusUnsupportedMediaType, "Unsupported Media Type") + ctx.Reply().Error(&Error{ + Code: http.StatusUnsupportedMediaType, + Message: http.StatusText(http.StatusUnsupportedMediaType), + }) return flowStop } // Content Negotitaion - Offered, refer to Github #75 if isOfferedExists && !ess.IsSliceContainsString(offeredContentTypes, ctx.Req.AcceptContentType.Mime) { log.Warnf("Content type '%v' not offered by server", ctx.Req.AcceptContentType.Mime) - writeErrorInfo(ctx, http.StatusUnsupportedMediaType, "Not Acceptable") + ctx.Reply().Error(&Error{ + Code: http.StatusNotAcceptable, + Message: http.StatusText(http.StatusNotAcceptable), + }) return flowStop } @@ -74,7 +80,10 @@ func (e *engine) parseRequestParams(ctx *Context) flowResult { ctx.Req.Payload = payloadBytes } else { log.Errorf("unable to read request body for '%s': %s", contentType, err) - writeErrorInfo(ctx, http.StatusBadRequest, "unable to read request body") + ctx.Reply().Error(&Error{ + Code: http.StatusBadRequest, + Message: http.StatusText(http.StatusBadRequest), + }) return flowStop } case ahttp.ContentTypeForm.Mime: diff --git a/param_test.go b/param_test.go index 8148a376..b73a1105 100644 --- a/param_test.go +++ b/param_test.go @@ -120,6 +120,7 @@ func TestParamParseLocaleFromAppConfiguration(t *testing.T) { func TestParamContentNegotiation(t *testing.T) { defer ess.DeleteFiles("testapp.pid") + errorHandler = defaultErrorHandler e := engine{} // Accepted @@ -132,6 +133,7 @@ func TestParamContentNegotiation(t *testing.T) { reply: acquireReply(), } result1 := e.parseRequestParams(ctx1) + assert.Equal(t, http.StatusUnsupportedMediaType, ctx1.Reply().err.Code) assert.True(t, result1 == 1) isAcceptedExists = false @@ -139,12 +141,14 @@ func TestParamContentNegotiation(t *testing.T) { isOfferedExists = true offeredContentTypes = []string{"application/json"} r2 := httptest.NewRequest("POST", "http://localhost:8080/v1/userinfo", nil) + r1.Header.Set(ahttp.HeaderContentType, "application/json") r2.Header.Set(ahttp.HeaderAccept, "application/xml") ctx2 := &Context{ Req: ahttp.AcquireRequest(r2), reply: acquireReply(), } result2 := e.parseRequestParams(ctx2) + assert.Equal(t, http.StatusNotAcceptable, ctx2.Reply().err.Code) assert.True(t, result2 == 1) isOfferedExists = false } diff --git a/reply.go b/reply.go index 064a45c9..09506181 100644 --- a/reply.go +++ b/reply.go @@ -32,6 +32,7 @@ type Reply struct { path string done bool gzip bool + err *Error } //‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ @@ -171,8 +172,6 @@ func (r *Reply) JSON(data interface{}) *Reply { // JSONP method renders given data as JSONP response with callback. // Also it sets HTTP 'Content-Type' as 'application/json; charset=utf-8'. // Response rendered pretty if 'render.pretty' is true. -// Note: If `callback` param is empty and `callback` query param is exists then -// query param value will be used. func (r *Reply) JSONP(data interface{}, callback string) *Reply { j := acquireJSON() j.Data = data @@ -298,6 +297,13 @@ func (r *Reply) RedirectSts(redirectURL string, code int) *Reply { return r } +// Error method is used send an error reply, which is handled by centralized +// error handler. +func (r *Reply) Error(err *Error) *Reply { + r.err = err + return r +} + //‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ // Reply methods //___________________________________ @@ -387,6 +393,7 @@ func (r *Reply) Reset() { r.path = "" r.done = false r.gzip = true + r.err = nil } //‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ diff --git a/router.go b/router.go index 4feb9487..07ccc473 100644 --- a/router.go +++ b/router.go @@ -10,7 +10,6 @@ import ( "html/template" "net/http" "path/filepath" - "reflect" "strings" "aahframework.org/ahttp.v0" @@ -148,51 +147,35 @@ func handleRtsOptionsMna(ctx *Context, domain *router.Domain, rts bool) error { // HTTP: OPTIONS if reqMethod == ahttp.MethodOptions { if domain.AutoOptions { - processAllowedMethods(reply, domain.Allowed(reqMethod, reqPath), "Auto 'OPTIONS', ") - writeErrorInfo(ctx, http.StatusOK, "'OPTIONS' allowed HTTP Methods") - return nil + if processAllowedMethods(reply, domain.Allowed(reqMethod, reqPath), "Auto 'OPTIONS', ") { + ctx.Reply().Text("") + return nil + } } } // 405 Method Not Allowed if domain.MethodNotAllowed { - processAllowedMethods(reply, domain.Allowed(reqMethod, reqPath), "405 response, ") - writeErrorInfo(ctx, http.StatusMethodNotAllowed, "Method Not Allowed") - return nil + if processAllowedMethods(reply, domain.Allowed(reqMethod, reqPath), "405 response, ") { + ctx.Reply().Error(&Error{ + Code: http.StatusMethodNotAllowed, + Message: http.StatusText(http.StatusMethodNotAllowed), + }) + return nil + } } return errors.New("route not found") } -func processAllowedMethods(reply *Reply, allowed, prefix string) { +func processAllowedMethods(reply *Reply, allowed, prefix string) bool { if !ess.IsStrEmpty(allowed) { allowed += ", " + ahttp.MethodOptions reply.Header(ahttp.HeaderAllow, allowed) log.Debugf("%sAllowed HTTP Methods: %s", prefix, allowed) + return true } -} - -// handleRouteNotFound method is used for 1. route action not found, 2. route is -// not found and 3. static file/directory. -func handleRouteNotFound(ctx *Context, domain *router.Domain, route *router.Route) { - // handle effectively to reduce heap allocation - if domain.NotFoundRoute == nil { - log.Warnf("Route not found: %s, isStaticRoute: false", ctx.Req.Path) - writeErrorInfo(ctx, http.StatusNotFound, "Not Found") - return - } - - log.Warnf("Route not found: %s, isStaticRoute: %v", ctx.Req.Path, route.IsStatic) - if err := ctx.setTarget(route); err == errTargetNotFound { - writeErrorInfo(ctx, http.StatusNotFound, "Not Found") - return - } - - target := reflect.ValueOf(ctx.target) - notFoundAction := target.MethodByName(ctx.action.Name) - - log.Debugf("Calling user defined not-found action: %s.%s", ctx.controller, ctx.action.Name) - notFoundAction.Call(emptyArg) + return false } //‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ diff --git a/security.go b/security.go index 4491dd70..390b5479 100644 --- a/security.go +++ b/security.go @@ -69,7 +69,10 @@ func (e engine) handleAuthcAndAuthz(ctx *Context) flowResult { // and routes `auth` attribute is not defined then framework treats that route as `403 Forbidden`. if AppSecurityManager().IsAuthSchemesConfigured() { log.Warnf("Auth schemes are configured in security.conf, however attribute 'auth' or 'default_auth' is not defined in routes.conf, so treat it as 403 forbidden: %v", ctx.Req.Path) - writeErrorInfo(ctx, http.StatusForbidden, "Forbidden") + ctx.Reply().Error(&Error{ + Code: http.StatusForbidden, + Message: http.StatusText(http.StatusForbidden), + }) return flowStop } @@ -169,7 +172,10 @@ func (e *engine) doAuthcAndAuthz(ascheme scheme.Schemer, ctx *Context) flowResul ctx.Reply().Header(ahttp.HeaderWWWAuthenticate, `Basic realm="`+basicAuth.RealmName+`"`) } - writeErrorInfo(ctx, http.StatusUnauthorized, "Unauthorized") + ctx.Reply().Error(&Error{ + Code: http.StatusUnauthorized, + Message: http.StatusText(http.StatusUnauthorized), + }) return flowStop } diff --git a/view.go b/view.go index eca8453e..d33c7e86 100644 --- a/view.go +++ b/view.go @@ -14,7 +14,7 @@ import ( "aahframework.org/config.v0" "aahframework.org/essentials.v0" "aahframework.org/log.v0" - "aahframework.org/view.v0" + "aahframework.org/view.v0-unstable" ) var ( @@ -99,49 +99,38 @@ func initViewEngine(viewDir string, appCfg *config.Config) error { // 1) Prepare ViewArgs // 2) If HTML content type find appropriate template func (e *engine) resolveView(ctx *Context) { + if appViewEngine == nil || ctx.Reply().err != nil || !ctHTML.IsEqual(ctx.Reply().ContType) { + return + } + + // Resolving view by convention and configuration reply := ctx.Reply() + if reply.Rdr == nil { + reply.Rdr = &HTML{} + } - // HTML response - if ctHTML.IsEqual(reply.ContType) && appViewEngine != nil { - if reply.Rdr == nil { - reply.Rdr = &HTML{} - } + htmlRdr := reply.Rdr.(*HTML) - htmlRdr := reply.Rdr.(*HTML) + if ess.IsStrEmpty(htmlRdr.Layout) && appIsDefaultLayoutEnabled { + htmlRdr.Layout = appDefaultTmplLayout + } - if ess.IsStrEmpty(htmlRdr.Layout) && appIsDefaultLayoutEnabled { - htmlRdr.Layout = appDefaultTmplLayout - } + if htmlRdr.ViewArgs == nil { + htmlRdr.ViewArgs = make(map[string]interface{}) + } - if htmlRdr.ViewArgs == nil { - htmlRdr.ViewArgs = make(map[string]interface{}) + for k, v := range ctx.ViewArgs() { + if _, found := htmlRdr.ViewArgs[k]; found { + continue } + htmlRdr.ViewArgs[k] = v + } - for k, v := range ctx.ViewArgs() { - if _, found := htmlRdr.ViewArgs[k]; found { - continue - } - htmlRdr.ViewArgs[k] = v - } + // ViewArgs values from framework + addFrameworkValuesIntoViewArgs(ctx, htmlRdr) - // ViewArgs values from framework - htmlRdr.ViewArgs["Scheme"] = ctx.Req.Scheme - htmlRdr.ViewArgs["Host"] = ctx.Req.Host - htmlRdr.ViewArgs["HTTPMethod"] = ctx.Req.Method - htmlRdr.ViewArgs["RequestPath"] = ctx.Req.Path - htmlRdr.ViewArgs["Locale"] = ctx.Req.Locale - htmlRdr.ViewArgs["ClientIP"] = ctx.Req.ClientIP - htmlRdr.ViewArgs["IsJSONP"] = ctx.Req.IsJSONP() - htmlRdr.ViewArgs["IsAJAX"] = ctx.Req.IsAJAX() - htmlRdr.ViewArgs["HTTPReferer"] = ctx.Req.Referer - htmlRdr.ViewArgs["AahVersion"] = Version - htmlRdr.ViewArgs["EnvProfile"] = AppProfile() - htmlRdr.ViewArgs["AppBuildInfo"] = AppBuildInfo() - htmlRdr.ViewArgs[KeyViewArgSubject] = ctx.Subject() - - // find view template by convention if not provided - findViewTemplate(ctx) - } + // find view template by convention if not provided + findViewTemplate(ctx) } // defaultContentType method returns the Content-Type based on 'render.default' @@ -226,6 +215,22 @@ func tmplControllerName(ctx *Context) string { return cName } +func addFrameworkValuesIntoViewArgs(ctx *Context, html *HTML) { + html.ViewArgs["Scheme"] = ctx.Req.Scheme + html.ViewArgs["Host"] = ctx.Req.Host + html.ViewArgs["HTTPMethod"] = ctx.Req.Method + html.ViewArgs["RequestPath"] = ctx.Req.Path + html.ViewArgs["Locale"] = ctx.Req.Locale + html.ViewArgs["ClientIP"] = ctx.Req.ClientIP + html.ViewArgs["IsJSONP"] = ctx.Req.IsJSONP() + html.ViewArgs["IsAJAX"] = ctx.Req.IsAJAX() + html.ViewArgs["HTTPReferer"] = ctx.Req.Referer + html.ViewArgs["AahVersion"] = Version + html.ViewArgs["EnvProfile"] = AppProfile() + html.ViewArgs["AppBuildInfo"] = AppBuildInfo() + html.ViewArgs[KeyViewArgSubject] = ctx.Subject() +} + func init() { AddTemplateFunc(template.FuncMap{ "config": tmplConfig, diff --git a/view_test.go b/view_test.go index 0d17c899..a9ec8e4c 100644 --- a/view_test.go +++ b/view_test.go @@ -18,7 +18,7 @@ import ( "aahframework.org/config.v0" "aahframework.org/essentials.v0" "aahframework.org/test.v0/assert" - "aahframework.org/view.v0" + "aahframework.org/view.v0-unstable" ) func TestViewInit(t *testing.T) { From 13b782cb5ca4476b57e6d74139818a83c3035f4a Mon Sep 17 00:00:00 2001 From: Jeevanandam M Date: Sat, 12 Aug 2017 17:43:51 -0700 Subject: [PATCH 06/20] Improvements (#96) - added get and set method on context for values - less allocation in request flow and optimization --- aah.go | 2 +- aah_test.go | 4 +--- access_log.go | 6 ++++-- access_log_test.go | 3 ++- context.go | 10 ++++++++++ engine.go | 2 +- error.go | 4 ++-- event_test.go | 25 ++++++++++++----------- i18n.go | 15 ++++++++++---- i18n_test.go | 5 ++++- param.go | 31 ++++++++++++++++------------- param_test.go | 2 ++ render.go | 36 ++++++++++++++-------------------- server.go | 2 +- static.go | 49 +++++++++++++++++++++++----------------------- static_test.go | 3 +-- util.go | 4 ++-- view_test.go | 3 +-- 18 files changed, 112 insertions(+), 94 deletions(-) diff --git a/aah.go b/aah.go index c76fc818..7615467f 100644 --- a/aah.go +++ b/aah.go @@ -107,7 +107,7 @@ func AppHTTPAddress() string { // AppHTTPPort method returns aah application HTTP port number based on `server.port` // value. Possible outcomes are user-defined port, `80`, `443` and `8080`. func AppHTTPPort() string { - port := firstNonEmpty(AppConfig().StringDefault("server.proxyport", ""), + port := firstNonZeroString(AppConfig().StringDefault("server.proxyport", ""), AppConfig().StringDefault("server.port", appDefaultHTTPPort)) return parsePort(port) } diff --git a/aah_test.go b/aah_test.go index 996a07b5..9f37ff8e 100644 --- a/aah_test.go +++ b/aah_test.go @@ -6,7 +6,6 @@ package aah import ( "errors" - "fmt" "io/ioutil" "path/filepath" "strings" @@ -199,7 +198,7 @@ func TestAahLogDir(t *testing.T) { assert.Nil(t, err) appBuildInfo = nil - appLogFatal = func(v ...interface{}) { fmt.Println(v) } + appLogFatal = func(v ...interface{}) { t.Log(v) } logAsFatal(errors.New("test msg")) } @@ -235,7 +234,6 @@ func TestAahBuildInfo(t *testing.T) { func TestAahConfigValidation(t *testing.T) { err := checkSSLConfigValues(true, false, "/path/to/cert.pem", "/path/to/cert.key") assert.Equal(t, "SSL cert file not found: /path/to/cert.pem", err.Error()) - fmt.Println(err) certPath := filepath.Join(getTestdataPath(), "cert.pem") defer ess.DeleteFiles(certPath) diff --git a/access_log.go b/access_log.go index 383a3a10..d0b38048 100644 --- a/access_log.go +++ b/access_log.go @@ -66,6 +66,7 @@ type ( StartTime time.Time ElapsedDuration time.Duration Request *ahttp.Request + RequestID string ResStatus int ResBytes int ResHdr http.Header @@ -174,7 +175,7 @@ func listenForAccessLog() { func sendToAccessLog(ctx *Context) { al := acquireAccessLog() - al.StartTime = ctx.values[appReqStartTimeKey].(time.Time) + al.StartTime = ctx.Get(appReqStartTimeKey).(time.Time) // All the bytes have been written on the wire // so calculate elapsed time @@ -182,6 +183,7 @@ func sendToAccessLog(ctx *Context) { req := *ctx.Req al.Request = &req + al.RequestID = firstNonZeroString(req.Header.Get(appReqIDHdrKey), "-") al.ResStatus = ctx.Res.Status() al.ResBytes = ctx.Res.BytesWritten() al.ResHdr = ctx.Res.Header() @@ -207,7 +209,7 @@ func accessLogFormatter(al *accessLog) string { case fmtFlagRequestProto: buf.WriteString(al.Request.Unwrap().Proto) case fmtFlagRequestID: - buf.WriteString(al.GetRequestHdr(appReqIDHdrKey)) + buf.WriteString(al.RequestID) case fmtFlagRequestHeader: buf.WriteString(al.GetRequestHdr(part.Format)) case fmtFlagQueryString: diff --git a/access_log_test.go b/access_log_test.go index b33fb812..95de7646 100644 --- a/access_log_test.go +++ b/access_log_test.go @@ -47,10 +47,11 @@ func TestAccessLogFormatter(t *testing.T) { al.Request.Header = al.Request.Raw.Header al.Request.Header.Add(ahttp.HeaderAccept, "text/html") al.Request.Header.Set(ahttp.HeaderXRequestID, "5946ed129bf23409520736de") + al.RequestID = "5946ed129bf23409520736de" al.Request.ClientIP = "127.0.0.1" al.ResHdr.Add("content-type", "application/json") allAvailablePatterns := "%clientip %reqid %reqtime %restime %resstatus %ressize %reqmethod %requrl %reqhdr:accept %querystr %reshdr" - expectedForAllAvailablePatterns := fmt.Sprintf(`%s "%s" %s %v %d %d %s %s "%s" "%s" %s`, + expectedForAllAvailablePatterns := fmt.Sprintf(`%s %s %s %v %d %d %s %s "%s" "%s" %s`, al.Request.ClientIP, al.Request.Header.Get(ahttp.HeaderXRequestID), al.StartTime.Format(time.RFC3339), fmt.Sprintf("%.4f", al.ElapsedDuration.Seconds()*1e3), al.ResStatus, al.ResBytes, al.Request.Method, diff --git a/context.go b/context.go index 61b6ed61..d0e16851 100644 --- a/context.go +++ b/context.go @@ -215,6 +215,16 @@ func (ctx *Context) Reset() { ctx.decorated = false } +// Set method is used to set value for the given key in the current request flow. +func (ctx *Context) Set(key string, value interface{}) { + ctx.values[key] = value +} + +// Get method returns the value for the given key, otherwise it returns nil. +func (ctx *Context) Get(key string) interface{} { + return ctx.values[key] +} + //‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ // Context Unexported methods //___________________________________ diff --git a/engine.go b/engine.go index 688ae8df..c527c617 100644 --- a/engine.go +++ b/engine.go @@ -69,7 +69,7 @@ func (e *engine) ServeHTTP(w http.ResponseWriter, r *http.Request) { startTime := time.Now() ctx := e.prepareContext(w, r) - ctx.values[appReqStartTimeKey] = startTime + ctx.Set(appReqStartTimeKey, startTime) defer releaseContext(ctx) // Recovery handling, capture every possible panic(s) diff --git a/error.go b/error.go index 1ef571ea..3d103413 100644 --- a/error.go +++ b/error.go @@ -22,7 +22,7 @@ var defaultErrorHTMLTemplate = template.Must(template.New("error_template").Pars - {{ .Error.Code }} - {{ .Error.Message }} + {{ .Error.Code }} {{ .Error.Message }}