Update fault injection to use a single x-bp-fault header.

[HiramSilvey]

💸 TL;DR

This PR consolidates all of the fault injection headers into one. This is necessary to allow multiple fault injection configurations on a single request.

📜 Details

DM me for the doc.

Note that this PR removes support for the original x-bp-fault- headers. My understanding is that this is OK since this is a new unused feature. If backwards compatibility is required, however, let me know and I can update this to ensure the previous headers are still read as backups.

🧪 Testing Steps / Validation

This was tested solely through comprehensive unit testing.

✅ Checks

• CI tests (if present) are passing
• Adheres to code style for repo
• Contributor License Agreement (CLA) completed if not a Reddit employee

[pacejackson]

headerbp only forwards a single value so I'm not sure that this will work?

[HiramSilvey]

headerbp only forwards a single value so I'm not sure that this will work?

That's fine if the HTTP headers are combined as per RFC 9110. Follow-up question: If headerbp sees multiple values for 1 header which aren't combined into a single line yet, can it support joining them before it forwards?

[pacejackson]

That's fine if the HTTP headers are combined as per RFC 9110.

headerbp isn't just concerned with HTTP, it also has to care about thrift and gRPC, thrift in particular just has a single value per header.

If headerbp sees multiple values for 1 header which aren't combined into a single line yet, can it support joining them before it forwards?

It currently doesn't even support this, it only accepts a single value for each header.

There are solutions for this, but one major concern with headerbp in general is request size and supporting potentially an unbounded list is another vector through which this could be used to blow up request sizes or cause issues with headers being chopped off because the size is too big.

[HiramSilvey]

That's fine if the HTTP headers are combined as per RFC 9110.

headerbp isn't just concerned with HTTP, it also has to care about thrift and gRPC, thrift in particular just has a single value per header.

If headerbp sees multiple values for 1 header which aren't combined into a single line yet, can it support joining them before it forwards?

It currently doesn't even support this, it only accepts a single value for each header.

There are solutions for this, but one major concern with headerbp in general is request size and supporting potentially an unbounded list is another vector through which this could be used to blow up request sizes or cause issues with headers being chopped off because the size is too big.

Followed up offline. headerbp only forwarding a single header is perfectly acceptable as the code that populates x-bp-fault can ensure it's only a single value. Size issues are a valid concern and a limit may need to be defined and enforced either in headerbp or in the code that populates x-bp-fault.

[pacejackson]

Note that this PR removes support for the original x-bp-fault- headers. My understanding is that this is OK since this is a new unused feature. If backwards compatibility is required, however, let me know and I can update this to ensure the previous headers are still read as backups.

I think that should be fine, I don't think anyone actively used these headers yet?

[HiramSilvey]

Note that this PR removes support for the original x-bp-fault- headers. My understanding is that this is OK since this is a new unused feature. If backwards compatibility is required, however, let me know and I can update this to ensure the previous headers are still read as backups.

I think that should be fine, I don't think anyone actively used these headers yet?

Internally that is correct.