Feature Request: Option to use more_set_headers instead of add_header

[realrellek]

Hello,

this is going to be sort of a bug report.

Here's the issue. If you are setting headers in your vhost (in the server {} block), rocket-nginx will overwrite them with its status report headers (if the cache was a HIT, a MISS, or a BYPASS). This is due to how nginx works; it will get its information from the location block that fits best, and if it contains an add_header, it will dismiss all add_headers that might also match on the levels above.

So say you had HTST in your vhost, it's gone when rocket-nginx sends its status report.

My proposed fix is to use more_set_headers instead as this will not make nginx forget headers that are meant to be sent from the level above.

The problem is that it is not a core feature of nginx. On Debian-bases systems, it's the package libnginx-mod-http-headers-more-filter, and it's also included in nginx-extras. That might differ in other distros.

However, I'd say it's a good call to make the admin aware of this, suggest to install it and set a switch to true. Said switch would use more_set_headers instead of add_header.

I've prepared a PR that implements this.

Thank you!

[maximejobin]

This is interesting!

In fact, I've thought of this and decided to do it the other way around!

So you should add all your add_header in a file, then include that file in your Rocket-Nginx configuration and in other configurations you'd like to use.

Adding a dependency is to another module is something I'd like to avoid.

Any thoughts on the implementation we've made?

[realrellek]

I agree adding a dependency is not cool... For default behavior, that is. Standard should remain as it is. If you happen to have more_headers installed anyway, a switch to make your life a little easier (by not having to care about adding "your" other headers, again), is a quality-of-life improvement in my book.

For context, I did the switch from Apache to nginx recently. And I ran into this trap, with rocket-nginx doing add_header and because it's in a location, it therefore voided all the other headers that should be set globally. So... I thought maybe other admins have had the same...

[maximejobin]

You have a valid point and the documentation should be adjusted to further explain how the configuration file works.