Merge pull request #1475 from onionshare/1470_tempfiles

Stop storing temporary files in /tmp
This commit is contained in:
Micah Lee 2021-12-03 07:56:20 -08:00 committed by GitHub
commit 85ac5d1ef3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 117 additions and 89 deletions

View file

@ -40,9 +40,6 @@ class OnionShare(object):
self.onion_host = None self.onion_host = None
self.port = None self.port = None
# files and dirs to delete on shutdown
self.cleanup_filenames = []
# do not use tor -- for development # do not use tor -- for development
self.local_only = local_only self.local_only = local_only
@ -75,7 +72,9 @@ class OnionShare(object):
if self.local_only: if self.local_only:
self.onion_host = f"127.0.0.1:{self.port}" self.onion_host = f"127.0.0.1:{self.port}"
if not mode_settings.get("general", "public"): if not mode_settings.get("general", "public"):
self.auth_string = "E2GOT5LTUTP3OAMRCRXO4GSH6VKJEUOXZQUC336SRKAHTTT5OVSA" self.auth_string = (
"E2GOT5LTUTP3OAMRCRXO4GSH6VKJEUOXZQUC336SRKAHTTT5OVSA"
)
return return
self.onion_host = self.onion.start_onion_service( self.onion_host = self.onion.start_onion_service(

View file

@ -42,10 +42,11 @@ class SendBaseModeWeb:
self.is_zipped = False self.is_zipped = False
self.download_filename = None self.download_filename = None
self.download_filesize = None self.download_filesize = None
self.gzip_filename = None
self.gzip_filesize = None
self.zip_writer = None self.zip_writer = None
# Store the tempfile objects here, so when they're garbage collected the files are deleted
self.gzip_files = []
# If autostop_sharing, only allow one download at a time # If autostop_sharing, only allow one download at a time
self.download_in_progress = False self.download_in_progress = False
@ -192,12 +193,15 @@ class SendBaseModeWeb:
# gzip compress the individual file, if it hasn't already been compressed # gzip compress the individual file, if it hasn't already been compressed
if use_gzip: if use_gzip:
if filesystem_path not in self.gzip_individual_files: if filesystem_path not in self.gzip_individual_files:
gzip_filename = tempfile.mkstemp("wb+")[1] self.gzip_files.append(
self._gzip_compress(filesystem_path, gzip_filename, 6, None) tempfile.NamedTemporaryFile("wb+", dir=self.common.build_tmp_dir())
self.gzip_individual_files[filesystem_path] = gzip_filename )
gzip_file = self.gzip_files[-1]
self._gzip_compress(filesystem_path, gzip_file.name, 6, None)
self.gzip_individual_files[filesystem_path] = gzip_file.name
# Make sure the gzip file gets cleaned up when onionshare stops # Cleanup this temp file
self.web.cleanup_filenames.append(gzip_filename) self.web.cleanup_tempfiles.append(gzip_file)
file_to_download = self.gzip_individual_files[filesystem_path] file_to_download = self.gzip_individual_files[filesystem_path]
filesize = os.path.getsize(self.gzip_individual_files[filesystem_path]) filesize = os.path.getsize(self.gzip_individual_files[filesystem_path])

View file

@ -134,8 +134,12 @@ class ShareModeWeb(SendBaseModeWeb):
The web app routes for sharing files The web app routes for sharing files
""" """
@self.web.app.route("/", defaults={"path": ""}, methods=["GET"], provide_automatic_options=False) @self.web.app.route(
@self.web.app.route("/<path:path>", methods=["GET"], provide_automatic_options=False) "/", defaults={"path": ""}, methods=["GET"], provide_automatic_options=False
)
@self.web.app.route(
"/<path:path>", methods=["GET"], provide_automatic_options=False
)
def index(path): def index(path):
""" """
Render the template for the onionshare landing page. Render the template for the onionshare landing page.
@ -159,7 +163,9 @@ class ShareModeWeb(SendBaseModeWeb):
return self.render_logic(path) return self.render_logic(path)
@self.web.app.route("/download", methods=["GET"], provide_automatic_options=False) @self.web.app.route(
"/download", methods=["GET"], provide_automatic_options=False
)
def download(): def download():
""" """
Download the zip file. Download the zip file.
@ -183,7 +189,7 @@ class ShareModeWeb(SendBaseModeWeb):
# and serve that # and serve that
use_gzip = self.should_use_gzip() use_gzip = self.should_use_gzip()
if use_gzip: if use_gzip:
file_to_download = self.gzip_filename file_to_download = self.gzip_file.name
self.filesize = self.gzip_filesize self.filesize = self.gzip_filesize
etag = self.gzip_etag etag = self.gzip_etag
else: else:
@ -286,7 +292,9 @@ class ShareModeWeb(SendBaseModeWeb):
if if_unmod: if if_unmod:
if_date = parse_date(if_unmod) if_date = parse_date(if_unmod)
if if_date and not if_date.tzinfo: if if_date and not if_date.tzinfo:
if_date = if_date.replace(tzinfo=timezone.utc) # Compatible with Flask < 2.0.0 if_date = if_date.replace(
tzinfo=timezone.utc
) # Compatible with Flask < 2.0.0
if if_date and if_date > last_modified: if if_date and if_date > last_modified:
abort(412) abort(412)
elif range_header is None: elif range_header is None:
@ -459,7 +467,7 @@ class ShareModeWeb(SendBaseModeWeb):
return self.web.error404(history_id) return self.web.error404(history_id)
def build_zipfile_list(self, filenames, processed_size_callback=None): def build_zipfile_list(self, filenames, processed_size_callback=None):
self.common.log("ShareModeWeb", "build_zipfile_list") self.common.log("ShareModeWeb", "build_zipfile_list", f"filenames={filenames}")
for filename in filenames: for filename in filenames:
info = { info = {
"filename": filename, "filename": filename,
@ -484,23 +492,25 @@ class ShareModeWeb(SendBaseModeWeb):
self.download_etag = make_etag(f) self.download_etag = make_etag(f)
# Compress the file with gzip now, so we don't have to do it on each request # Compress the file with gzip now, so we don't have to do it on each request
self.gzip_filename = tempfile.mkstemp("wb+")[1] self.gzip_file = tempfile.NamedTemporaryFile(
self._gzip_compress( "wb+", dir=self.common.build_tmp_dir()
self.download_filename, self.gzip_filename, 6, processed_size_callback
) )
self.gzip_filesize = os.path.getsize(self.gzip_filename) self._gzip_compress(
with open(self.gzip_filename, "rb") as f: self.download_filename, self.gzip_file.name, 6, processed_size_callback
)
self.gzip_filesize = os.path.getsize(self.gzip_file.name)
with open(self.gzip_file.name, "rb") as f:
self.gzip_etag = make_etag(f) self.gzip_etag = make_etag(f)
# Make sure the gzip file gets cleaned up when onionshare stops
self.web.cleanup_filenames.append(self.gzip_filename)
self.is_zipped = False self.is_zipped = False
# Cleanup this tempfile
self.web.cleanup_tempfiles.append(self.gzip_file)
else: else:
# Zip up the files and folders # Zip up the files and folders
self.zip_writer = ZipWriter( self.zip_writer = ZipWriter(
self.common, processed_size_callback=processed_size_callback self.common, self.web, processed_size_callback=processed_size_callback
) )
self.download_filename = self.zip_writer.zip_filename self.download_filename = self.zip_writer.zip_filename
for info in self.file_info["files"]: for info in self.file_info["files"]:
@ -519,10 +529,6 @@ class ShareModeWeb(SendBaseModeWeb):
with open(self.download_filename, "rb") as f: with open(self.download_filename, "rb") as f:
self.download_etag = make_etag(f) self.download_etag = make_etag(f)
# Make sure the zip file gets cleaned up when onionshare stops
self.web.cleanup_filenames.append(self.zip_writer.zip_filename)
self.web.cleanup_filenames.append(self.zip_writer.zip_temp_dir)
self.is_zipped = True self.is_zipped = True
return True return True
@ -535,17 +541,24 @@ class ZipWriter(object):
filename. filename.
""" """
def __init__(self, common, zip_filename=None, processed_size_callback=None): def __init__(
self, common, web=None, zip_filename=None, processed_size_callback=None
):
self.common = common self.common = common
self.web = web
self.cancel_compression = False self.cancel_compression = False
if zip_filename: if zip_filename:
self.zip_filename = zip_filename self.zip_filename = zip_filename
else: else:
self.zip_temp_dir = tempfile.mkdtemp() self.zip_temp_dir = tempfile.TemporaryDirectory(
self.zip_filename = ( dir=self.common.build_tmp_dir()
f"{self.zip_temp_dir}/onionshare_{self.common.random_string(4, 6)}.zip"
) )
self.zip_filename = f"{self.zip_temp_dir.name}/onionshare_{self.common.random_string(4, 6)}.zip"
# Cleanup this temp dir
if self.web:
self.web.cleanup_tempdirs.append(self.zip_temp_dir)
self.z = zipfile.ZipFile(self.zip_filename, "w", allowZip64=True) self.z = zipfile.ZipFile(self.zip_filename, "w", allowZip64=True)
self.processed_size_callback = processed_size_callback self.processed_size_callback = processed_size_callback

View file

@ -155,7 +155,8 @@ class Web:
self.socketio.init_app(self.app) self.socketio.init_app(self.app)
self.chat_mode = ChatModeWeb(self.common, self) self.chat_mode = ChatModeWeb(self.common, self)
self.cleanup_filenames = [] self.cleanup_tempfiles = []
self.cleanup_tempdirs = []
def get_mode(self): def get_mode(self):
if self.mode == "share": if self.mode == "share":
@ -198,18 +199,19 @@ class Web:
""" """
for header, value in self.security_headers: for header, value in self.security_headers:
r.headers.set(header, value) r.headers.set(header, value)
# Set a CSP header unless in website mode and the user has disabled it # Set a CSP header unless in website mode and the user has disabled it
default_csp = "default-src 'self'; frame-ancestors 'none'; form-action 'self'; base-uri 'self'; img-src 'self' data:;" default_csp = "default-src 'self'; frame-ancestors 'none'; form-action 'self'; base-uri 'self'; img-src 'self' data:;"
if self.mode != "website" or (not self.settings.get("website", "disable_csp") and not self.settings.get("website", "custom_csp")): if self.mode != "website" or (
r.headers.set( not self.settings.get("website", "disable_csp")
"Content-Security-Policy", and not self.settings.get("website", "custom_csp")
default_csp ):
) r.headers.set("Content-Security-Policy", default_csp)
else: else:
if self.settings.get("website", "custom_csp"): if self.settings.get("website", "custom_csp"):
r.headers.set( r.headers.set(
"Content-Security-Policy", "Content-Security-Policy",
self.settings.get("website", "custom_csp") self.settings.get("website", "custom_csp"),
) )
return r return r
@ -387,14 +389,13 @@ class Web:
""" """
self.common.log("Web", "cleanup") self.common.log("Web", "cleanup")
# Cleanup files # Close all of the tempfile.NamedTemporaryFile
try: for file in self.cleanup_tempfiles:
for filename in self.cleanup_filenames: file.close()
if os.path.isfile(filename):
os.remove(filename) # Clean up the tempfile.NamedTemporaryDirectory objects
elif os.path.isdir(filename): for dir in self.cleanup_tempdirs:
shutil.rmtree(filename) dir.cleanup()
except Exception:
# Don't crash if file is still in use self.cleanup_tempfiles = []
pass self.cleanup_tempdirs = []
self.cleanup_filenames = []

View file

@ -37,7 +37,7 @@ def temp_dir():
"""Creates a persistent temporary directory for the CLI tests to use""" """Creates a persistent temporary directory for the CLI tests to use"""
global test_temp_dir global test_temp_dir
if not test_temp_dir: if not test_temp_dir:
test_temp_dir = tempfile.mkdtemp() test_temp_dir = tempfile.TemporaryDirectory()
return test_temp_dir return test_temp_dir
@ -47,10 +47,9 @@ def temp_dir_1024(temp_dir):
particular size (1024 bytes). particular size (1024 bytes).
""" """
new_temp_dir = tempfile.mkdtemp(dir=temp_dir) new_temp_dir = tempfile.TemporaryDirectory(dir=temp_dir.name)
tmp_file, tmp_file_path = tempfile.mkstemp(dir=new_temp_dir) tmp_file = tempfile.NamedTemporaryFile(dir=new_temp_dir.name)
with open(tmp_file, "wb") as f: tmp_file.write(b"*" * 1024)
f.write(b"*" * 1024)
return new_temp_dir return new_temp_dir
@ -61,9 +60,8 @@ def temp_dir_1024_delete(temp_dir):
the file inside) will be deleted after fixture usage. the file inside) will be deleted after fixture usage.
""" """
with tempfile.TemporaryDirectory(dir=temp_dir) as new_temp_dir: with tempfile.TemporaryDirectory(dir=temp_dir.name) as new_temp_dir:
tmp_file, tmp_file_path = tempfile.mkstemp(dir=new_temp_dir) with open(os.path.join(new_temp_dir, "file"), "wb") as f:
with open(tmp_file, "wb") as f:
f.write(b"*" * 1024) f.write(b"*" * 1024)
yield new_temp_dir yield new_temp_dir
@ -72,9 +70,10 @@ def temp_dir_1024_delete(temp_dir):
def temp_file_1024(temp_dir): def temp_file_1024(temp_dir):
"""Create a temporary file of a particular size (1024 bytes).""" """Create a temporary file of a particular size (1024 bytes)."""
with tempfile.NamedTemporaryFile(delete=False, dir=temp_dir) as tmp_file: filename = os.path.join(temp_dir.name, "file")
tmp_file.write(b"*" * 1024) with open(filename, "wb") as f:
return tmp_file.name f.write(b"*" * 1024)
return filename
@pytest.fixture @pytest.fixture
@ -84,11 +83,11 @@ def temp_file_1024_delete(temp_dir):
The temporary file will be deleted after fixture usage. The temporary file will be deleted after fixture usage.
""" """
with tempfile.NamedTemporaryFile(dir=temp_dir, delete=False) as tmp_file: with tempfile.NamedTemporaryFile(dir=temp_dir.name, delete=False) as tmp_file:
tmp_file.write(b"*" * 1024) tmp_file.write(b"*" * 1024)
tmp_file.flush() tmp_file.flush()
tmp_file.close() tmp_file.close()
yield tmp_file.name yield tmp_file
@pytest.fixture(scope="session") @pytest.fixture(scope="session")

View file

@ -54,7 +54,7 @@ class TestSettings:
"socks_port": 9999, "socks_port": 9999,
"use_stealth": True, "use_stealth": True,
} }
tmp_file, tmp_file_path = tempfile.mkstemp(dir=temp_dir) tmp_file, tmp_file_path = tempfile.mkstemp(dir=temp_dir.name)
with open(tmp_file, "w") as f: with open(tmp_file, "w") as f:
json.dump(custom_settings, f) json.dump(custom_settings, f)
settings_obj.filename = tmp_file_path settings_obj.filename = tmp_file_path
@ -69,7 +69,7 @@ class TestSettings:
def test_save(self, monkeypatch, temp_dir, settings_obj): def test_save(self, monkeypatch, temp_dir, settings_obj):
settings_filename = "default_settings.json" settings_filename = "default_settings.json"
new_temp_dir = tempfile.mkdtemp(dir=temp_dir) new_temp_dir = tempfile.mkdtemp(dir=temp_dir.name)
settings_path = os.path.join(new_temp_dir, settings_filename) settings_path = os.path.join(new_temp_dir, settings_filename)
settings_obj.filename = settings_path settings_obj.filename = settings_path
settings_obj.save() settings_obj.save()

