Merge pull request #752 from mig5/receiver-mode-gui-hold-timeout-share-open-til-upload-finished

Hold a share open if its timer has expired but a file is still uploading.

13 files changed, 98 insertions, 111 deletions

diff --git a/onionshare/__init__.py b/onionshare/__init__.py
index 1e81333e..0d064639 100644
--- a/onionshare/__init__.py
+++ b/onionshare/__init__.py
@@ -209,6 +209,13 @@ def main(cwd=None):
                             print(strings._("close_on_timeout"))
                             web.stop(app.port)
                             break
+                    if mode == 'receive':
+                        if web.receive_mode.upload_count == 0 or not web.receive_mode.uploads_in_progress:
+                            print(strings._("close_on_timeout"))
+                            web.stop(app.port)
+                            break
+                        else:
+                            web.receive_mode.can_upload = False
             # Allow KeyboardInterrupt exception to be handled with threads
             # https://stackoverflow.com/questions/3788208/python-threading-ignores-keyboardinterrupt-exception
             time.sleep(0.2)
diff --git a/onionshare/settings.py b/onionshare/settings.py
index 42f7259c..02f644af 100644
--- a/onionshare/settings.py
+++ b/onionshare/settings.py
@@ -94,7 +94,6 @@ class Settings(object):
             'slug': '',
             'hidservauth_string': '',
             'downloads_dir': self.build_default_downloads_dir(),
-            'receive_allow_receiver_shutdown': True,
             'locale': None # this gets defined in fill_in_defaults()
         }
         self._settings = {}
diff --git a/onionshare/web/receive_mode.py b/onionshare/web/receive_mode.py
index 3149029f..6985f38a 100644
--- a/onionshare/web/receive_mode.py
+++ b/onionshare/web/receive_mode.py
@@ -17,7 +17,9 @@ class ReceiveModeWeb(object):
 
         self.web = web
 
+        self.can_upload = True
         self.upload_count = 0
+        self.uploads_in_progress = []
 
         self.define_routes()
 
@@ -30,25 +32,25 @@ class ReceiveModeWeb(object):
 
             if self.common.settings.get('public_mode'):
                 upload_action = '/upload'
-                close_action = '/close'
             else:
                 upload_action = '/{}/upload'.format(self.web.slug)
-                close_action = '/{}/close'.format(self.web.slug)
 
             r = make_response(render_template(
                 'receive.html',
-                upload_action=upload_action,
-                close_action=close_action,
-                receive_allow_receiver_shutdown=self.common.settings.get('receive_allow_receiver_shutdown')))
+                upload_action=upload_action))
             return self.web.add_security_headers(r)
 
         @self.web.app.route("/<slug_candidate>")
         def index(slug_candidate):
+            if not self.can_upload:
+                return self.web.error403()
             self.web.check_slug_candidate(slug_candidate)
             return index_logic()
 
         @self.web.app.route("/")
         def index_public():
+            if not self.can_upload:
+                return self.web.error403()
             if not self.common.settings.get('public_mode'):
                 return self.web.error404()
             return index_logic()
@@ -144,43 +146,41 @@ class ReceiveModeWeb(object):
                 for filename in filenames:
                     flash('Sent {}'.format(filename), 'info')
 
-            if self.common.settings.get('public_mode'):
-                return redirect('/')
+            if self.can_upload:
+                if self.common.settings.get('public_mode'):
+                    path = '/'
+                else:
+                    path = '/{}'.format(slug_candidate)
+
+                return redirect('{}'.format(path))
             else:
-                return redirect('/{}'.format(slug_candidate))
+                # It was the last upload and the timer ran out
+                if self.common.settings.get('public_mode'):
+                    return thankyou_logic(slug_candidate)
+                else:
+                    return thankyou_logic()
+
+        def thankyou_logic(slug_candidate=''):
+            r = make_response(render_template(
+                'thankyou.html'))
+            return self.web.add_security_headers(r)
 
         @self.web.app.route("/<slug_candidate>/upload", methods=['POST'])
         def upload(slug_candidate):
+            if not self.can_upload:
+                return self.web.error403()
             self.web.check_slug_candidate(slug_candidate)
             return upload_logic(slug_candidate)
 
         @self.web.app.route("/upload", methods=['POST'])
         def upload_public():
+            if not self.can_upload:
+                return self.web.error403()
             if not self.common.settings.get('public_mode'):
                 return self.web.error404()
             return upload_logic()
 
 
