- Refactor the Web.ShareMode client_cancel variable to be Web.stop_q, a thread-safe queue that communicates to both share and receive mode when the user stops the server. In share mode this still stops sending the file. In receive mode, if there's a transfer in progress, it cancels it in the middle, and doesn't end up saving that file

- In receive mode, make the receive mode dir right before saving a file (so if it doesn't complete, don't make an empty dir)
- Minor UX tweak: resizing the window stretches the History widget first

8 files changed, 102 insertions, 39 deletions

diff --git a/onionshare/web/receive_mode.py b/onionshare/web/receive_mode.py
index 6985f38a..e1034af6 100644
--- a/onionshare/web/receive_mode.py
+++ b/onionshare/web/receive_mode.py
@@ -60,26 +60,11 @@ class ReceiveModeWeb(object):
             """
             Upload files.
             """
-            # Make sure the receive mode dir exists
+            # Figure out what the receive mode dir should be
             now = datetime.now()
             date_dir = now.strftime("%Y-%m-%d")
             time_dir = now.strftime("%H.%M.%S")
             receive_mode_dir = os.path.join(self.common.settings.get('downloads_dir'), date_dir, time_dir)
-            valid = True
-            try:
-                os.makedirs(receive_mode_dir, 0o700)
-            except PermissionError:
-                self.web.add_request(self.web.REQUEST_ERROR_DOWNLOADS_DIR_CANNOT_CREATE, request.path, {
-                    "receive_mode_dir": receive_mode_dir
-                })
-                print(strings._('error_cannot_create_downloads_dir').format(receive_mode_dir))
-                valid = False
-            if not valid:
-                flash('Error uploading, please inform the OnionShare user', 'error')
-                if self.common.settings.get('public_mode'):
-                    return redirect('/')
-                else:
-                    return redirect('/{}'.format(slug_candidate))
 
             files = request.files.getlist('file[]')
             filenames = []
@@ -134,6 +119,23 @@ class ReceiveModeWeb(object):
                         'dir': receive_mode_dir
                     })
 
+                    # Make sure receive mode dir exists before writing file
+                    valid = True
+                    try:
+                        os.makedirs(receive_mode_dir, 0o700)
+                    except PermissionError:
+                        self.web.add_request(self.web.REQUEST_ERROR_DOWNLOADS_DIR_CANNOT_CREATE, request.path, {
+                            "receive_mode_dir": receive_mode_dir
+                        })
+                        print(strings._('error_cannot_create_downloads_dir').format(receive_mode_dir))
+                        valid = False
+                    if not valid:
+                        flash('Error uploading, please inform the OnionShare user', 'error')
+                        if self.common.settings.get('public_mode'):
+                            return redirect('/')
+                        else:
+                            return redirect('/{}'.format(slug_candidate))
+
                     self.common.log('ReceiveModeWeb', 'define_routes', '/upload, uploaded {}, saving to {}'.format(f.filename, local_path))
                     print(strings._('receive_mode_received_file').format(local_path))
                     f.save(local_path)
@@ -193,6 +195,7 @@ class ReceiveModeWSGIMiddleware(object):
 
     def __call__(self, environ, start_response):
         environ['web'] = self.web
+        environ['stop_q'] = self.web.stop_q
         return self.app(environ, start_response)
 
 
