Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove uses of securesystemslib.util.TempFile #918

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions tests/test_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,11 @@ def test_download_url_to_tempfileobj(self):
download_file = download.safe_download

temp_fileobj = download_file(self.url, self.target_data_length)
self.assertEqual(self.target_data, temp_fileobj.read().decode('utf-8'))
self.assertEqual(self.target_data_length, len(temp_fileobj.read()))
temp_fileobj.close_temp_file()
temp_fileobj.seek(0)
temp_file_data = temp_fileobj.read().decode('utf-8')
self.assertEqual(self.target_data, temp_file_data)
self.assertEqual(self.target_data_length, len(temp_file_data))
temp_fileobj.close()



Expand Down Expand Up @@ -158,7 +160,7 @@ def test_download_url_to_tempfileobj_and_performance(self):

self.assertEqual(self.target_data, temp_fileobj.read())
self.assertEqual(self.target_data_length, len(temp_fileobj.read()))
temp_fileobj.close_temp_file()
temp_fileobj.close()

print "Performance cpu time: "+str(end_cpu - star_cpu)
print "Performance real time: "+str(end_real - star_real)
Expand Down
6 changes: 3 additions & 3 deletions tests/test_updater.py
Original file line number Diff line number Diff line change
Expand Up @@ -1568,7 +1568,7 @@ def test_9__get_target_hash(self):