-        def close_logic(slug_candidate=''):
-            if self.common.settings.get('receive_allow_receiver_shutdown'):
-                self.web.force_shutdown()
-                r = make_response(render_template('closed.html'))
-                self.web.add_request(self.web.REQUEST_CLOSE_SERVER, request.path)
-                return self.web.add_security_headers(r)
-            else:
-                return redirect('/{}'.format(slug_candidate))
-
-        @self.web.app.route("/<slug_candidate>/close", methods=['POST'])
-        def close(slug_candidate):
-            self.web.check_slug_candidate(slug_candidate)
-            return close_logic(slug_candidate)
-
-        @self.web.app.route("/close", methods=['POST'])
-        def close_public():
-            if not self.common.settings.get('public_mode'):
-                return self.web.error404()
-            return close_logic()
-
 
 class ReceiveModeWSGIMiddleware(object):
     """
@@ -256,28 +256,34 @@ class ReceiveModeRequest(Request):
             # A dictionary that maps filenames to the bytes uploaded so far
             self.progress = {}
 
-            # Create an upload_id, attach it to the request
-            self.upload_id = self.web.receive_mode.upload_count
-            self.web.receive_mode.upload_count += 1
+            # Prevent new uploads if we've said so (timer expired)
+            if self.web.receive_mode.can_upload:
 
-            # Figure out the content length
-            try:
-                self.content_length = int(self.headers['Content-Length'])
-            except:
-                self.content_length = 0
+                # Create an upload_id, attach it to the request
+                self.upload_id = self.web.receive_mode.upload_count
 
-            print("{}: {}".format(
-                datetime.now().strftime("%b %d, %I:%M%p"),
-                strings._("receive_mode_upload_starting").format(self.web.common.human_readable_filesize(self.content_length))
-            ))
+                self.web.receive_mode.upload_count += 1
 
-            # Tell the GUI
-            self.web.add_request(self.web.REQUEST_STARTED, self.path, {
-                'id': self.upload_id,
-                'content_length': self.content_length
-            })
+               # Figure out the content length
+                try:
+                    self.content_length = int(self.headers['Content-Length'])
+                except:
+                    self.content_length = 0
+
+                print("{}: {}".format(
+                    datetime.now().strftime("%b %d, %I:%M%p"),
+                    strings._("receive_mode_upload_starting").format(self.web.common.human_readable_filesize(self.content_length))
+                ))
+
+                # Tell the GUI
+                self.web.add_request(self.web.REQUEST_STARTED, self.path, {
+                    'id': self.upload_id,
+                    'content_length': self.content_length
+                })
+
+                self.web.receive_mode.uploads_in_progress.append(self.upload_id)
 
-            self.previous_file = None
+                self.previous_file = None
 
     def _get_file_stream(self, total_content_length, content_type, filename=None, content_length=None):
         """
@@ -297,11 +303,15 @@ class ReceiveModeRequest(Request):
         Closing the request.
         """
         super(ReceiveModeRequest, self).close()
-        if self.upload_request:
+        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': self.upload_id
+                'id': upload_id
             })
+            self.web.receive_mode.uploads_in_progress.remove(upload_id)
+        except AttributeError:
+            pass
 
     def file_write_func(self, filename, length):
         """
diff --git a/onionshare/web/web.py b/onionshare/web/web.py
index a423b2e1..5ecbad27 100644
--- a/onionshare/web/web.py
+++ b/onionshare/web/web.py
@@ -142,6 +142,12 @@ class Web(object):
         r = make_response(render_template('404.html'), 404)
         return self.add_security_headers(r)
 
+    def error403(self):
+        self.add_request(Web.REQUEST_OTHER, request.path)
+
+        r = make_response(render_template('403.html'), 403)
+        return self.add_security_headers(r)
+
     def add_security_headers(self, r):
         """
         Add security headers to a request
diff --git a/onionshare_gui/mode/receive_mode/__init__.py b/onionshare_gui/mode/receive_mode/__init__.py
index d6c0c351..c53f1ea1 100644
--- a/onionshare_gui/mode/receive_mode/__init__.py
+++ b/onionshare_gui/mode/receive_mode/__init__.py
@@ -96,8 +96,16 @@ class ReceiveMode(Mode):
         """
         The shutdown timer expired, should we stop the server? Returns a bool
         """
-        # TODO: wait until the final upload is done before stoppign the server?
-        return True
+        # If there were no attempts to upload files, or all uploads are done, we can stop
+        if self.web.receive_mode.upload_count == 0 or not self.web.receive_mode.uploads_in_progress:
+            self.server_status.stop_server()
+            self.server_status_label.setText(strings._('close_on_timeout'))
+            return True
+        # An upload is probably still running - hold off on stopping the share, but block new shares.
+        else:
+            self.server_status_label.setText(strings._('timeout_upload_still_running'))
+            self.web.receive_mode.can_upload = False
+            return False
 
     def start_server_custom(self):
         """
