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

[BUG] Support Multiple custom headers with comma value #148

Open
flowdopip opened this issue Nov 13, 2023 · 3 comments
Open

[BUG] Support Multiple custom headers with comma value #148

flowdopip opened this issue Nov 13, 2023 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@flowdopip
Copy link

Describe the bug
I want to emulate several upstream response with cache-control header. As you know cache-control policy can have several values with "," as a delimiter.
Example:
Cache-Control: max-age=300, public.must-revalidate

Current the code is splitting the custom headers with "," and the proposal is to switch to ";" in order to support several values on a header

To Reproduce
X-ECHO-HEADER=Cache-Control: max-age=300, public; One: 1

Expected behavior
The response should have the following headers:
Cache-Control: max-age=300, public
One: 1

Additional context
I can open a PR if you want to fix this issue

@flowdopip flowdopip added the bug Something isn't working label Nov 13, 2023
@Ealenn
Copy link
Owner

Ealenn commented Nov 14, 2023

Hello ! Thanks for this issue. You're right! It's probably better to have configuration about that. The first reason is the potential breaking change... What do you think about that ?

I have to maintain this repository and the related Helm chart, I planned to do it this weekend or in few weeks. So, as you want 🤷‍♂️

@flowdopip
Copy link
Author

You may control the breaking change, but it will add extra complexity. For instance, you can support the split by "," but you need to guarantee that the next item should be something [a-zA-z]: that is the only way to support the current behaviour.
IMHO, you can launch a major change and avoid that complexity and put your service working as expected.

@Ealenn
Copy link
Owner

Ealenn commented Nov 19, 2023

Probably not a big deal to do a breaking change... The Docker Image version will increase so 🤷‍♂️

However, I'm sorry I can't do the MR now 👀 But probably during this week (some personal things to do)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants