Skip to content

fix: do not set Vary: Origin for static string origin option#405

Open
Vansh1811 wants to merge 1 commit intoexpressjs:masterfrom
Vansh1811:fix/no-vary-for-static-string-origin
Open

fix: do not set Vary: Origin for static string origin option#405
Vansh1811 wants to merge 1 commit intoexpressjs:masterfrom
Vansh1811:fix/no-vary-for-static-string-origin

Conversation

@Vansh1811
Copy link
Copy Markdown
Contributor

Problem

When the origin option is set to a static string (e.g. origin: 'https://example.com'), the current code incorrectly sets Vary: Origin in the response:

} else if (isString(options.origin)) {
  // fixed origin
  headers.push([{ key: 'Access-Control-Allow-Origin', value: options.origin }]);
  headers.push([{ key: 'Vary', value: 'Origin' }]); // <-- Bug: should NOT be here
}

This violates the Fetch spec:

If Access-Control-Allow-Origin is set to * or a static origin for a particular resource, then configure the server to always send Access-Control-Allow-Origin in responses for the resource — for non-CORS requests as well as CORS requests — and do not use Vary.

When origin is a static string, Access-Control-Allow-Origin is always the same value regardless of the request's Origin header. Setting Vary: Origin in this case unnecessarily inflates cache size — every unique client origin creates a separate cache entry even though the response is identical.

Fix

Remove the Vary: Origin header from the isString(options.origin) branch of configureOrigin(). Vary: Origin is still correctly set for dynamic origin cases (RegExp, Array, function) where the response varies based on the request's Origin header.

Testing

The existing test suite covers the static string origin case. A new test can verify Vary: Origin is NOT present in the response when origin is set to a static string.

Fixes #332

When origin option is a static string, the Access-Control-Allow-Origin
response is always the same value regardless of the request's Origin header.
Per the Fetch spec, Vary: Origin must NOT be set in this case, as it would
unnecessarily increase cache size without any benefit.

Fixes expressjs#332
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vary: Origin should not be set if the Origin request header is ignored

1 participant