Repeated headers are occluded incorrectly

[sevmonster]

For example, if your proxy returns a Set-Cookie header, and you attempt to add a new Set-Cookie header to the response, all the previous Set-Cookie headers will be lost. This functionality makes sense for headers that should only be specified once, but for multi-use headers like Set-Cookie the list should be appended instead of replaced. My particular use case for this was adding an upstream cookie after an auth_request.

If this is an intended feature as #18 suggests, maybe it should be added to the README.

[zhuizhuhaomeng]

I think add an append mode is a good choice.

[lynch1981]

I added this feature, please check out my patch when you have time. @zhuizhuhaomeng

[milas]

I see #151 was merged but limited to Set-Cookie. Since it's behind a new -a (append) flag, would you accept a PR to remove the Set-Cookie restriction and allow it for any header?

I'd like to use it with X-Robots-Tag, so I can target specific crawlers, which requires multiple headers. (See https://http.dev/x-robots-tag#targeting-specific-web-crawlers.)

[lynch1981]

Most headers cannot be repeated, so removing the Set-Cookie restriction is not a good idea. And we can't have a complete list of which headers can support repeated additions. So as agentzh said, it is recommended that you use the ngx_lua module to do this.

[zhuizhuhaomeng]

@lynch1981 @milas Removing the restriction is acceptable to me.
because removing the restriction won't break the backward compatibility.

It is the user's responsibility to choose the right option of more_set_headers.

[lynch1981]

@zhuizhuhaomeng @milas
Let me fix this problem. There needs to be clear instructions in the readme file.

[lynch1981]

@zhuizhuhaomeng I think the behavior of built-in headers should not be changed, your opinion?

[zhuizhuhaomeng]

@zhuizhuhaomeng I think the behavior of built-in headers should not be changed, your opinion?

Yes.
Adding checks is great, but don't make the code too complicated.

[zhuizhuhaomeng]

I have reviewed the code.
We should return an error when -a option is on the built-in header.

[lynch1981]

Good idea, this makes it clearer and easier for users to use

[milas]

I believe this was resolved with #152 and v0.36, so this issue can likely be closed now. Thanks!

[sevmonster]

Thank you, I had forgotten to close this.