chore(storage): Error handling for open and unlinked files - #35031
chore(storage): Error handling for open and unlinked files#35031shubhangi-google wants to merge 3 commits into
Conversation
bc63dcf to
92e3408
Compare
| @@ -63,33 +63,35 @@ def self.crc32c_for local_file | |||
| # Computes a base64-encoded digest for a local file or IO stream. | |||
| # | |||
| # This method handles two types of inputs for `local_file`: | |||
| # 1. A file path (String or Pathname): It efficiently streams the file | |||
There was a problem hiding this comment.
Why are we removing this?
There was a problem hiding this comment.
we are swapping the order of Case 1 and Case 2 to match the updated logic below. We need to check if the object is an IO-like stream (i.e., responds to :read) before checking if it's a file path. If we check for the file path first, an unlinked Tempfile will still return true for :to_path, but attempting to open that path will raise an Errno::ENOENT error. Checking if we can read it directly (:read) first fixes this.
| # reading to ensure its position is not permanently changed. | ||
| # 2. A file path (String or Pathname): It efficiently streams the file |
There was a problem hiding this comment.
I'm not sure if this is true
There was a problem hiding this comment.
reverted to previous version
| # Case 1: Input is a file path (String, Pathname, or object that responds to :to_path). | ||
| ::File.open Pathname(local_file).to_path, "rb" do |f| | ||
| digest_class.file(f).base64digest | ||
| if local_file.respond_to?(:read) |
There was a problem hiding this comment.
Why are we removing existing code?
There was a problem hiding this comment.
we are swapping the existing code to check for the :read capability (the IO-stream case) before falling back to the :to_path capability (the file path case).
2 participants
Fixes issue where uploading unlinked temporary files (such as large payloads handled by the Puma web server) fails during the CRC32c digest computation.
closes #34987