@@ -201,7 +204,8 @@ class ReceiveModeTemporaryFile(object):
     A custom TemporaryFile that tells ReceiveModeRequest every time data gets
     written to it, in order to track the progress of uploads.
     """
-    def __init__(self, filename, write_func, close_func):
+    def __init__(self, request, filename, write_func, close_func):
+        self.onionshare_request = request
         self.onionshare_filename = filename
         self.onionshare_write_func = write_func
         self.onionshare_close_func = close_func
@@ -222,6 +226,11 @@ class ReceiveModeTemporaryFile(object):
         """
         Custom write method that calls out to onionshare_write_func
         """
+        if not self.onionshare_request.stop_q.empty():
+            self.close()
+            self.onionshare_request.close()
+            return
+
         bytes_written = self.f.write(b)
         self.onionshare_write_func(self.onionshare_filename, bytes_written)
 
@@ -241,6 +250,12 @@ class ReceiveModeRequest(Request):
     def __init__(self, environ, populate_request=True, shallow=False):
         super(ReceiveModeRequest, self).__init__(environ, populate_request, shallow)
         self.web = environ['web']
+        self.stop_q = environ['stop_q']
+
+        self.web.common.log('ReceiveModeRequest', '__init__')
+
+        # Prevent running the close() method more than once
+        self.closed = False
 
         # Is this a valid upload request?
         self.upload_request = False
@@ -296,19 +311,35 @@ class ReceiveModeRequest(Request):
                 'complete': False
             }
 
-        return ReceiveModeTemporaryFile(filename, self.file_write_func, self.file_close_func)
+        return ReceiveModeTemporaryFile(self, filename, self.file_write_func, self.file_close_func)
 
     def close(self):
         """
         Closing the request.
         """
         super(ReceiveModeRequest, self).close()
+
+        # Prevent calling this method more than once per request
+        if self.closed:
+            return
+        self.closed = True
+
+        self.web.common.log('ReceiveModeRequest', 'close')
+
         try:
             upload_id = self.upload_id
-            # Inform the GUI that the upload has finished
-            self.web.add_request(self.web.REQUEST_UPLOAD_FINISHED, self.path, {
-                'id': upload_id
-            })
+
+            if not self.web.stop_q.empty():
+                # Inform the GUI that the upload has canceled
+                self.web.add_request(self.web.REQUEST_UPLOAD_CANCELED, self.path, {
+                    'id': upload_id
+                })
+            else:
+                # Inform the GUI that the upload has finished
+                self.web.add_request(self.web.REQUEST_UPLOAD_FINISHED, self.path, {
+                    'id': upload_id
+                })
+
             self.web.receive_mode.uploads_in_progress.remove(upload_id)
         except AttributeError:
             pass
diff --git a/onionshare/web/share_mode.py b/onionshare/web/share_mode.py
index a57d0a39..eb487c42 100644
--- a/onionshare/web/share_mode.py
+++ b/onionshare/web/share_mode.py
@@ -34,11 +34,6 @@ class ShareModeWeb(object):
         # one download at a time.
         self.download_in_progress = False
 
-        # If the client closes the OnionShare window while a download is in progress,
-        # it should immediately stop serving the file. The client_cancel global is
-        # used to tell the download function that the client is canceling the download.
-        self.client_cancel = False
-
         self.define_routes()
 
     def define_routes(self):
@@ -146,9 +141,6 @@ class ShareModeWeb(object):
             basename = os.path.basename(self.download_filename)
 
             def generate():
-                # The user hasn't canceled the download
-                self.client_cancel = False
-
                 # Starting a new download
                 if not self.web.stay_open:
                     self.download_in_progress = True
@@ -160,7 +152,7 @@ class ShareModeWeb(object):
                 canceled = False
                 while not self.web.done:
                     # The user has canceled the download, so stop serving the file
-                    if self.client_cancel:
+                    if not self.web.stop_q.empty():
                         self.web.add_request(self.web.REQUEST_CANCELED, path, {
                             'id': download_id
                         })
diff --git a/onionshare/web/web.py b/onionshare/web/web.py
index 0f156941..2ea5ff35 100644
--- a/onionshare/web/web.py
+++ b/onionshare/web/web.py
@@ -39,7 +39,8 @@ class Web(object):
     REQUEST_UPLOAD_FILE_RENAMED = 7
     REQUEST_UPLOAD_SET_DIR = 8
     REQUEST_UPLOAD_FINISHED = 9
-    REQUEST_ERROR_DOWNLOADS_DIR_CANNOT_CREATE = 10
+    REQUEST_UPLOAD_CANCELED = 10
+    REQUEST_ERROR_DOWNLOADS_DIR_CANNOT_CREATE = 11
 
     def __init__(self, common, is_gui, mode='share'):
         self.common = common
@@ -58,6 +59,11 @@ class Web(object):
         # Are we running in GUI mode?
         self.is_gui = is_gui
 
+        # If the user stops the server while a transfer is in progress, it should
+        # immediately stop the transfer. In order to make it thread-safe, stop_q
+        # is a queue. If anything is in it, then the user stopped the server
+        self.stop_q = queue.Queue()
+
         # Are we using receive mode?
         self.mode = mode
         if self.mode == 'receive':
@@ -225,6 +231,13 @@ class Web(object):
 
         self.stay_open = stay_open
 
+        # Make sure the stop_q is empty when starting a new server
+        while not self.stop_q.empty():
+            try:
+                self.stop_q.get(block=False)
+            except queue.Empty:
+                pass
+
         # In Whonix, listen on 0.0.0.0 instead of 127.0.0.1 (#220)
         if os.path.exists('/usr/share/anon-ws-base-files/workstation'):
             host = '0.0.0.0'
@@ -238,11 +251,10 @@ class Web(object):
         """
         Stop the flask web server by loading /shutdown.
         """
+        self.common.log('Web', 'stop', 'stopping server')
 
-        if self.mode == 'share':
-            # If the user cancels the download, let the download function know to stop
-            # serving the file
-            self.share_mode.client_cancel = True
+        # Let the mode know that the user stopped the server
+        self.stop_q.put(True)
 
         # To stop flask, load http://127.0.0.1:<port>/<shutdown_slug>/shutdown
         if self.running:
diff --git a/onionshare_gui/mode/__init__.py b/onionshare_gui/mode/__init__.py
index 5110289f..bd363915 100644
--- a/onionshare_gui/mode/__init__.py
+++ b/onionshare_gui/mode/__init__.py
@@ -335,3 +335,9 @@ class Mode(QtWidgets.QWidget):
         Handle REQUEST_UPLOAD_FINISHED event.
         """
         pass
