Skip to content

Draft: Block any attempt to read the body of X-Sendfile responses

Robert May requested to merge action-dispatch-patch into master

What does this MR do and why?

Prevents the reading of the response body on X-Sendfile request/response. When returning large static files we use Rails' send_file method to return an X-Sendfile header with the path of the file, and the reverse proxy in front of the Rails app delivers this file to the client, rather than either reading the entire file into memory in Ruby or streaming it (which will keep a Ruby process entirely occupied until finished). This affects self-hosted installs using local file storage, rather than object storage.

However, even if this is the case, an attempt to read the body of the response will result in Rails loading the file into memory, which is somewhat of an issue when it comes to LFS objects, job artifacts, etc.

This patches a part of ActionDispatch::Response to return a modified subclass of ActionDispatch::Response::FileBody when using send_file, which throws an exception if there's an attempt to read the file body.

Curiously, we might not even need to turn on the feature flag, as the primary use case of this is preventing bugs being introduced when developing new features, and the flag is enabled by default in the test environment.

Related #419239 (closed)

Related #420738 (closed)

Related #442048

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Robert May

Merge request reports