Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TT-13440] fix: correctly sync multi-value response headers with coprocess middl… #6394

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sebkehr
Copy link

@sebkehr sebkehr commented Jul 8, 2024

Hi,

as this is my first PR to your project please let me know what further information you need for a review.

Description

When synchronizing single- and multi-valued header representations (of coprocess-based middleware responses) the list of values for any multi-valued header is currently replaced by a list containing only the value given by its single-value representation effectively dropping all but the first value. Instead synchronization should affect/replace only the first value and retain possibly remaining values.

Related Issue

fixes #6411

Motivation and Context

We like to employ Tyk Gateway with a coprocess-based response middleware attached to an upstream possibly responding with multiple Set-Cookie headers. We also require our middleware to modify other headers like Location. As is due to synchronization only the first Set-Cookie header passes our middleware.

How This Has Been Tested

This PR adds a test which should fail without proposed fix.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

@sebkehr sebkehr force-pushed the fix_correctly_sync_multivalue_response_headers branch from 8f2e1de to fe1d9c4 Compare July 11, 2024 08:43
@sebkehr
Copy link
Author

sebkehr commented Oct 30, 2024

To clarify: are the proposed changes consistent with the specified semantics in #5173?

@andyo-tyk andyo-tyk changed the title fix: correctly sync multi-value response headers with coprocess middl… [TT-13440] fix: correctly sync multi-value response headers with coprocess middl… Oct 30, 2024
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.

[TT-13440] Multi-value response headers lost after sync with coprocess middleware
1 participant