View file

@ -50,7 +50,8 @@ def web_obj(temp_dir, common_obj, mode, num_files=0):
web = Web(common_obj, False, mode_settings, mode) web = Web(common_obj, False, mode_settings, mode)
web.running = True web.running = True
web.cleanup_filenames == [] web.cleanup_tempfiles == []
web.cleanup_tempdirs == []
web.app.testing = True web.app.testing = True
# Share mode # Share mode
@ -58,7 +59,9 @@ def web_obj(temp_dir, common_obj, mode, num_files=0):
# Add files # Add files
files = [] files = []
for _ in range(num_files): for _ in range(num_files):
with tempfile.NamedTemporaryFile(delete=False, dir=temp_dir) as tmp_file: with tempfile.NamedTemporaryFile(
delete=False, dir=temp_dir.name
) as tmp_file:
tmp_file.write(b"*" * 1024) tmp_file.write(b"*" * 1024)
files.append(tmp_file.name) files.append(tmp_file.name)
web.share_mode.set_file_info(files) web.share_mode.set_file_info(files)
@ -131,7 +134,9 @@ class TestWeb:
with web.app.test_client() as c: with web.app.test_client() as c:
# Load / with valid auth # Load / with valid auth
res = c.get("/",) res = c.get(
"/",
)
res.get_data() res.get_data()
assert res.status_code == 200 assert res.status_code == 200
@ -169,7 +174,7 @@ class TestWeb:
def test_receive_mode_message_no_files(self, temp_dir, common_obj): def test_receive_mode_message_no_files(self, temp_dir, common_obj):
web = web_obj(temp_dir, common_obj, "receive") web = web_obj(temp_dir, common_obj, "receive")
data_dir = os.path.join(temp_dir, "OnionShare") data_dir = os.path.join(temp_dir.name, "OnionShare")
os.makedirs(data_dir, exist_ok=True) os.makedirs(data_dir, exist_ok=True)
web.settings.set("receive", "data_dir", data_dir) web.settings.set("receive", "data_dir", data_dir)
@ -200,7 +205,7 @@ class TestWeb:
def test_receive_mode_message_and_files(self, temp_dir, common_obj): def test_receive_mode_message_and_files(self, temp_dir, common_obj):
web = web_obj(temp_dir, common_obj, "receive") web = web_obj(temp_dir, common_obj, "receive")
data_dir = os.path.join(temp_dir, "OnionShare") data_dir = os.path.join(temp_dir.name, "OnionShare")
os.makedirs(data_dir, exist_ok=True) os.makedirs(data_dir, exist_ok=True)
web.settings.set("receive", "data_dir", data_dir) web.settings.set("receive", "data_dir", data_dir)
@ -235,7 +240,7 @@ class TestWeb:
def test_receive_mode_files_no_message(self, temp_dir, common_obj): def test_receive_mode_files_no_message(self, temp_dir, common_obj):
web = web_obj(temp_dir, common_obj, "receive") web = web_obj(temp_dir, common_obj, "receive")
data_dir = os.path.join(temp_dir, "OnionShare") data_dir = os.path.join(temp_dir.name, "OnionShare")
os.makedirs(data_dir, exist_ok=True) os.makedirs(data_dir, exist_ok=True)
web.settings.set("receive", "data_dir", data_dir) web.settings.set("receive", "data_dir", data_dir)
@ -267,7 +272,7 @@ class TestWeb:
def test_receive_mode_no_message_no_files(self, temp_dir, common_obj): def test_receive_mode_no_message_no_files(self, temp_dir, common_obj):
web = web_obj(temp_dir, common_obj, "receive") web = web_obj(temp_dir, common_obj, "receive")
data_dir = os.path.join(temp_dir, "OnionShare") data_dir = os.path.join(temp_dir.name, "OnionShare")
os.makedirs(data_dir, exist_ok=True) os.makedirs(data_dir, exist_ok=True)
web.settings.set("receive", "data_dir", data_dir) web.settings.set("receive", "data_dir", data_dir)
@ -300,15 +305,21 @@ class TestWeb:
res.get_data() res.get_data()
assert res.status_code == 200 assert res.status_code == 200
def test_cleanup(self, common_obj, temp_dir_1024, temp_file_1024): def test_cleanup(self, common_obj, temp_dir_1024):
web = web_obj(temp_dir_1024, common_obj, "share", 3) web = web_obj(temp_dir_1024, common_obj, "share", 3)
web.cleanup_filenames = [temp_dir_1024, temp_file_1024] temp_file = tempfile.NamedTemporaryFile()
temp_dir = tempfile.TemporaryDirectory()
web.cleanup_tempfiles = [temp_file]
web.cleanup_tempdirs = [temp_dir]
web.cleanup() web.cleanup()
assert os.path.exists(temp_file_1024) is False assert os.path.exists(temp_file.name) is False
assert os.path.exists(temp_dir_1024) is False assert os.path.exists(temp_dir.name) is False
assert web.cleanup_filenames == []
assert web.cleanup_tempfiles == []
assert web.cleanup_tempdirs == []
class TestZipWriterDefault: class TestZipWriterDefault:
@ -339,8 +350,10 @@ class TestZipWriterDefault:
assert default_zw.processed_size_callback(None) is None assert default_zw.processed_size_callback(None) is None
def test_add_file(self, default_zw, temp_file_1024_delete): def test_add_file(self, default_zw, temp_file_1024_delete):
default_zw.add_file(temp_file_1024_delete) default_zw.add_file(temp_file_1024_delete.name)
zipfile_info = default_zw.z.getinfo(os.path.basename(temp_file_1024_delete)) zipfile_info = default_zw.z.getinfo(
os.path.basename(temp_file_1024_delete.name)
)
assert zipfile_info.compress_type == zipfile.ZIP_DEFLATED assert zipfile_info.compress_type == zipfile.ZIP_DEFLATED
assert zipfile_info.file_size == 1024 assert zipfile_info.file_size == 1024
@ -568,7 +581,6 @@ class TestRangeRequests:
resp = client.get(url, headers=headers) resp = client.get(url, headers=headers)
assert resp.status_code == 206 assert resp.status_code == 206
@pytest.mark.skipif(sys.platform != "linux", reason="requires Linux") @pytest.mark.skipif(sys.platform != "linux", reason="requires Linux")
@check_unsupported("curl", ["--version"]) @check_unsupported("curl", ["--version"])
def test_curl(self, temp_dir, tmpdir, common_obj): def test_curl(self, temp_dir, tmpdir, common_obj):

