http: add removeIf() header map function

[mattklein123]

To be used by out of tree extensions.

Risk Level: Low
Testing: New tests and existing
Docs Changes: N/A
Release Notes: N/A

[mattklein123]

cc @jessicayuen @LisaLudique

[jmarantz]

Couple of minor comments; I'm fine leaving things as they are too.

[mattklein123]

@jmarantz updated

[rgs1]

Hmm I am seeing appends in the :Authority header (e.g.: :authority = foo,bar -- which did not happened before) after this change... could it be related?

[rgs1]

Just confirmed -- reverting this change makes the appends in :authority go away. I'll dig in.

Clarification:

The issue started somewhere between these changes:

Added watchdog support for a Multi-Kill threshold. #12108
direct response filter: added unit test cases #12064
make header_order.py python3 #12193
dev: remove pcap workaround from devcontainer #12191
Revert "tcp: switching to the new pool #12180" #12192
security: add weekly patches #12156
fuzz: fix oss-fuzz crash in route_fuzz_test due to validation #12176
cleanup: removing tracer well_known_name file #12187
hds: change HdsCluster::cluster_ to a value instead of reference #12137
http: add removeIf() header map function #12160
cleanup: clang tidy naming #12181

I am going to revert just #12160 locally, to confirm it's causing this.

[rgs1]

Ok -- per @fishcakez this is probably related to the fact that we historically used request_headers_to_add to manipulate the host header, when we should have used host_rewrite.

So that's probably the issue, I'll confirm in a bit. We might still want to do something about this, since it might bite others.

[mattklein123]

Hmm, I wouldn't think there would be a behavior change here but remove() did change slightly to not check for it being an O(1) header and to do a full scan. I can fix this if we think this is going to be an issue. @rgs1 can you provide a more complete description of the flow causing the issue?

[rgs1]

Instead of using host_rewrite we had:

             "request_headers_to_add": [
              {
                "append": false,
                "header": {
                  "key": "host",
                  "value": "api.foo.com"
                }
              },

After this change, append: false isn't honored anymore and we get :authority = foo,bar.

I think it's more of our mistake for manipulating the host header directly instead of using host_rewrite, than
a regression since this is technically an incorrect way of doing a host rewrite.

If we don't fix it we could at least clarify the docs or disallow referencing the host header
in requests_headers_to_add? Happy to send a PR for either one.

[mattklein123]

OK let me look at this a bit and get back to you.

[mattklein123]

OK I see the issue, basically the old path was doing the host -> authority translation and the new path is not.

@rgs1 I agree to be on the safe side I will go ahead and fix this. Can I fix this tomorrow or do you need this reverted urgently?

[rgs1]

OK I see the issue, basically the old path was doing the host -> authority translation and the new path is not.

@rgs1 I agree to be on the safe side I will go ahead and fix this. Can I fix this tomorrow or do you need this reverted urgently?

No rush on our end, we got unblocked by moving over to using host_rewrite. Thanks for the quick return!