Move header_read_timeout_buffered_io feature flag
What does this MR do and why?
In !78065 (merged) we added a timeout for reading header data. We would use a patched Net::HTTP class, which then uses a patched Net::BufferedIO class. This is a risky change because it is overriding two ruby core libraries (Net::HTTP and Net::BufferedIO). To limit the impact, we added two security measures:
- The patched classes are only used if the
use_read_total_timeout
option is used for the Gitlab::HTTP request. - A timeout error is only raised when the
header_read_timeout_buffered_io
feature is active.
In this MR we are only using the patched classes when the header_read_timeout_buffered_io
feature is active and removing the check for the use_read_total_timeout
option because:
- Using the feature flag to control usage of the patched classes lets us better control the rollout. In case something is wrong with classes, we can simply turn it off.
- The
use_read_total_timeout
option was actually intended to control theReadTotalTimeout
and we want to remove the option in a future iteration - An option to opt-out of the timeout is not needed because reading header data is never expected to run into a timeout.
Screenshots or screen recordings
I've recorded a video that explains read total timeout. It is not showing the feature that is touched in this MR but it might be helpful to watch:
How to set up and validate locally
See #336999 (comment 808312938) (confidential)
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.