Question: Allow unsetting headers in merge_*_headers?

[aj-foster]

Hi there 👋🏼

This PR is primarily for discussion; code is just for demonstration.

Would you be interested in allowing folks to remove / unset headers in Plug.Conn.merge_req_headers/2 and Plug.Conn.merge_resp_headers/2? This would potentially allow more succinct management of headers (as opposed to calling the corresponding delete_ functions multiple times).

For context: some headers, like X-Frame-Options, have semantics based on their existence in addition to their value. This means that certain behaviours require removing the header completely if it was previously set. That's possible today with delete_resp_header, but a merge call can be more expressive in certain contexts.

Cheers ❤️

[josevalim]

Thank you! Any thoughts @whatyouhide? The only concern is that if you forget to set a header, you don't get an error, and instead you may even end-up fully erasing a previous value.

I know we have such style of merges elsewhere (although I can't recall where, can you @aj-foster?) but in this case the security implications seem a bit higher.

[aj-foster]

Instead of nil, we could consider using a sentinel value like :unset or similar. Similarly invalid if used today, but less likely to be set accidentally.

Edit: I can't think of an example off the top of my head, but I also recognize the pattern from somewhere.

[josevalim]

If we are going with an explicit value, then maybe :delete is better?

[whatyouhide]

I know we have such style of merges elsewhere (although I can't recall where, can you @aj-foster?) but in this case the security implications seem a bit higher.

I agree. I think I would rather go with the multiple calls to delete_req_header/2 or have delete_req_headers/2 to avoid that (I'd prefer the new function).

[josevalim]

Closing this, as making the function polymorphic could accidentally lead to behaviour where headers are removed when they are not meant to.