diff --git a/onionshare_gui/settings_dialog.py b/onionshare_gui/settings_dialog.py
index edd2528b..958d49fd 100644
--- a/onionshare_gui/settings_dialog.py
+++ b/onionshare_gui/settings_dialog.py
@@ -186,15 +186,9 @@ class SettingsDialog(QtWidgets.QDialog):
         downloads_layout.addWidget(self.downloads_dir_lineedit)
         downloads_layout.addWidget(downloads_button)
 
-        # Allow the receiver to shutdown the server
-        self.receive_allow_receiver_shutdown_checkbox = QtWidgets.QCheckBox()
-        self.receive_allow_receiver_shutdown_checkbox.setCheckState(QtCore.Qt.Checked)
-        self.receive_allow_receiver_shutdown_checkbox.setText(strings._("gui_settings_receive_allow_receiver_shutdown_checkbox"))
-
         # Receiving options layout
         receiving_group_layout = QtWidgets.QVBoxLayout()
         receiving_group_layout.addLayout(downloads_layout)
-        receiving_group_layout.addWidget(self.receive_allow_receiver_shutdown_checkbox)
         receiving_group = QtWidgets.QGroupBox(strings._("gui_settings_receiving_label"))
         receiving_group.setLayout(receiving_group_layout)
 
@@ -506,12 +500,6 @@ class SettingsDialog(QtWidgets.QDialog):
         downloads_dir = self.old_settings.get('downloads_dir')
         self.downloads_dir_lineedit.setText(downloads_dir)
 
-        receive_allow_receiver_shutdown = self.old_settings.get('receive_allow_receiver_shutdown')
-        if receive_allow_receiver_shutdown:
-            self.receive_allow_receiver_shutdown_checkbox.setCheckState(QtCore.Qt.Checked)
-        else:
-            self.receive_allow_receiver_shutdown_checkbox.setCheckState(QtCore.Qt.Unchecked)
-
         public_mode = self.old_settings.get('public_mode')
         if public_mode:
             self.public_mode_checkbox.setCheckState(QtCore.Qt.Checked)
@@ -965,7 +953,6 @@ class SettingsDialog(QtWidgets.QDialog):
             settings.set('hidservauth_string', '')
 
         settings.set('downloads_dir', self.downloads_dir_lineedit.text())
-        settings.set('receive_allow_receiver_shutdown', self.receive_allow_receiver_shutdown_checkbox.isChecked())
         settings.set('public_mode', self.public_mode_checkbox.isChecked())
         settings.set('use_stealth', self.stealth_checkbox.isChecked())
         # Always unset the HidServAuth if Stealth mode is unset
diff --git a/share/locale/en.json b/share/locale/en.json
index b02e522f..7fb30df8 100644
--- a/share/locale/en.json
+++ b/share/locale/en.json
@@ -13,6 +13,7 @@
     "close_on_timeout": "Stopped because auto-stop timer ran out",
     "closing_automatically": "Stopped because download finished",
     "timeout_download_still_running": "Waiting for download to complete",
+    "timeout_upload_still_running": "Waiting for upload to complete",
     "large_filesize": "Warning: Sending a large share could take hours",
     "systray_menu_exit": "Quit",
     "systray_download_started_title": "OnionShare Download Started",
@@ -165,7 +166,6 @@
     "gui_settings_receiving_label": "Receiving settings",
     "gui_settings_downloads_label": "Save files to",
     "gui_settings_downloads_button": "Browse",
-    "gui_settings_receive_allow_receiver_shutdown_checkbox": "Receive mode can be stopped by the sender",
     "gui_settings_public_mode_checkbox": "Public mode",
     "systray_close_server_title": "OnionShare Server Closed",
     "systray_close_server_message": "A user closed the server",
diff --git a/share/templates/403.html b/share/templates/403.html
new file mode 100644
index 00000000..df81f3e7
--- /dev/null
+++ b/share/templates/403.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+  <head>
+      <title>OnionShare: 403 Forbidden</title>
+      <link href="/static/img/favicon.ico" rel="icon" type="image/x-icon" />
+      <link href="/static/css/style.css" rel="stylesheet" type="text/css" />
+  </head>
+  <body>
+      <div class="info-wrapper">
+          <div class="info">
+              <p><img class="logo" src="/static/img/logo_large.png" title="OnionShare"></p>
+              <p class="info-header">You are not allowed to perform that action at this time.</p>
+	  </div>
+      </div>
+  </body>
+</html>
diff --git a/share/templates/receive.html b/share/templates/receive.html
index d8b02f73..e85b6ff9 100644
--- a/share/templates/receive.html
+++ b/share/templates/receive.html
@@ -34,14 +34,5 @@
         </form>
       </div>
     </div>
