Fix CORS implementation: validate origin, short-circuit preflights, add Vary#1105
Merged
ggallotti merged 1 commit intogeai-release-stablefrom Apr 28, 2026
Merged
Conversation
…dd Vary - CORSHelper now validates the request Origin against a configured allowlist (single origin or comma-separated list) and only emits CORS headers when the origin is allowed. Adds Vary: Origin to keep caches correct. - '*' is no longer combined with Allow-Credentials (forbidden by the spec). - Access-Control-Max-Age and Allow-Methods/Allow-Headers are emitted only on preflight (OPTIONS) responses. - Servlet CorsFilter (javax + jakarta) now short-circuits valid preflights with 204 instead of letting them flow through to downstream handlers (which usually returned 405). - JAXRSCorsFilter (javax + jakarta) now also implements a @PreMatching ContainerRequestFilter that aborts preflights with 204 + CORS headers before resource matching runs. - Adds CORSHelperTest with 15 tests covering disabled mode, allowlist matching, wildcard semantics, preflight vs simple, Vary, Max-Age and case-insensitive header lookup.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The CORS implementation in
wrappercommonand the servlet/JAX-RS filters had several bugs and spec violations. This PR rewritesCORSHelperand updates all four filters (javax+jakarta, servlet + JAX-RS) and adds unit tests.Companion PR for the release branch: #1105.
Bugs fixed
CorsFilterset CORS headers and then always calledfilterChain.doFilter(...), soOPTIONSrequests flowed down to handlers that didn't accept them (typically405). Now valid preflights return204 No Contentimmediately.Access-Control-Allow-Origin: *was combined withAccess-Control-Allow-Credentials: true. Browsers reject that combination per the CORS spec. Credentials are now only emitted when echoing a real origin.Originwas never read or validated. The filter blindly returned the configured value regardless of the request.CORSHelpernow readsOriginand validates it against the configured allowlist (single origin or comma-separated list).Vary: Originwas missing. Caches/CDNs could serve a CORS response from one origin to a different origin. Now emitted whenever the response varies onOrigin.Originthat's in the allowlist.Access-Control-Max-AgeandAllow-Methods/Allow-Headerswere sent on actual responses. They only make sense on preflights. Moved to preflight-only.JAXRSCorsFilterwas aContainerResponseFilteronly, so preflights still went through resource matching. Added@PreMatching ContainerRequestFilterbehavior that aborts preflights with204+ CORS headers before resource matching runs.final, added null-safe handling forhttpMethod, madeMap-based header lookup case-insensitive (HTTP headers are case-insensitive).Configuration
Properties are resolved in this order (via
IniFile+EnvVarReader):confmapping.jsonremapping (if present inWEB-INF/), which lets you bind a property to a custom env var name.client.cfg, section[Client].client.cfgkey (section[Client])GX_CORS_ALLOW_ORIGINCORS_ALLOW_ORIGIN•
*— any origin (without credentials, per spec)•
https://app.example— a single origin•
https://a.example,https://b.example— comma-separated allowlistBehavior
OPTIONSflows through the chain unchanged.Originheader are treated as same-origin and receive no CORS headers.Originnot in the allowlist receive no CORS headers (the browser then blocks the response).OriginreceiveAccess-Control-Allow-Originechoing the matched origin (or*when configured),Vary: Origin(omitted for*), andAccess-Control-Allow-Credentials: true(omitted for*).OPTIONS) also receivesAccess-Control-Max-Age: 86400, plusAccess-Control-Allow-Methods/Access-Control-Allow-Headersreflecting the requested values, and the response is short-circuited with204 No Content.Notes on
Access-Control-Max-Age = 86400The implementation hardcodes
86400(1 day). Browsers cap this:So the effective cache lifetime is browser-determined; the configured value is just an upper bound. Not currently exposed as a property — can be added on request.
Tests
New
CORSHelperTest(15 cases) covering:Originheader → no headers emittedVaryfor*isPreflightsemanticshttpMethoddoesn't throwAll tests pass locally (
java org.junit.runner.JUnitCore com.genexus.cors.CORSHelperTest→OK (15 tests)).Test plan
CORSHelperTest.GX_CORS_ALLOW_ORIGIN=https://foo.example:https://foo.example: response hasAccess-Control-Allow-Origin: https://foo.example,Vary: Origin,Allow-Credentials: true.https://bar.example: no CORS headers (browser blocks).OPTIONSpreflight fromhttps://foo.example:204+Allow-Methods+Max-Age+Allow-Headers.GX_CORS_ALLOW_ORIGIN=*: response hasAllow-Origin: *and noAllow-Credentials.