+
+    def handle_request_upload_canceled(self, event):
+        """
+        Handle REQUEST_UPLOAD_CANCELED event.
+        """
+        pass
diff --git a/onionshare_gui/mode/history.py b/onionshare_gui/mode/history.py
index e72a3838..5f895d75 100644
--- a/onionshare_gui/mode/history.py
+++ b/onionshare_gui/mode/history.py
@@ -184,7 +184,7 @@ class UploadHistoryItemFile(QtWidgets.QWidget):
 
         # macOS
         elif self.common.platform == 'Darwin':
-            subprocess.call(['open', '-R', abs_filename]) 
+            subprocess.call(['open', '-R', abs_filename])
 
         # Windows
         elif self.common.platform == 'Windows':
@@ -295,6 +295,13 @@ class UploadHistoryItem(HistoryItem):
                 )
             self.label.setText(text)
 
+        elif data['action'] == 'canceled':
+            # Hide the progress bar
+            self.progress_bar.hide()
+
+            # Change the label
+            self.label.setText(strings._('gui_canceled'))
+
 
 class HistoryItemList(QtWidgets.QScrollArea):
     """
diff --git a/onionshare_gui/mode/receive_mode/__init__.py b/onionshare_gui/mode/receive_mode/__init__.py
index c53f1ea1..cde2cb8a 100644
--- a/onionshare_gui/mode/receive_mode/__init__.py
+++ b/onionshare_gui/mode/receive_mode/__init__.py
@@ -83,7 +83,7 @@ class ReceiveMode(Mode):
         # Wrapper layout
         self.wrapper_layout = QtWidgets.QHBoxLayout()
         self.wrapper_layout.addLayout(self.main_layout)
-        self.wrapper_layout.addWidget(self.history)
+        self.wrapper_layout.addWidget(self.history, stretch=1)
         self.setLayout(self.wrapper_layout)
 
     def get_stop_server_shutdown_timeout_text(self):
@@ -198,6 +198,18 @@ class ReceiveMode(Mode):
         self.history.update_completed()
         self.history.update_in_progress()
 
+    def handle_request_upload_canceled(self, event):
+        """
+        Handle REQUEST_UPLOAD_CANCELED event.
+        """
+        self.history.update(event["data"]["id"], {
+            'action': 'canceled'
+        })
+        self.history.completed_count += 1
+        self.history.in_progress_count -= 1
+        self.history.update_completed()
+        self.history.update_in_progress()
+
     def on_reload_settings(self):
         """
         We should be ok to re-enable the 'Start Receive Mode' button now.
diff --git a/onionshare_gui/mode/share_mode/__init__.py b/onionshare_gui/mode/share_mode/__init__.py
index 0cc00f92..d86a17e4 100644
--- a/onionshare_gui/mode/share_mode/__init__.py
+++ b/onionshare_gui/mode/share_mode/__init__.py
@@ -115,7 +115,7 @@ class ShareMode(Mode):
         # Wrapper layout
         self.wrapper_layout = QtWidgets.QHBoxLayout()
         self.wrapper_layout.addLayout(self.main_layout)
-        self.wrapper_layout.addWidget(self.history)
+        self.wrapper_layout.addWidget(self.history, stretch=1)
         self.setLayout(self.wrapper_layout)
 
         # Always start with focus on file selection
diff --git a/onionshare_gui/onionshare_gui.py b/onionshare_gui/onionshare_gui.py
index eab3261e..c4f69b2d 100644
--- a/onionshare_gui/onionshare_gui.py
+++ b/onionshare_gui/onionshare_gui.py
@@ -396,6 +396,9 @@ class OnionShareGui(QtWidgets.QMainWindow):
             elif event["type"] == Web.REQUEST_UPLOAD_FINISHED:
                 mode.handle_request_upload_finished(event)
 
+            elif event["type"] == Web.REQUEST_UPLOAD_CANCELED:
+                mode.handle_request_upload_canceled(event)
+
             if event["type"] == Web.REQUEST_ERROR_DOWNLOADS_DIR_CANNOT_CREATE:
                 Alert(self.common, strings._('error_cannot_create_downloads_dir').format(event["data"]["receive_mode_dir"]))