send_response: fix multi-valued response headers#54
Conversation
Joining the values with a comma, like get_headers(true) does, is invalid for some headers, eg. Set-Cookie. Instead, use get_headers(false) to have separate loop iterations for each value of the header. This way we have a separate add_header call for each value, and let haproxy take care of the rest.
|
Thanks for your PR. Can you please add a test: https://github.com/TimWolla/haproxy-auth-request/tree/main/test? Testing is done with VTest. You can find an example in the GitHub Actions config: and in the test README of HAProxy itself: https://github.com/haproxy/haproxy/blob/master/reg-tests/README |
|
Should I add a new test, or extend one of the existing tests? |
|
A new test (or even several new tests if that makes sense), please. I've taken care to ensure that all tests only test some very specific part, as that makes it easier to find the actual issue and it keeps the test simple. |
|
Looks like VTest has no way to check multi-valued headers. It just finds the first occurrence and looks at that. |
Oh, that's not good! Thank you for adding the test, even though it doesn't check everything. It helps me understand the issue and expected solution a little better. I'll see if I can adjust the test to verify this properly. It should be possible to pass the response through a dedicated frontend/backend and perform the counting / checking with HAProxy ACLs. I don't expect you to make any further changes here, I've put it on my TODO to handle the rest. |
|
Now merged (with an improved test). Thanks for your contribution! |
2 participants
Joining the values with a comma, like get_headers(true) does, is invalid for some headers, eg. Set-Cookie.
Instead, use get_headers(false) to have separate loop iterations for each value of the header.
This way we have a separate add_header call for each value, and let haproxy take care of the rest.