Skip to content

Commit

Permalink
fix(middleware/cors): Validation of multiple Origins (#2883)
Browse files Browse the repository at this point in the history
* fix: allow origins check

Refactor CORS origin validation and normalization to trim leading or trailing whitespace in the cfg.AllowOrigins string [list]. URLs with whitespace inside the URL are invalid, so the normalizeOrigin will return false because url.Parse will fail, and the middleware will panic.

fixes #2882

* test: AllowOrigins with whitespace

* test(middleware/cors): add benchmarks

* chore: fix linter errors

* test(middleware/cors): use h() instead of app.Test()

* test(middleware/cors): add miltiple origins in Test_CORS_AllowOriginScheme

* chore: refactor validate and normalize

* test(cors/middleware): add more benchmarks

(cherry picked from commit d456e7d)
  • Loading branch information
sixcolors authored and ReneWerner87 committed Mar 1, 2024
1 parent 3c08c1b commit 4ab8629
Show file tree
Hide file tree
Showing 2 changed files with 513 additions and 15 deletions.
36 changes: 21 additions & 15 deletions middleware/cors/cors.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,23 +113,31 @@ func New(config ...Config) fiber.Handler {
log.Panic("[CORS] Insecure setup, 'AllowCredentials' is set to true, and 'AllowOrigins' is set to a wildcard.") //nolint:revive // we want to exit the program
}

// Validate and normalize static AllowOrigins if not using AllowOriginsFunc
if cfg.AllowOriginsFunc == nil && cfg.AllowOrigins != "" && cfg.AllowOrigins != "*" {
validatedOrigins := []string{}
for _, origin := range strings.Split(cfg.AllowOrigins, ",") {
isValid, normalizedOrigin := normalizeOrigin(origin)
// allowOrigins is a slice of strings that contains the allowed origins
// defined in the 'AllowOrigins' configuration.
var allowOrigins []string

// Validate and normalize static AllowOrigins
if cfg.AllowOrigins != "" && cfg.AllowOrigins != "*" {
origins := strings.Split(cfg.AllowOrigins, ",")
allowOrigins = make([]string, len(origins))

for i, origin := range origins {
trimmedOrigin := strings.TrimSpace(origin)
isValid, normalizedOrigin := normalizeOrigin(trimmedOrigin)

if isValid {
validatedOrigins = append(validatedOrigins, normalizedOrigin)
allowOrigins[i] = normalizedOrigin
} else {
log.Panicf("[CORS] Invalid origin format in configuration: %s", origin) //nolint:revive // we want to exit the program
log.Panicf("[CORS] Invalid origin format in configuration: %s", trimmedOrigin) //nolint:revive // we want to exit the program
}
}
cfg.AllowOrigins = strings.Join(validatedOrigins, ",")
} else {
// If AllowOrigins is set to a wildcard or not set,
// set allowOrigins to a slice with a single element
allowOrigins = []string{cfg.AllowOrigins}
}

// Convert string to slice
allowOrigins := strings.Split(strings.ReplaceAll(cfg.AllowOrigins, " ", ""), ",")

// Strip white spaces
allowMethods := strings.ReplaceAll(cfg.AllowMethods, " ", "")
allowHeaders := strings.ReplaceAll(cfg.AllowHeaders, " ", "")
Expand Down Expand Up @@ -164,10 +172,8 @@ func New(config ...Config) fiber.Handler {
// Run AllowOriginsFunc if the logic for
// handling the value in 'AllowOrigins' does
// not result in allowOrigin being set.
if allowOrigin == "" && cfg.AllowOriginsFunc != nil {
if cfg.AllowOriginsFunc(originHeader) {
allowOrigin = originHeader
}
if allowOrigin == "" && cfg.AllowOriginsFunc != nil && cfg.AllowOriginsFunc(originHeader) {
allowOrigin = originHeader
}

// Simple request
Expand Down
Loading

1 comment on commit 4ab8629

@ReneWerner87
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 4ab8629 Previous: d456e7d Ratio
Benchmark_Middleware_Favicon 207.2 ns/op 12 B/op 4 allocs/op 90.43 ns/op 3 B/op 1 allocs/op 2.29

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.