def test_10__hard_check_file_length(self):
# Test for exception if file object is not equal to trusted file length.
temp_file_object = securesystemslib.util.TempFile()
temp_file_object = tempfile.TemporaryFile()
temp_file_object.write(b'X')
temp_file_object.seek(0)
self.assertRaises(tuf.exceptions.DownloadLengthMismatchError,
Expand All @@ -1581,7 +1581,7 @@ def test_10__hard_check_file_length(self):

def test_10__soft_check_file_length(self):
# Test for exception if file object is not equal to trusted file length.
temp_file_object = securesystemslib.util.TempFile()
temp_file_object = tempfile.TemporaryFile()
temp_file_object.write(b'XXX')
temp_file_object.seek(0)
self.assertRaises(tuf.exceptions.DownloadLengthMismatchError,
Expand Down Expand Up @@ -1704,7 +1704,7 @@ def test_10__visit_child_role(self):

def test_11__verify_uncompressed_metadata_file(self):
# Test for invalid metadata content.
metadata_file_object = securesystemslib.util.TempFile()
metadata_file_object = tempfile.TemporaryFile()
metadata_file_object.write(b'X')
metadata_file_object.seek(0)

Expand Down
65 changes: 31 additions & 34 deletions tuf/client/updater.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@
import fnmatch
import copy
import warnings
import tempfile

import tuf
import tuf.download
Expand Down Expand Up @@ -1129,6 +1130,7 @@ def _update_root_metadata(self, current_root_metadata):
latest_root_metadata_file = self._get_metadata_file(
'root', 'root.json', DEFAULT_ROOT_UPPERLENGTH, None)

latest_root_metadata_file.seek(0)
latest_root_metadata = securesystemslib.util.load_json_string(
latest_root_metadata_file.read().decode('utf-8'))

Expand Down Expand Up @@ -1161,9 +1163,7 @@ def _check_hashes(self, file_object, trusted_hashes):

<Arguments>
file_object:
A 'securesystemslib.util.TempFile' file-like object. 'file_object'
ensures that a read() without a size argument properly reads the entire
file.
A file object.

trusted_hashes:
A dictionary with hash-algorithm names as keys and hashes as dict values.
Expand All @@ -1175,6 +1175,7 @@ def _check_hashes(self, file_object, trusted_hashes):

<Side Effects>
Hash digest object is created using the 'securesystemslib.hash' module.
Position within file_object is changed.

<Returns>
None.
Expand All @@ -1184,6 +1185,10 @@ def _check_hashes(self, file_object, trusted_hashes):
# return.
for algorithm, trusted_hash in six.iteritems(trusted_hashes):
digest_object = securesystemslib.hash.digest(algorithm)
# Ensure we read from the beginning of the file object
# TODO: should we store file position (before the loop) and reset after we
# seek about?
file_object.seek(0)
digest_object.update(file_object.read())
computed_hash = digest_object.hexdigest()

Expand All @@ -1210,9 +1215,7 @@ def _hard_check_file_length(self, file_object, trusted_file_length):

<Arguments>
file_object:
A 'securesystemslib.util.TempFile' file-like object. 'file_object'
ensures that a read() without a size argument properly reads the entire
file.
A file object.

trusted_file_length:
A non-negative integer that is the trusted length of the file.
Expand All @@ -1223,14 +1226,13 @@ def _hard_check_file_length(self, file_object, trusted_file_length):
<Side Effects>
Reads the contents of 'file_object' and logs a message if 'file_object'
matches the trusted length.
Position within file_object is changed.

<Returns>
None.
"""

# Read the entire contents of 'file_object', a
# 'securesystemslib.util.TempFile' file-like object that ensures the entire
# file is read.
file_object.seek(0)
observed_length = len(file_object.read())

# Return and log a message if the length 'file_object' is equal to
Expand All @@ -1252,17 +1254,14 @@ def _hard_check_file_length(self, file_object, trusted_file_length):
def _soft_check_file_length(self, file_object, trusted_file_length):
"""
<Purpose>
Non-public method that checks the trusted file length of a
'securesystemslib.util.TempFile' file-like object. The length of the file
must be less than or equal to the expected length. This is a deliberately
redundant implementation designed to complement
tuf.download._check_downloaded_length().
Non-public method that checks the trusted file length of a file object.
The length of the file must be less than or equal to the expected
length. This is a deliberately redundant implementation designed to
complement tuf.download._check_downloaded_length().

<Arguments>
file_object:
A 'securesystemslib.util.TempFile' file-like object. 'file_object'
ensures that a read() without a size argument properly reads the entire
file.
A file object.

trusted_file_length:
A non-negative integer that is the trusted length of the file.
Expand All @@ -1274,14 +1273,14 @@ def _soft_check_file_length(self, file_object, trusted_file_length):
<Side Effects>
Reads the contents of 'file_object' and logs a message if 'file_object'
is less than or equal to the trusted length.
Position within file_object is changed.

<Returns>
None.
"""

# Read the entire contents of 'file_object', a
# 'securesystemslib.util.TempFile' file-like object that ensures the entire
# file is read.
file_object.seek(0)
observed_length = len(file_object.read())

# Return and log a message if 'file_object' is less than or equal to
Expand Down Expand Up @@ -1329,7 +1328,7 @@ def _get_target_file(self, target_filepath, file_length, file_hashes):
a temporary file and returned.

<Returns>
A 'securesystemslib.util.TempFile' file-like object containing the target.
A file object containing the target.
"""

# Define a callable function that is passed as an argument to _get_file()
Expand Down Expand Up @@ -1365,9 +1364,7 @@ def _verify_uncompressed_metadata_file(self, metadata_file_object,

<Arguments>
metadata_file_object:
A 'securesystemslib.util.TempFile' instance containing the metadata
file. 'metadata_file_object' ensures the entire file is returned with
read().
A file object containing the metadata file.

metadata_role:
The role name of the metadata (e.g., 'root', 'targets',
Expand All @@ -1391,12 +1388,14 @@ def _verify_uncompressed_metadata_file(self, metadata_file_object,
In case the metadata file does not have a valid signature.

<Side Effects>
The content of 'metadata_file_object' is read and loaded.
The content of 'metadata_file_object' is read and loaded, the current
position within the file is changed.

<Returns>
None.
"""

metadata_file_object.seek(0)
metadata = metadata_file_object.read().decode('utf-8')

try:
Expand Down Expand Up @@ -1463,8 +1462,7 @@ def _get_metadata_file(self, metadata_role, remote_filename,
file and returned.

<Returns>
A 'securesystemslib.util.TempFile' file-like object containing the
metadata.
A file object containing the metadata.
"""

file_mirrors = tuf.mirrors.get_list_of_mirrors('meta', remote_filename,
Expand All @@ -1478,6 +1476,7 @@ def _get_metadata_file(self, metadata_role, remote_filename,
try:
file_object = tuf.download.unsafe_download(file_mirror,
upperbound_filelength)
file_object.seek(0)

# Verify 'file_object' according to the callable function.
# 'file_object' is also verified if decompressed above (i.e., the
Expand Down Expand Up @@ -1608,8 +1607,8 @@ def _get_file(self, filepath, verify_file_function, file_type, file_length,
The relative metadata or target filepath.

verify_file_function:
A callable function that expects a 'securesystemslib.util.TempFile'
file-like object and raises an exception if the file is invalid.
A callable function that expects a file object and raises an exception
if the file is invalid.
Target files and uncompressed versions of metadata may be verified with
'verify_file_function'.

Expand Down Expand Up @@ -1637,8 +1636,7 @@ def _get_file(self, filepath, verify_file_function, file_type, file_length,
file and returned.

<Returns>
A 'securesystemslib.util.TempFile' file-like object containing the
metadata or target.
A file object containing the metadata or target.
"""

file_mirrors = tuf.mirrors.get_list_of_mirrors(file_type, filepath,
Expand Down Expand Up @@ -1777,12 +1775,11 @@ def _update_metadata(self, metadata_role, upperbound_filelength, version=None):
shutil.move(current_filepath, previous_filepath)

# Next, move the verified updated metadata file to the 'current' directory.
# Note that the 'move' method comes from securesystemslib.util's TempFile class.
# 'metadata_file_object' is an instance of securesystemslib.util.TempFile.
metadata_file_object.seek(0)
metadata_signable = \
securesystemslib.util.load_json_string(metadata_file_object.read().decode('utf-8'))

metadata_file_object.move(current_filepath)
securesystemslib.util.persist_temp_file(metadata_file_object, current_filepath)

# Extract the metadata object so we can store it to the metadata store.
# 'current_metadata_object' set to 'None' if there is not an object
Expand Down Expand Up @@ -3249,4 +3246,4 @@ def download_target(self, target, destination_directory):
else:
raise

target_file_object.move(destination)
securesystemslib.util.persist_temp_file(target_file_object, destination)
42 changes: 10 additions & 32 deletions tuf/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,7 @@
<Purpose>
Download metadata and target files and check their validity. The hash and
length of a downloaded file has to match the hash and length supplied by the
metadata of that file. The downloaded file is technically a file-like
object that will automatically destroys itself once closed. Note that the
file-like object, 'securesystemslib.util.TempFile', is returned by the
'_download_file()' function.
metadata of that file.
"""

# Help with Python 3 compatibility, where the print statement is a function, an
Expand All @@ -37,6 +34,7 @@
import logging
import time
import timeit
import tempfile

import tuf
import requests
Expand Down Expand Up @@ -76,11 +74,6 @@ def safe_download(url, required_length):
tuf.download.unsafe_download() may be called if an upper download limit is
preferred.

'securesystemslib.util.TempFile', the file-like object returned, is used
instead of regular tempfile object because of additional functionality
provided, such as handling compressed metadata and automatically closing
files after moving to final destination.

<Arguments>
url:
A URL string that represents the location of the file.
Expand All @@ -90,8 +83,7 @@ def safe_download(url, required_length):
limit.

<Side Effects>
A 'securesystemslib.util.TempFile' object is created on disk to store the
contents of 'url'.
A file object is created on disk to store the contents of 'url'.

<Exceptions>
tuf.ssl_commons.exceptions.DownloadLengthMismatchError, if there was a
Expand All @@ -103,8 +95,7 @@ def safe_download(url, required_length):
Any other unforeseen runtime exception.

<Returns>
A 'securesystemslib.util.TempFile' file-like object that points to the
contents of 'url'.
A file object that points to the contents of 'url'.
"""

# Do all of the arguments have the appropriate format?
Expand All @@ -127,11 +118,6 @@ def unsafe_download(url, required_length):
tuf.download.safe_download() may be called if an exact download limit is
preferred.

'securesystemslib.util.TempFile', the file-like object returned, is used
instead of regular tempfile object because of additional functionality
provided, such as handling compressed metadata and automatically closing
files after moving to final destination.

<Arguments>
url:
A URL string that represents the location of the file.
Expand All @@ -141,8 +127,7 @@ def unsafe_download(url, required_length):
limit.

<Side Effects>
A 'securesystemslib.util.TempFile' object is created on disk to store the
contents of 'url'.
A file object is created on disk to store the contents of 'url'.

<Exceptions>
tuf.ssl_commons.exceptions.DownloadLengthMismatchError, if there was a
Expand All @@ -154,8 +139,7 @@ def unsafe_download(url, required_length):
Any other unforeseen runtime exception.

<Returns>
A 'securesystemslib.util.TempFile' file-like object that points to the
contents of 'url'.
A file object that points to the contents of 'url'.
"""

# Do all of the arguments have the appropriate format?
Expand All @@ -178,10 +162,6 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True):
the file's length is not checked and a slow retrieval exception is raised
if the downloaded rate falls below the acceptable rate).

securesystemslib.util.TempFile is used instead of regular tempfile object
because of additional functionality provided by
'securesystemslib.util.TempFile'.

<Arguments>
url:
A URL string that represents the location of the file.
Expand All @@ -196,8 +176,7 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True):
timestamp metadata, which has no signed required_length.

<Side Effects>
A 'securesystemslib.util.TempFile' object is created on disk to store the
contents of 'url'.
A file object is created on disk to store the contents of 'url'.

<Exceptions>
tuf.exceptions.DownloadLengthMismatchError, if there was a
Expand All @@ -209,8 +188,7 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True):
Any other unforeseen runtime exception.

<Returns>
A 'securesystemslib.util.TempFile' file-like object that points to the
contents of 'url'.
A file object that points to the contents of 'url'.
"""

# Do all of the arguments have the appropriate format?
Expand All @@ -228,7 +206,7 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True):

# This is the temporary file that we will return to contain the contents of
# the downloaded file.
temp_file = securesystemslib.util.TempFile()
temp_file = tempfile.TemporaryFile()

try:
# Use a different requests.Session per schema+hostname combination, to
Expand Down Expand Up @@ -304,7 +282,7 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True):

except Exception:
# Close 'temp_file'. Any written data is lost.
temp_file.close_temp_file()
temp_file.close()
logger.exception('Could not download URL: ' + repr(url))
raise

Expand Down
Loading