CORS: invalid ACCESS_CONTROL_MAX_AGE is passed through unvalidated with no warning
- Dominant language
- Go
- Stars
- 20.9k
- Forks
- 1.8k
- Avg merge
- 5d 18h
- Merged PRs (30d)
- 39
Description
### What
`middleware.GetConfigs` copies `ACCESS_CONTROL_MAX_AGE` into the CORS header map with only a non-empty check ([`pkg/gofr/http/middleware/config.go#L29-L42`](https://github.com/gofr-dev/gofr/blob/development/pkg/gofr/http/middleware/config.go#L29-L42)), and `setMiddlewareHeaders` writes it to the response verbatim via the "additional custom headers" loop ([`pkg/gofr/http/middleware/cors.go#L69-L74`](https://github.com/gofr-dev/gofr/blob/development/pkg/gofr/http/middleware/cors.go#L69-L74)).
A value like `10m`, `600s`, `-1` or `abc` therefore produces a malformed `Access-Control-Max-Age` response header.
### Impact
Browsers discard the malformed header and fall back to their own default preflight cache (5s in Chrome), so the preflight caching the user configured silently never happens. Nothing in the logs indicates the misconfiguration — the app starts clean and the header looks present if you only eyeball the response.
The same class of gap applies to `ACCESS_CONTROL_ALLOW_CREDENTIALS`, where any non-boolean string is passed through unchecked.
### To reproduce
```bash
ACCESS_CONTROL_ALLOW_ORIGIN=* ACCESS_CONTROL_MAX_AGE=10m go run ./main.go
curl -i -X OPTIONS localhost:8000/some-route
# Access-Control-Max-Age: 10m <- invalid, silently ignored by the browser
```
### Expected
Validate the value at startup:
- parse `ACCESS_CONTROL_MAX_AGE` as a non-negative integer (seconds);
- on failure, log a warning naming the config key and the offending value, and drop the header rather than emitting an invalid one;
- consider the same treatment for `ACCESS_CONTROL_ALLOW_CREDENTIALS` (should parse as a bool).
Not proposing a default value for `ACCESS_CONTROL_MAX_AGE` — omitting the header and letting the browser apply its own default is reasonable, and picking a framework-wide TTL would be a surprising behaviour change on upgrade. This issue is only about failing loud on an invalid value.
### Note for the implementer
`GetConfigs(c config.Config)` currently has no logger available. The call site in [`pkg/gofr/factory.go#L31`](https://github.com/gofr-dev/gofr/blob/development/pkg/gofr/factory.go#L31) does have `app.container`, so the fix either threads a logger into `GetConfigs` or returns the validation problems for the caller to log.
Contributor guide
Research direction
Start with pkg/gofr/http/middleware/config.go and cors.go to trace how ACCESS_CONTROL_MAX_AGE and ACCESS_CONTROL_ALLOW_CREDENTIALS reach response headers, then inspect the call site in pkg/gofr/factory.go. Done means invalid values are identified at startup, warnings name the key and offending value, and invalid headers are omitted without introducing a default max-age.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 64/100