View file

@ -660,9 +660,6 @@ class Tab(QtWidgets.QWidget):
# Close # Close
if self.close_dialog.clickedButton() == self.close_dialog.accept_button: if self.close_dialog.clickedButton() == self.close_dialog.accept_button:
self.common.log("Tab", "close_tab", "close, closing tab")
self.get_mode().stop_server()
self.get_mode().web.cleanup()
return True return True
# Cancel # Cancel
else: else:
@ -671,8 +668,10 @@ class Tab(QtWidgets.QWidget):
def cleanup(self): def cleanup(self):
self.common.log("Tab", "cleanup", f"tab_id={self.tab_id}") self.common.log("Tab", "cleanup", f"tab_id={self.tab_id}")
if self.get_mode() and self.get_mode().web_thread: if self.get_mode():
self.get_mode().web.stop(self.get_mode().app.port) if self.get_mode().web_thread:
self.get_mode().web_thread.quit() self.get_mode().web.stop(self.get_mode().app.port)
self.get_mode().web_thread.wait() self.get_mode().web_thread.quit()
self.get_mode().web_thread.wait()
self.get_mode().web.cleanup() self.get_mode().web.cleanup()

View file

@ -316,6 +316,7 @@ class TabWidget(QtWidgets.QTabWidget):
self.common.log("TabWidget", "closing a service tab") self.common.log("TabWidget", "closing a service tab")
if tab.close_tab(): if tab.close_tab():
self.common.log("TabWidget", "user is okay with closing the tab") self.common.log("TabWidget", "user is okay with closing the tab")
tab.cleanup()
# If the tab is persistent, delete the settings file from disk # If the tab is persistent, delete the settings file from disk
if tab.settings.get("persistent", "enabled"): if tab.settings.get("persistent", "enabled"):