Skip to content

Commit c0970a7

Browse files
committed
Prevent file objects unexpectedly being closed by boto after uploading to s3. Fixes #153
1 parent 8f79c0b commit c0970a7

File tree

2 files changed

+38
-0
lines changed

2 files changed

+38
-0
lines changed

tests/storage/test_storage.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,22 @@ def test_fetch_file_contents_using_s3_select_raises_errors(
702702
)
703703
assert len(file_contents) == 0
704704

705+
def test_non_closeable_buffered_reader_does_not_close_underlying_file(self):
706+
# deliberately not using a context manager to
707+
# open/read the file so we can correctly observe
708+
# the closing behavior.
709+
tmp = tempfile.TemporaryFile(mode="w+b")
710+
tmp.write(b"data")
711+
tmp.seek(0)
712+
try:
713+
wrapped = storage.NonCloseableBufferedReader(tmp)
714+
wrapped.close()
715+
716+
# Should have flushed, but NOT closed
717+
assert not tmp.closed
718+
finally:
719+
tmp.close()
720+
705721

706722
class TestMemoryFileStore:
707723
def setup_method(self):

xocto/storage/storage.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,15 @@ class ReadableBinaryFile(Protocol):
125125
def read(self, size: int = ...) -> bytes: ...
126126

127127

128+
class NonCloseableBufferedReader(io.BufferedReader):
129+
def __init__(self, raw: ReadableBinaryFile, *args: Any, **kwargs: Any):
130+
super().__init__(cast(io.RawIOBase, raw), *args, **kwargs)
131+
132+
def close(self) -> None:
133+
# Don't close the underlying file object, just flush the buffer.
134+
self.flush()
135+
136+
128137
class StreamingBodyIOAdapter(io.RawIOBase):
129138
"""
130139
Wrapper to adapt a boto3 S3 object to a standard Python "file-like" object
@@ -467,6 +476,16 @@ def store_file(
467476

468477
readable = _to_stream(contents=contents)
469478

479+
# If `readable` is an actual file-like object(like files from `open()` or Django’s
480+
# `UploadedFile`) wrap it with `NonCloseableBufferedReader` to prevent closing
481+
# by boto3 after upload. No need to do this for BytesIO/StringIO because they are
482+
# in-memory buffers thus do not necessarily need protection from `close()`. Also,
483+
# `BufferedReader` expects a raw binary stream, not a BytesIO or StringIO.
484+
if hasattr(readable, "read") and not isinstance(
485+
readable, (io.BytesIO, io.StringIO)
486+
):
487+
readable = NonCloseableBufferedReader(readable)
488+
470489
# `boto_client.upload_fileobj` is type annotated with `Fileobj: BinaryIO`. However, in
471490
# practice the only file-like method it needs is `read(size=...)`. This cast allows us to
472491
# use `upload_fileobj` with any Fileobj that implements the `ReadableBinaryFile` protocol
@@ -489,6 +508,9 @@ def store_file(
489508
ExtraArgs=extra_args,
490509
)
491510

511+
if isinstance(file_obj, NonCloseableBufferedReader):
512+
file_obj.detach()
513+
492514
return self.bucket_name, key_path
493515

494516
def store_versioned_file(

0 commit comments

Comments
 (0)