Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow header support in Router, MethodNotFoundHandler (405) and CORS middleware #2039

Merged
merged 2 commits into from
Jan 3, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,13 @@ type (
}
)

const (
// ContextKeyHeaderAllow is set by Router for getting value for `Allow` header in later stages of handler call chain.
// Allow header is mandatory for status 405 (method not found) and useful for OPTIONS method requests.
// It is added to context only when Router does not find matching method handler for request.
ContextKeyHeaderAllow = "____echo____header_allow"
aldas marked this conversation as resolved.
Show resolved Hide resolved
)

const (
defaultMemory = 32 << 20 // 32 MB
indexPage = "index.html"
Expand Down
14 changes: 12 additions & 2 deletions echo.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,11 @@ const (

// Headers
const (
HeaderAccept = "Accept"
HeaderAcceptEncoding = "Accept-Encoding"
HeaderAccept = "Accept"
HeaderAcceptEncoding = "Accept-Encoding"
// HeaderAllow is header field that lists the set of methods advertised as supported by the target resource.
// Allow header is mandatory for status 405 (method not found) and useful OPTIONS method responses.
// See: https://datatracker.ietf.org/doc/html/rfc7231#section-7.4.1
aldas marked this conversation as resolved.
Show resolved Hide resolved
HeaderAllow = "Allow"
HeaderAuthorization = "Authorization"
HeaderContentDisposition = "Content-Disposition"
Expand Down Expand Up @@ -301,6 +304,13 @@ var (
}

MethodNotAllowedHandler = func(c Context) error {
// 'Allow' header RFC: https://datatracker.ietf.org/doc/html/rfc7231#section-7.4.1
// >> An origin server MUST generate an Allow field in a 405 (Method Not Allowed) response
// and MAY do so in any other response.
aldas marked this conversation as resolved.
Show resolved Hide resolved
routerAllowMethods, ok := c.Get(ContextKeyHeaderAllow).(string)
if ok && routerAllowMethods != "" {
lammel marked this conversation as resolved.
Show resolved Hide resolved
c.Response().Header().Set(HeaderAllow, routerAllowMethods)
}
return ErrMethodNotAllowed
}
)
Expand Down
3 changes: 3 additions & 0 deletions echo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -716,13 +716,16 @@ func TestEchoNotFound(t *testing.T) {

func TestEchoMethodNotAllowed(t *testing.T) {
e := New()

e.GET("/", func(c Context) error {
return c.String(http.StatusOK, "Echo!")
})
req := httptest.NewRequest(http.MethodPost, "/", nil)
rec := httptest.NewRecorder()
e.ServeHTTP(rec, req)

assert.Equal(t, http.StatusMethodNotAllowed, rec.Code)
assert.Equal(t, "OPTIONS, GET", rec.Header().Get(HeaderAllow))
}

func TestEchoContext(t *testing.T) {
Expand Down
69 changes: 46 additions & 23 deletions middleware/cors.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ type (
// AllowMethods defines a list methods allowed when accessing the resource.
// This is used in response to a preflight request.
// Optional. Default value DefaultCORSConfig.AllowMethods.
// If `allowMethods` is left empty will fill for preflight request `Access-Control-Allow-Methods` header value
// from `Allow` header that echo.Router set into context.
AllowMethods []string `yaml:"allow_methods"`

// AllowHeaders defines a list of request headers that can be used when
Expand All @@ -41,6 +43,8 @@ type (
// a response to a preflight request, this indicates whether or not the
// actual request can be made using credentials.
// Optional. Default value false.
// Security: avoid using `AllowCredentials = true` with `AllowOrigins = *`.
// See http://blog.portswigger.net/2016/10/exploiting-cors-misconfigurations-for.html
AllowCredentials bool `yaml:"allow_credentials"`

// ExposeHeaders defines a whitelist headers that clients are allowed to
Expand Down Expand Up @@ -80,7 +84,9 @@ func CORSWithConfig(config CORSConfig) echo.MiddlewareFunc {
if len(config.AllowOrigins) == 0 {
config.AllowOrigins = DefaultCORSConfig.AllowOrigins
}
hasCustomAllowMethods := true
if len(config.AllowMethods) == 0 {
hasCustomAllowMethods = false
config.AllowMethods = DefaultCORSConfig.AllowMethods
}

Expand Down Expand Up @@ -109,10 +115,28 @@ func CORSWithConfig(config CORSConfig) echo.MiddlewareFunc {
origin := req.Header.Get(echo.HeaderOrigin)
allowOrigin := ""

preflight := req.Method == http.MethodOptions
res.Header().Add(echo.HeaderVary, echo.HeaderOrigin)

// No Origin provided
// Preflight request is an OPTIONS request, using three HTTP request headers: Access-Control-Request-Method,
// Access-Control-Request-Headers, and the Origin header. See: https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request
// For simplicity we just consider method type and later `Origin` header.
preflight := req.Method == http.MethodOptions

// Although router adds special handler in case of OPTIONS method we avoid calling next for OPTIONS in this middleware
// as CORS requests do not have cookies / authentication headers by default, so we could get stuck in auth
// middlewares by calling next(c).
// But we still want to send `Allow` header as response in case of Non-CORS OPTIONS request as router default
// handler does.
routerAllowMethods := ""
if preflight {
tmpAllowMethods, ok := c.Get(echo.ContextKeyHeaderAllow).(string)
if ok && tmpAllowMethods != "" {
routerAllowMethods = tmpAllowMethods
c.Response().Header().Set(echo.HeaderAllow, routerAllowMethods)
}
}

// No Origin provided. This is (probably) not request from actual browser - proceed executing middleware chain
if origin == "" {
if !preflight {
return next(c)
Expand Down Expand Up @@ -145,19 +169,15 @@ func CORSWithConfig(config CORSConfig) echo.MiddlewareFunc {
}
}

// Check allowed origin patterns
for _, re := range allowOriginPatterns {
if allowOrigin == "" {
didx := strings.Index(origin, "://")
if didx == -1 {
continue
}
domAuth := origin[didx+3:]
// to avoid regex cost by invalid long domain
if len(domAuth) > 253 {
break
}

checkPatterns := false
if allowOrigin == "" {
// to avoid regex cost by invalid (long) domains (253 is domain name max limit)
if len(origin) <= (253+3+4) && strings.Contains(origin, "://") {
aldas marked this conversation as resolved.
Show resolved Hide resolved
checkPatterns = true
}
}
if checkPatterns {
for _, re := range allowOriginPatterns {
if match, _ := regexp.MatchString(re, origin); match {
allowOrigin = origin
break
Expand All @@ -174,12 +194,13 @@ func CORSWithConfig(config CORSConfig) echo.MiddlewareFunc {
return c.NoContent(http.StatusNoContent)
}

res.Header().Set(echo.HeaderAccessControlAllowOrigin, allowOrigin)
if config.AllowCredentials {
res.Header().Set(echo.HeaderAccessControlAllowCredentials, "true")
}

// Simple request
if !preflight {
res.Header().Set(echo.HeaderAccessControlAllowOrigin, allowOrigin)
if config.AllowCredentials {
res.Header().Set(echo.HeaderAccessControlAllowCredentials, "true")
}
if exposeHeaders != "" {
res.Header().Set(echo.HeaderAccessControlExposeHeaders, exposeHeaders)
}
Expand All @@ -189,11 +210,13 @@ func CORSWithConfig(config CORSConfig) echo.MiddlewareFunc {
// Preflight request
res.Header().Add(echo.HeaderVary, echo.HeaderAccessControlRequestMethod)
res.Header().Add(echo.HeaderVary, echo.HeaderAccessControlRequestHeaders)
res.Header().Set(echo.HeaderAccessControlAllowOrigin, allowOrigin)
res.Header().Set(echo.HeaderAccessControlAllowMethods, allowMethods)
if config.AllowCredentials {
res.Header().Set(echo.HeaderAccessControlAllowCredentials, "true")

if !hasCustomAllowMethods && routerAllowMethods != "" {
lammel marked this conversation as resolved.
Show resolved Hide resolved
res.Header().Set(echo.HeaderAccessControlAllowMethods, routerAllowMethods)
} else {
res.Header().Set(echo.HeaderAccessControlAllowMethods, allowMethods)
}

if allowHeaders != "" {
res.Header().Set(echo.HeaderAccessControlAllowHeaders, allowHeaders)
} else {
Expand Down
Loading