Ealenn / Echo-Server

REST Server Tests (Echo-Server) API / Docker / Kubernetes / Helm
https://ealenn.github.io/Echo-Server
GNU General Public License v3.0
265 stars 53 forks source link

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

Open flowdopip opened 12 months ago

flowdopip commented 12 months ago

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

Ealenn commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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)