-    {% if receive_allow_receiver_shutdown %}
-      {% with messages = get_flashed_messages() %}
-        {% if messages %}
-          <form method="post" action="{{ close_action }}">
-            <input type="submit" class="close-button" value="I'm Finished Sending" />
-          </form>
-        {% endif %}
-      {% endwith %}
-    {% endif %}
   </body>
 </html>
diff --git a/share/templates/closed.html b/share/templates/thankyou.html
index 64c8b369..64c8b369 100644
--- a/share/templates/closed.html
+++ b/share/templates/thankyou.html
diff --git a/tests/local_onionshare_settings_dialog_test.py b/tests/local_onionshare_settings_dialog_test.py
index 6d8923b6..c1e48122 100644
--- a/tests/local_onionshare_settings_dialog_test.py
+++ b/tests/local_onionshare_settings_dialog_test.py
@@ -85,11 +85,6 @@ class SettingsGuiTest(unittest.TestCase, SettingsGuiBaseTest):
 
         # receive mode
         self.gui.downloads_dir_lineedit.setText('/tmp/OnionShareSettingsTest')
-        # allow receiver shutdown is on
-        self.assertTrue(self.gui.receive_allow_receiver_shutdown_checkbox.isChecked())
-        # disable receiver shutdown
-        QtTest.QTest.mouseClick(self.gui.receive_allow_receiver_shutdown_checkbox, QtCore.Qt.LeftButton, pos=QtCore.QPoint(2,self.gui.receive_allow_receiver_shutdown_checkbox.height()/2))
-        self.assertFalse(self.gui.receive_allow_receiver_shutdown_checkbox.isChecked())
 
 
         # bundled mode is enabled
@@ -168,7 +163,6 @@ class SettingsGuiTest(unittest.TestCase, SettingsGuiBaseTest):
         self.assertTrue(data["save_private_key"])
         self.assertTrue(data["use_stealth"])
         self.assertEqual(data["downloads_dir"], "/tmp/OnionShareSettingsTest")
-        self.assertFalse(data["receive_allow_receiver_shutdown"])
         self.assertFalse(data["close_after_first_download"])
         self.assertEqual(data["connection_type"], "bundled")
         self.assertFalse(data["tor_bridges_use_obfs4"])
diff --git a/tests/test_onionshare_settings.py b/tests/test_onionshare_settings.py
index 371b2d27..bb619c4d 100644
--- a/tests/test_onionshare_settings.py
+++ b/tests/test_onionshare_settings.py
@@ -65,7 +65,6 @@ class TestSettings:
             'slug': '',
             'hidservauth_string': '',
             'downloads_dir': os.path.expanduser('~/OnionShare'),
-            'receive_allow_receiver_shutdown': True,
             'public_mode': False
         }
         for key in settings_obj._settings:
diff --git a/tests/test_onionshare_web.py b/tests/test_onionshare_web.py
index d42adde4..3f9540ae 100644
--- a/tests/test_onionshare_web.py
+++ b/tests/test_onionshare_web.py
@@ -139,36 +139,6 @@ class TestWeb:
             res.get_data()
             assert res.status_code == 200
 
-    def test_receive_mode_allow_receiver_shutdown_on(self, common_obj):
-        web = web_obj(common_obj, 'receive')
-
-        common_obj.settings.set('receive_allow_receiver_shutdown', True)
-
-        assert web.running == True
-
-        with web.app.test_client() as c:
-            # Load close page
-            res = c.post('/{}/close'.format(web.slug))
-            res.get_data()
-            # Should return ok, and server should stop
-            assert res.status_code == 200
-            assert web.running == False
-
-    def test_receive_mode_allow_receiver_shutdown_off(self, common_obj):
-        web = web_obj(common_obj, 'receive')
-
-        common_obj.settings.set('receive_allow_receiver_shutdown', False)
-
-        assert web.running == True
-
-        with web.app.test_client() as c:
-            # Load close page
-            res = c.post('/{}/close'.format(web.slug))
-            res.get_data()
-            # Should redirect to index, and server should still be running
-            assert res.status_code == 302
-            assert web.running == True
-
     def test_public_mode_on(self, common_obj):
         web = web_obj(common_obj, 'receive')
         common_obj.settings.set('public_mode', True)