fix(router): add Vary to SkippedHeaders to prevent duplicate header by vickyshaw29 · Pull Request #2737 · wundergraph/cosmo

vickyshaw29 · 2026-04-02T19:25:33Z

gzhttp already adds Vary: Accept-Encoding to every compressed response. The same header coming from subgraphs gets propagated again through headerPropagationWriter, causing duplicates. This just adds Vary to the SkippedHeaders map so it doesn't get forwarded.

Summary by CodeRabbit

Bug Fixes
- Router no longer forwards Vary response headers from upstream services to clients, improving HTTP caching and content negotiation behavior.
Tests
- Added tests to ensure Vary is treated as an ignored header and is not propagated to clients.

coderabbitai · 2026-04-02T19:25:55Z

Walkthrough

Added Vary to the router's skipped headers so the Vary response header is not forwarded or propagated by the router; tests were added/updated to assert Vary is treated as ignored (case-insensitive) and not propagated to clients.

Changes

Cohort / File(s)	Summary
Headers configuration `router/internal/headers/headers.go`	Inserted `"Vary"` into the exported `SkippedHeaders` map to prevent forwarding the `Vary` response header.
Integration tests (header propagation) `router-tests/protocol/header_propagation_test.go`	Updated test setup to have subgraph set `Vary: Origin` and assert the router does not propagate `Vary` to the client.
Unit tests (header rule engine) `router/core/header_rule_engine_test.go`	Added `TestIsIgnoredHeader_Vary` verifying `isIgnoredHeader` treats `Vary` case-insensitively as ignored and does not ignore unrelated headers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding Vary to SkippedHeaders to prevent duplicate header propagation.
Linked Issues check	✅ Passed	The PR fully addresses issue `#2614` by adding Vary to SkippedHeaders map and including unit and integration tests to verify the fix.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the duplicate Vary header issue: updating SkippedHeaders, and adding corresponding unit and integration tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.4)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

endigma · 2026-04-02T19:43:42Z

Hi @vickyshaw29, can you add a test? Both unit + integraion would be great!

vickyshaw29 · 2026-04-02T21:01:49Z

Added both in b546c6f, unit test for isIgnoredHeader and extended the existing integration test for ignored response headers.

github-actions · 2026-04-17T05:54:38Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

endigma · 2026-04-17T14:32:37Z

+func TestIsIgnoredHeader_Vary(t *testing.T) {
+	t.Parallel()
+
+	assert.True(t, isIgnoredHeader("Vary"))
+	assert.True(t, isIgnoredHeader("vary"))
+	assert.False(t, isIgnoredHeader("X-Custom-Header"))
+}


I stand corrected, this is a useless test, integration looks good though.

Removed the unit test, kept the integration one

coderabbitai

🧹 Nitpick comments (1)

router-tests/protocol/header_propagation_test.go (1)
120-124: Integration coverage for Vary is on point.

The propagate rule + subgraph-side Vary: Origin injection + client-side NotEqual assertion mirrors the existing pattern for Content-Type/Content-Encoding/Connection and correctly exercises the fix for #2614.

Optional nit (non-blocking, and consistent with the surrounding cases so feel free to ignore): require.NotEqual(t, "Origin", vary, ...) would still pass if the router forwarded, say, Origin, Accept-Encoding. A stricter check would be require.NotContains(t, vary, "Origin") or asserting Vary is absent from the subgraph-injected value entirely. Same caveat applies to the neighboring ignored-header assertions, so changing just Vary would make the tests inconsistent.

Also applies to: 142-142, 165-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@router-tests/protocol/header_propagation_test.go` around lines 120 - 124, The
Vary propagation test uses require.NotEqual which would still pass if the router
forwarded a compound header like "Origin, Accept-Encoding"; update the
assertion(s) associated with the propagate rule for Named "Vary" (the case using
config.HeaderRuleOperationPropagate and
config.ResponseHeaderRuleAlgorithmLastWrite) to use a stricter check such as
require.NotContains(t, vary, "Origin") or assert that the Vary header does not
contain "Origin" (or is entirely absent) instead of NotEqual; apply the same
change to the other occurrences that assert Vary in this test to keep
consistency with the neighboring ignored-header assertions.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@router-tests/protocol/header_propagation_test.go`:
- Around line 120-124: The Vary propagation test uses require.NotEqual which
would still pass if the router forwarded a compound header like "Origin,
Accept-Encoding"; update the assertion(s) associated with the propagate rule for
Named "Vary" (the case using config.HeaderRuleOperationPropagate and
config.ResponseHeaderRuleAlgorithmLastWrite) to use a stricter check such as
require.NotContains(t, vary, "Origin") or assert that the Vary header does not
contain "Origin" (or is entirely absent) instead of NotEqual; apply the same
change to the other occurrences that assert Vary in this test to keep
consistency with the neighboring ignored-header assertions.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fd08a859-aa1d-4e80-9185-1674c572bf8e

📥 Commits

Reviewing files that changed from the base of the PR and between 9c4348b and 8358e81.

📒 Files selected for processing (2)

router-tests/protocol/header_propagation_test.go
router/core/header_rule_engine_test.go

fix(router): add Vary to SkippedHeaders to prevent duplicate header

9c4348b

vickyshaw29 requested review from Noroth, SkArchon, StarpTech, devsergiy, endigma and jensneuse as code owners April 2, 2026 19:25

github-actions Bot added the router label Apr 2, 2026

test(router): add unit and integration tests for Vary in SkippedHeaders

b546c6f

github-actions Bot added the Stale label Apr 17, 2026

endigma reviewed Apr 17, 2026

View reviewed changes

Merge branch 'main' into fix/skip-vary-header-propagation

8358e81

coderabbitai Bot reviewed Apr 17, 2026

View reviewed changes

remove unit test for isIgnoredHeader

0e4c1e4

github-actions Bot removed the Stale label Apr 18, 2026

Merge branch 'main' into fix/skip-vary-header-propagation

468d588

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(router): add Vary to SkippedHeaders to prevent duplicate header#2737

fix(router): add Vary to SkippedHeaders to prevent duplicate header#2737
vickyshaw29 wants to merge 5 commits intowundergraph:mainfrom
vickyshaw29:fix/skip-vary-header-propagation

vickyshaw29 commented Apr 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 2, 2026 •

edited

Loading

Uh oh!

endigma commented Apr 2, 2026

Uh oh!

vickyshaw29 commented Apr 2, 2026

Uh oh!

github-actions Bot commented Apr 17, 2026

Uh oh!

endigma Apr 17, 2026

Uh oh!

vickyshaw29 Apr 17, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

vickyshaw29 commented Apr 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

endigma commented Apr 2, 2026

Uh oh!

vickyshaw29 commented Apr 2, 2026

Uh oh!

github-actions Bot commented Apr 17, 2026

Uh oh!

endigma Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

vickyshaw29 Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vickyshaw29 commented Apr 2, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 2, 2026 •

edited

Loading