From 04019389dad4fc1b7311fbfe77b412e0c576c143 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Tue, 4 May 2021 16:21:42 +1000 Subject: [PATCH 1/4] Move the cleanup() function from Onionshare class to Web class, so that the list of files to be cleaned up is always available (needed for website temp files) --- cli/onionshare_cli/__init__.py | 3 +-- cli/onionshare_cli/onionshare.py | 19 ----------------- cli/onionshare_cli/web/send_base_mode.py | 3 +-- cli/onionshare_cli/web/share_mode.py | 4 ++-- cli/onionshare_cli/web/web.py | 21 +++++++++++++++++++ cli/tests/test_cli.py | 9 -------- cli/tests/test_cli_web.py | 11 ++++++++++ desktop/src/onionshare/tab/mode/__init__.py | 2 +- .../onionshare/tab/mode/share_mode/threads.py | 2 +- desktop/src/onionshare/tab/tab.py | 4 ++-- 10 files changed, 40 insertions(+), 38 deletions(-) diff --git a/cli/onionshare_cli/__init__.py b/cli/onionshare_cli/__init__.py index 6a7a0fde..c046e472 100644 --- a/cli/onionshare_cli/__init__.py +++ b/cli/onionshare_cli/__init__.py @@ -442,7 +442,6 @@ def main(cwd=None): print("Compressing files.") try: web.share_mode.set_file_info(filenames) - app.cleanup_filenames += web.share_mode.cleanup_filenames except OSError as e: print(e.strerror) sys.exit(1) @@ -536,7 +535,7 @@ def main(cwd=None): web.stop(app.port) finally: # Shutdown - app.cleanup() + web.cleanup() onion.cleanup() diff --git a/cli/onionshare_cli/onionshare.py b/cli/onionshare_cli/onionshare.py index e518d2fb..bd94100f 100644 --- a/cli/onionshare_cli/onionshare.py +++ b/cli/onionshare_cli/onionshare.py @@ -19,7 +19,6 @@ along with this program. If not, see . """ import os -import shutil from .common import AutoStopTimer @@ -89,21 +88,3 @@ class OnionShare(object): Stop the onion service """ self.onion.stop_onion_service(mode_settings) - - def cleanup(self): - """ - Shut everything down and clean up temporary files, etc. - """ - self.common.log("OnionShare", "cleanup") - - # Cleanup files - try: - for filename in self.cleanup_filenames: - if os.path.isfile(filename): - os.remove(filename) - elif os.path.isdir(filename): - shutil.rmtree(filename) - except Exception: - # Don't crash if file is still in use - pass - self.cleanup_filenames = [] diff --git a/cli/onionshare_cli/web/send_base_mode.py b/cli/onionshare_cli/web/send_base_mode.py index c7347347..77cb8ba5 100644 --- a/cli/onionshare_cli/web/send_base_mode.py +++ b/cli/onionshare_cli/web/send_base_mode.py @@ -70,7 +70,6 @@ class SendBaseModeWeb: self.root_files = ( {} ) # This is only the root files and dirs, as opposed to all of them - self.cleanup_filenames = [] self.cur_history_id = 0 self.file_info = {"files": [], "dirs": []} self.gzip_individual_files = {} @@ -177,7 +176,7 @@ class SendBaseModeWeb: self.gzip_individual_files[filesystem_path] = gzip_filename # Make sure the gzip file gets cleaned up when onionshare stops - self.cleanup_filenames.append(gzip_filename) + self.web.cleanup_filenames.append(gzip_filename) file_to_download = self.gzip_individual_files[filesystem_path] filesize = os.path.getsize(self.gzip_individual_files[filesystem_path]) diff --git a/cli/onionshare_cli/web/share_mode.py b/cli/onionshare_cli/web/share_mode.py index 4dee0cee..c5007d7f 100644 --- a/cli/onionshare_cli/web/share_mode.py +++ b/cli/onionshare_cli/web/share_mode.py @@ -497,7 +497,7 @@ class ShareModeWeb(SendBaseModeWeb): self.gzip_etag = make_etag(f) # Make sure the gzip file gets cleaned up when onionshare stops - self.cleanup_filenames.append(self.gzip_filename) + self.web.cleanup_filenames.append(self.gzip_filename) self.is_zipped = False @@ -524,7 +524,7 @@ class ShareModeWeb(SendBaseModeWeb): self.download_etag = make_etag(f) # Make sure the zip file gets cleaned up when onionshare stops - self.cleanup_filenames.append(self.zip_writer.zip_filename) + self.web.cleanup_filenames.append(self.zip_writer.zip_filename) self.is_zipped = True diff --git a/cli/onionshare_cli/web/web.py b/cli/onionshare_cli/web/web.py index da15c23b..d88a7e4e 100644 --- a/cli/onionshare_cli/web/web.py +++ b/cli/onionshare_cli/web/web.py @@ -21,6 +21,7 @@ import logging import os import queue import requests +import shutil from distutils.version import LooseVersion as Version import flask @@ -162,6 +163,8 @@ class Web: self.socketio.init_app(self.app) self.chat_mode = ChatModeWeb(self.common, self) + self.cleanup_filenames = [] + def get_mode(self): if self.mode == "share": return self.share_mode @@ -423,3 +426,21 @@ class Web: # Reset any password that was in use self.password = None + + def cleanup(self): + """ + Shut everything down and clean up temporary files, etc. + """ + self.common.log("Web", "cleanup") + + # Cleanup files + try: + for filename in self.cleanup_filenames: + if os.path.isfile(filename): + os.remove(filename) + elif os.path.isdir(filename): + shutil.rmtree(filename) + except Exception: + # Don't crash if file is still in use + pass + self.cleanup_filenames = [] diff --git a/cli/tests/test_cli.py b/cli/tests/test_cli.py index be71240b..d838f712 100644 --- a/cli/tests/test_cli.py +++ b/cli/tests/test_cli.py @@ -36,7 +36,6 @@ class TestOnionShare: def test_init(self, onionshare_obj): assert onionshare_obj.hidserv_dir is None assert onionshare_obj.onion_host is None - assert onionshare_obj.cleanup_filenames == [] assert onionshare_obj.local_only is False def test_start_onion_service(self, onionshare_obj, mode_settings_obj): @@ -48,11 +47,3 @@ class TestOnionShare: onionshare_obj.local_only = True onionshare_obj.start_onion_service("share", mode_settings_obj) assert onionshare_obj.onion_host == "127.0.0.1:{}".format(onionshare_obj.port) - - def test_cleanup(self, onionshare_obj, temp_dir_1024, temp_file_1024): - onionshare_obj.cleanup_filenames = [temp_dir_1024, temp_file_1024] - onionshare_obj.cleanup() - - assert os.path.exists(temp_dir_1024) is False - assert os.path.exists(temp_dir_1024) is False - assert onionshare_obj.cleanup_filenames == [] diff --git a/cli/tests/test_cli_web.py b/cli/tests/test_cli_web.py index ec51654c..f8c96f9c 100644 --- a/cli/tests/test_cli_web.py +++ b/cli/tests/test_cli_web.py @@ -51,6 +51,7 @@ def web_obj(temp_dir, common_obj, mode, num_files=0): web.generate_password() web.running = True + web.cleanup_filenames == [] web.app.testing = True # Share mode @@ -345,6 +346,16 @@ class TestWeb: res.get_data() assert res.status_code == 200 + def test_cleanup(self, common_obj, temp_dir_1024, temp_file_1024): + web = web_obj(temp_dir_1024, common_obj, "share", 3) + + web.cleanup_filenames = [temp_dir_1024, temp_file_1024] + web.cleanup() + + assert os.path.exists(temp_file_1024) is False + assert os.path.exists(temp_dir_1024) is False + assert web.cleanup_filenames == [] + def _make_auth_headers(self, password): auth = base64.b64encode(b"onionshare:" + password.encode()).decode() h = Headers() diff --git a/desktop/src/onionshare/tab/mode/__init__.py b/desktop/src/onionshare/tab/mode/__init__.py index 567dc123..16944af8 100644 --- a/desktop/src/onionshare/tab/mode/__init__.py +++ b/desktop/src/onionshare/tab/mode/__init__.py @@ -382,7 +382,7 @@ class Mode(QtWidgets.QWidget): except Exception: # Probably we had no port to begin with (Onion service didn't start) pass - self.app.cleanup() + self.web.cleanup() self.stop_server_custom() diff --git a/desktop/src/onionshare/tab/mode/share_mode/threads.py b/desktop/src/onionshare/tab/mode/share_mode/threads.py index 74d5099a..5be94e88 100644 --- a/desktop/src/onionshare/tab/mode/share_mode/threads.py +++ b/desktop/src/onionshare/tab/mode/share_mode/threads.py @@ -47,7 +47,7 @@ class CompressThread(QtCore.QThread): self.mode.filenames, processed_size_callback=self.set_processed_size ) self.success.emit() - self.mode.app.cleanup_filenames += ( + self.mode.web.cleanup_filenames += ( self.mode.web.share_mode.cleanup_filenames ) except OSError as e: diff --git a/desktop/src/onionshare/tab/tab.py b/desktop/src/onionshare/tab/tab.py index 3d88ded5..5d929d01 100644 --- a/desktop/src/onionshare/tab/tab.py +++ b/desktop/src/onionshare/tab/tab.py @@ -668,7 +668,7 @@ class Tab(QtWidgets.QWidget): 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.app.cleanup() + mode.web.cleanup() return True # Cancel else: @@ -681,4 +681,4 @@ class Tab(QtWidgets.QWidget): self.get_mode().web.stop(self.get_mode().app.port) self.get_mode().web_thread.quit() self.get_mode().web_thread.wait() - self.app.cleanup() + self.get_mode().web.cleanup() From d8801ff061ef38c339bc106b2b334c9e7989ea0f Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Tue, 4 May 2021 16:30:38 +1000 Subject: [PATCH 2/4] move self.get_mode().web.cleanup() inside the if self.get_mode() conditional, in desktop --- desktop/src/onionshare/tab/tab.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/desktop/src/onionshare/tab/tab.py b/desktop/src/onionshare/tab/tab.py index 5d929d01..a0263a50 100644 --- a/desktop/src/onionshare/tab/tab.py +++ b/desktop/src/onionshare/tab/tab.py @@ -681,4 +681,4 @@ class Tab(QtWidgets.QWidget): self.get_mode().web.stop(self.get_mode().app.port) self.get_mode().web_thread.quit() self.get_mode().web_thread.wait() - self.get_mode().web.cleanup() + self.get_mode().web.cleanup() From c6ccd4de579332187232f0e4bfeb33fe531f76d8 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Tue, 4 May 2021 16:42:23 +1000 Subject: [PATCH 3/4] Remove unnecessary cleanup_filenames appending in the CompressThread. Ensure we also remove the temp dir that a Zip file is made within, by setting the temp dir in ZipWriter as a variable --- cli/onionshare_cli/web/share_mode.py | 4 +++- desktop/src/onionshare/tab/mode/share_mode/threads.py | 3 --- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cli/onionshare_cli/web/share_mode.py b/cli/onionshare_cli/web/share_mode.py index c5007d7f..95aec1ba 100644 --- a/cli/onionshare_cli/web/share_mode.py +++ b/cli/onionshare_cli/web/share_mode.py @@ -525,6 +525,7 @@ class ShareModeWeb(SendBaseModeWeb): # 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 @@ -545,8 +546,9 @@ class ZipWriter(object): if zip_filename: self.zip_filename = zip_filename else: + self.zip_temp_dir = tempfile.mkdtemp() self.zip_filename = ( - f"{tempfile.mkdtemp()}/onionshare_{self.common.random_string(4, 6)}.zip" + f"{self.zip_temp_dir}/onionshare_{self.common.random_string(4, 6)}.zip" ) self.z = zipfile.ZipFile(self.zip_filename, "w", allowZip64=True) diff --git a/desktop/src/onionshare/tab/mode/share_mode/threads.py b/desktop/src/onionshare/tab/mode/share_mode/threads.py index 5be94e88..839d30ea 100644 --- a/desktop/src/onionshare/tab/mode/share_mode/threads.py +++ b/desktop/src/onionshare/tab/mode/share_mode/threads.py @@ -47,9 +47,6 @@ class CompressThread(QtCore.QThread): self.mode.filenames, processed_size_callback=self.set_processed_size ) self.success.emit() - self.mode.web.cleanup_filenames += ( - self.mode.web.share_mode.cleanup_filenames - ) except OSError as e: self.error.emit(e.strerror) From 49a1e2890be9a86ac8b041d9dfef927e674a6ee2 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Tue, 4 May 2021 16:48:42 +1000 Subject: [PATCH 4/4] Fix call to web.cleanup() when closing a tab that has a running share --- desktop/src/onionshare/tab/tab.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/desktop/src/onionshare/tab/tab.py b/desktop/src/onionshare/tab/tab.py index a0263a50..d39cf826 100644 --- a/desktop/src/onionshare/tab/tab.py +++ b/desktop/src/onionshare/tab/tab.py @@ -668,7 +668,7 @@ class Tab(QtWidgets.QWidget): if self.close_dialog.clickedButton() == self.close_dialog.accept_button: self.common.log("Tab", "close_tab", "close, closing tab") self.get_mode().stop_server() - mode.web.cleanup() + self.get_mode().web.cleanup() return True # Cancel else: