Skip to content

Commit 2463074

Browse files
authored
Avoid zip extract racing condition by using read+write instead extract (#5707)
Extract also creates the folder hierarchy, however we do not need that, the file itself being extracted to a temporary folder is good enough. Instead we read the content of the zip and then write it. The write is not locked but it's OK to update the same file multiple times given the update operation will not alter the content of the file. By not creating the folder hierarchy (default via extract) we no longer can run into the problem of two parallel extracts both trying to create the folder hierarchy without exists ok flag, and one must fail. Resolves #5223. Signed-off-by: Bernát Gábor <bgabor8@bloomberg.net>
1 parent 2ed84f5 commit 2463074

1 file changed

Lines changed: 18 additions & 3 deletions

File tree

requests/utils.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,13 +256,28 @@ def extract_zipped_paths(path):
256256

257257
# we have a valid zip archive and a valid member of that archive
258258
tmp = tempfile.gettempdir()
259-
extracted_path = os.path.join(tmp, *member.split('/'))
259+
extracted_path = os.path.join(tmp, member.split('/')[-1])
260260
if not os.path.exists(extracted_path):
261-
extracted_path = zip_file.extract(member, path=tmp)
262-
261+
# use read + write to avoid the creating nested folders, we only want the file, avoids mkdir racing condition
262+
with atomic_open(extracted_path) as file_handler:
263+
file_handler.write(zip_file.read(member))
263264
return extracted_path
264265

265266

267+
@contextlib.contextmanager
268+
def atomic_open(filename):
269+
"""Write a file to the disk in an atomic fashion"""
270+
replacer = os.rename if sys.version_info[0] == 2 else os.replace
271+
tmp_descriptor, tmp_name = tempfile.mkstemp(dir=os.path.dirname(filename))
272+
try:
273+
with os.fdopen(tmp_descriptor, 'wb') as tmp_handler:
274+
yield tmp_handler
275+
replacer(tmp_name, filename)
276+
except BaseException:
277+
os.remove(tmp_name)
278+
raise
279+
280+
266281
def from_key_val_list(value):
267282
"""Take an object and test to see if it can be represented as a
268283
dictionary. Unless it can not be represented as such, return an

0 commit comments

Comments
 (0)