fix: header modifier doesn't permit multiple values with commas by kkk777-7 · Pull Request #7436 · envoyproxy/gateway

kkk777-7 · 2025-11-05T17:40:39Z

What this PR does / why we need it:
Fixed an issue where some values were missing when multiple comma-separated values were specified in HeaderModifier set.

Root cause
The issue occurs because comma-separated input is stored as intermediate array data, and in the case of set, Envoy’s corev3.HeaderValueOption_OVERWRITE_IF_EXISTS_OR_ADD is used, which results in only the last element being added.
https://github.com/envoyproxy/gateway/blob/main/internal/xds/translator/route.go#L595

According to RFC 7230, when multiple values share the same header field name, it is recommended to send them as a comma-separated value. Therefore, stopped storing intermediate data as an array separated by commas.

Which issue(s) this PR fixes:

Fixes #5733

Release Notes: Yes

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

codecov · 2025-11-05T17:49:59Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.39%. Comparing base (1e295b6) to head (c493922).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7436      +/-   ##
==========================================
+ Coverage   72.36%   72.39%   +0.03%     
==========================================
  Files         231      231              
  Lines       34042    34042              
==========================================
+ Hits        24633    24646      +13     
+ Misses       7633     7622      -11     
+ Partials     1776     1774       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

zirain · 2025-11-05T23:38:52Z

/retest

arkodg · 2025-11-06T02:16:15Z

internal/gatewayapi/filters.go

 				Name:   headerKey,
 				Append: true,
-				Value:  strings.Split(addHeader.Value, ","),
+				Value:  []string{addHeader.Value},


doesnt this break request header case, fixed by #4031 ?

Exactly. I’m currently reconsidering whether the approach in #4031 is indeed the correct one.

my understanding is that when a header field contains comma-separated values,
Envoy should, as a proxy, pass the header through without splitting the values.
(I’m not very familiar with this area, so I might be mistaken 😅)

I’ll check how other OSS projects handle this.

FWIW, just my opinion as a user/outsider tracking this:

Envoy should, as a proxy, pass the header through without splitting the values.

This would be the expected behavior to me.

Permissions-Policy for example can either be sent as multiple headers or a single comma-separated one, but it's usually sent comma-separated (especially when it ends up being massive, where splitting it up would actually incur measurable response size overhead). I don't think there's any reason envoy should unnecessarily split it up, and IIRC nginx also sends comma-separated headers as-is.

@azey7f
thanks for your insight !
From a quick look at Istio’s code and documentation, it seems that Istio also doesn’t perform any splitting.

* revert: separate headers with commas Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> * add e2e Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

