Skip to content

Commit

Permalink
Fix race condition
Browse files Browse the repository at this point in the history
  • Loading branch information
andresriancho committed Jun 11, 2019
1 parent 666f517 commit 3494d43
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
21 changes: 19 additions & 2 deletions w3af/core/data/db/history.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ class HistoryItem(object):
_EXTENSION = 'trace'
_MSGPACK_CANARY = 'cute-and-yellow'

_TMP_EXTENSION = 'tmp'

_COMPRESSED_EXTENSION = 'zip'
_COMPRESSED_FILE_BATCH = 150
_UNCOMPRESSED_FILES = 50
Expand Down Expand Up @@ -327,7 +329,10 @@ def load_from_file(self, _id):
# we checked for os.path.exists(file_name) at the beginning of this
# method the file wasn't there yet, but is on disk now
#
return self._load_from_trace_file_concurrent(_id)
if os.path.exists(file_name):
return self._load_from_trace_file_concurrent(_id)

raise TraceReadException('No zip nor trace file for ID %s' % _id)

def _load_from_zip(self, _id):
files = os.listdir(self.get_session_dir())
Expand Down Expand Up @@ -619,6 +624,12 @@ def _process_pending_compression(self, pending_compression):
self._COMPRESSED_EXTENSION)
compressed_filename = os.path.join(session_dir, compressed_filename)

# To prevent race conditions between a thread that is writing the zip
# file and another thread that is attempting to read from it, we first
# write the contents of the zip file to a .tmp file, and when all the
# contents have been written and flushed, rename the file to a zip file
compressed_filename_temp = '%s.%s' % (compressed_filename, self._TMP_EXTENSION)

#
# I run some tests with tarfile to check if tar + gzip or tar + bzip2
# were faster / better suited for this task. This is what I got when
Expand All @@ -644,7 +655,7 @@ def _process_pending_compression(self, pending_compression):
# Summary: If you want to change the compression algorithm make sure
# that it is better than `zip`.
#
_zip = zipfile.ZipFile(file=compressed_filename,
_zip = zipfile.ZipFile(file=compressed_filename_temp,
mode='w',
compression=zipfile.ZIP_DEFLATED)

Expand All @@ -658,6 +669,12 @@ def _process_pending_compression(self, pending_compression):

_zip.close()

# Rename the file to .zip only after all the contents have been flushed
# to disk by .close().
#
# This prevents race conditions
os.rename(compressed_filename_temp, compressed_filename)

#
# And now remove the already compressed files
#
Expand Down
3 changes: 3 additions & 0 deletions w3af/core/data/db/tests/test_history.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ def test_save_load_compressed(self):
compressed_file = os.path.join(h.get_session_dir(), '1-150.zip')
self.assertTrue(os.path.exists(compressed_file))

compressed_file_temp = os.path.join(h.get_session_dir(), '1-150.zip.tmp')
self.assertFalse(os.path.exists(compressed_file_temp))

expected_files = ['%s.trace' % i for i in range(1, HistoryItem._COMPRESSED_FILE_BATCH + 1)]

_zip = zipfile.ZipFile(compressed_file, mode='r')
Expand Down

0 comments on commit 3494d43

Please sign in to comment.