* chore(examples): fix extensionserver build (#7398) Signed-off-by: Maxime Brunet <max@brnt.mx> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * chore: add missing endpoints in the crl test (#7402) fix test for #7199 Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * chore(make): exit on failure (#7387) Signed-off-by: Maxime Brunet <max@brnt.mx> Co-authored-by: zirain <zirain2009@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * fix: port typo (#7397) Signed-off-by: cong <q1875486458@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * build(deps): bump busybox from `2f590fc` to `e3652a0` in /tools/docker/envoy-gateway (#7409) build(deps): bump busybox in /tools/docker/envoy-gateway Bumps busybox from `2f590fc` to `e3652a0`. --- updated-dependencies: - dependency-name: busybox dependency-version: e3652a00a2fabd16ce889f0aa32c38eec347b997e73bd09e69c962ec7f8732ee dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * fix: validate EnvoyGateway configuration before reload (#7412) Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * build(deps): bump the actions group across 1 directory with 2 updates (#7410) Bumps the actions group with 2 updates in the / directory: [github/codeql-action](https://github.com/github/codeql-action) and [google/osv-scanner-action](https://github.com/google/osv-scanner-action). Updates `github/codeql-action` from 4.31.0 to 4.31.2 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@4e94bd1...0499de3) Updates `google/osv-scanner-action` from 2.2.3 to 2.2.4 - [Release notes](https://github.com/google/osv-scanner-action/releases) - [Commits](google/osv-scanner-action@e92b5d0...9bb6957) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.31.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: google/osv-scanner-action dependency-version: 2.2.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * fix: missing onInvalidMessage for ClientTrafficPolicy (#7417) Signed-off-by: i.makarychev <makarichev.ivan@gmail.com> Signed-off-by: i.makarychev <i.makarychev@tbank.ru> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * chore: add missing filters in the filter order configuration (#7404) * add missing filters in the filter order configuration Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> * fix wrong filter name Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * test: tcp security policy e2e (#7226) * feat(securitypolicy): Added e2e tests for tcp security policies Signed-off-by: davem-git <demathieu@gmail.com> * removed commented out line Signed-off-by: davem-git <demathieu@gmail.com> --------- Signed-off-by: davem-git <demathieu@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * Docs: tcp security policy (#7247) * updated release notes Signed-off-by: davem-git <demathieu@gmail.com> * updated docs Signed-off-by: davem-git <demathieu@gmail.com> * fixed merge conflict Signed-off-by: davem-git <demathieu@gmail.com> --------- Signed-off-by: davem-git <demathieu@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * feat: support both local and global ratelimit simultaneously (#7334) * update rate limit type Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> * feat: support both type rate limit Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * feat: support separated path match in ratelimit path (#7413) * update: path match ratelimit e2e Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * fix: handle optional next update for CRL (#7422) fix: handle optional next update for crl Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * fix: missing jwt provider when jwt is configured on multiple listeners sharing the same port (#7337) * fix jwt provider missing when jwt is configured at multiple ir listeners Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * fix: only insert proxy service once it exists (#7424) * maybe this is the fix? Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fixes Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * cleanup Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * consolidate Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> * fix Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> --------- Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * fix error when updating invalid gateway status (#7415) * fix error when updating invalid gateway status Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * fix: avoid calling the issuer's well-known endpoint for every routes (#7394) * fix: avoid calling the issuer's well-known endpoint for every routes with Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * fix: memory leak (#7429) Fix memory leak. Two watchable.Maps were never closed when shutting down the provider: - GatewayClassStatuses.Close() - missing in GatewayAPIStatuses.Close() - BackendTrafficPolicyStatuses.Close() - missing in PolicyStatuses.Close() Each unclosed map leaked 3 goroutines: 1. Internal watchable.Map.coalesce goroutine 2. HandleSubscription goroutine blocked on channel read 3. Error handler goroutine blocked on channel read Signed-off-by: Gonzalo Serrano <boikot@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * perf: move snapshot update above status update in xds layer (#7423) Signed-off-by: Arko Dasgupta <arko@tetrate.io> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * chore: cleanup logging when inserting proxy service cluster (#7431) cleanup Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * upgrade gofumpt (#7420) Signed-off-by: fabian4 <fabian.v.bao@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * feat(translator): relax backend restrictions for localhost when running standalone with Host infrastructure (#7427) Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * chore: improve api docs for http10.useDefaultHost (#7435) * imporove api docs for useDefaultHost Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * ci: disable lint.dependabot (#7445) Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * chore: bump github.com/containerd/containerd (#7448) Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * perf: do not set last transition time for status in watcher layer (#7268) Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * docs: fix gwapi docs (#7408) * docs: fix gwapi docs Signed-off-by: zirain <zirain2009@gmail.com> * fix Signed-off-by: zirain <zirain2009@gmail.com> * update Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * chore: renable lint.dependabot (#7454) Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * chore: remove last transition time comparison as no longer set (#7451) chore: remove last transition time comparision as no longer set Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> Co-authored-by: zirain <zirain2009@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * fix: merged policy status (#7376) Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * fix: header modifier doesn't permit multiple values with commas (#7436) * revert: separate headers with commas Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> * add e2e Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * fix auto http config with proxy protocol (#7439) * don't set TypedExtensionProtocolOptions when ProxyProtocol enabled Signed-off-by: zirain <zirain2009@gmail.com> * update test Signed-off-by: zirain <zirain2009@gmail.com> * enable auto ALPN for proxy protocol Signed-off-by: zirain <zirain2009@gmail.com> * add e2e Signed-off-by: zirain <zirain2009@gmail.com> * update Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * build(deps): bump sigs.k8s.io/controller-runtime from 0.22.3 to 0.22.4 in /examples/extension-server (#7470) build(deps): bump sigs.k8s.io/controller-runtime Bumps [sigs.k8s.io/controller-runtime](https://github.com/kubernetes-sigs/controller-runtime) from 0.22.3 to 0.22.4. - [Release notes](https://github.com/kubernetes-sigs/controller-runtime/releases) - [Changelog](https://github.com/kubernetes-sigs/controller-runtime/blob/main/RELEASE.md) - [Commits](kubernetes-sigs/controller-runtime@v0.22.3...v0.22.4) --- updated-dependencies: - dependency-name: sigs.k8s.io/controller-runtime dependency-version: 0.22.4 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * build(deps): bump softprops/action-gh-release from 2.4.1 to 2.4.2 in the actions group across 1 directory (#7461) build(deps): bump softprops/action-gh-release Bumps the actions group with 1 update in the / directory: [softprops/action-gh-release](https://github.com/softprops/action-gh-release). Updates `softprops/action-gh-release` from 2.4.1 to 2.4.2 - [Release notes](https://github.com/softprops/action-gh-release/releases) - [Changelog](https://github.com/softprops/action-gh-release/blob/master/CHANGELOG.md) - [Commits](softprops/action-gh-release@6da8fa9...5be0e66) --- updated-dependencies: - dependency-name: softprops/action-gh-release dependency-version: 2.4.2 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * build(deps): bump github.com/envoyproxy/go-control-plane/envoy from 1.35.0 to 1.36.0 in /examples/grpc-ext-proc (#7471) build(deps): bump github.com/envoyproxy/go-control-plane/envoy Bumps [github.com/envoyproxy/go-control-plane/envoy](https://github.com/envoyproxy/go-control-plane) from 1.35.0 to 1.36.0. - [Release notes](https://github.com/envoyproxy/go-control-plane/releases) - [Changelog](https://github.com/envoyproxy/go-control-plane/blob/main/CHANGELOG.md) - [Commits](envoyproxy/go-control-plane@envoy/v1.35.0...envoy/v1.36.0) --- updated-dependencies: - dependency-name: github.com/envoyproxy/go-control-plane/envoy dependency-version: 1.36.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * build(deps): bump github.com/envoyproxy/go-control-plane/envoy from 1.35.0 to 1.36.0 in /examples/envoy-ext-auth (#7467) build(deps): bump github.com/envoyproxy/go-control-plane/envoy Bumps [github.com/envoyproxy/go-control-plane/envoy](https://github.com/envoyproxy/go-control-plane) from 1.35.0 to 1.36.0. - [Release notes](https://github.com/envoyproxy/go-control-plane/releases) - [Changelog](https://github.com/envoyproxy/go-control-plane/blob/main/CHANGELOG.md) - [Commits](envoyproxy/go-control-plane@envoy/v1.35.0...envoy/v1.36.0) --- updated-dependencies: - dependency-name: github.com/envoyproxy/go-control-plane/envoy dependency-version: 1.36.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * build(deps): bump github.com/envoyproxy/go-control-plane/envoy from 1.35.1-0.20251029084203-42a4a9261f66 to 1.36.0 in /examples/extension-server (#7468) build(deps): bump github.com/envoyproxy/go-control-plane/envoy Bumps [github.com/envoyproxy/go-control-plane/envoy](https://github.com/envoyproxy/go-control-plane) from 1.35.1-0.20251029084203-42a4a9261f66 to 1.36.0. - [Release notes](https://github.com/envoyproxy/go-control-plane/releases) - [Changelog](https://github.com/envoyproxy/go-control-plane/blob/main/CHANGELOG.md) - [Commits](https://github.com/envoyproxy/go-control-plane/commits/envoy/v1.36.0) --- updated-dependencies: - dependency-name: github.com/envoyproxy/go-control-plane/envoy dependency-version: 1.36.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> * [release/v1.6] v1.6.0 release docs (#7475) Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> --------- Signed-off-by: Maxime Brunet <max@brnt.mx> Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com> Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com> Signed-off-by: cong <q1875486458@gmail.com> Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: zirain <zirain2009@gmail.com> Signed-off-by: i.makarychev <makarichev.ivan@gmail.com> Signed-off-by: i.makarychev <i.makarychev@tbank.ru> Signed-off-by: davem-git <demathieu@gmail.com> Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com> Signed-off-by: jukie <10012479+jukie@users.noreply.github.com> Signed-off-by: Gonzalo Serrano <boikot@gmail.com> Signed-off-by: Arko Dasgupta <arko@tetrate.io> Signed-off-by: fabian4 <fabian.v.bao@gmail.com> Co-authored-by: Maxime Brunet <max@brnt.mx> Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Co-authored-by: zirain <zirain2009@gmail.com> Co-authored-by: 聪 <q1875486458@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Inode1 <makarichevivan@gmail.com> Co-authored-by: davem-git <demathieu@gmail.com> Co-authored-by: Kota Kimura <86363983+kkk777-7@users.noreply.github.com> Co-authored-by: Isaac <10012479+jukie@users.noreply.github.com> Co-authored-by: Gonzalo Serrano <boikot@gmail.com> Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com> Co-authored-by: Fabian Bao <fabian.v.bao@gmail.com> Co-authored-by: Ignasi Barrera <nacx@apache.org>

kkk777-7 added 3 commits November 6, 2025 01:37

revert: separate headers with commas

17c7cca

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

add e2e

aa50bd6

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

add release note

c493922

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>

kkk777-7 requested a review from a team as a code owner November 5, 2025 17:40

zirain approved these changes Nov 5, 2025

View reviewed changes

arkodg reviewed Nov 6, 2025

View reviewed changes

arkodg approved these changes Nov 9, 2025

View reviewed changes

arkodg merged commit 89e8256 into envoyproxy:main Nov 9, 2025
53 of 55 checks passed

arkodg mentioned this pull request Nov 14, 2025

fix: handle multi-value response headers as comma-separated per RFC 7230 #6281

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: header modifier doesn't permit multiple values with commas#7436

fix: header modifier doesn't permit multiple values with commas#7436
arkodg merged 3 commits intoenvoyproxy:mainfrom
kkk777-7:fix-commas-header

kkk777-7 commented Nov 5, 2025

Uh oh!

codecov bot commented Nov 5, 2025 •

edited

Loading

Uh oh!

zirain commented Nov 5, 2025

Uh oh!

arkodg Nov 6, 2025

Uh oh!

kkk777-7 Nov 6, 2025

Uh oh!

kkk777-7 Nov 6, 2025

Uh oh!

maevii Nov 6, 2025

Uh oh!

kkk777-7 Nov 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

kkk777-7 commented Nov 5, 2025

Uh oh!

codecov bot commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

zirain commented Nov 5, 2025

Uh oh!

arkodg Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

kkk777-7 Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

kkk777-7 Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

maevii Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

kkk777-7 Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Nov 5, 2025 •

edited

Loading