From b442b5d41b0bce9e010d147d63cbd336cc165fe1 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Thu, 21 Dec 2017 12:15:17 +1100 Subject: [PATCH] Ensure we don't consider the Tor connection 'lost' if we're still in the connection dialog process (wait for that thread to finish). Also various implementations of onion.is_authenticated() for a more robust check than onion.connected_to_tor, which seemed to help some corner cases --- onionshare/onion.py | 7 +++- onionshare_gui/onionshare_gui.py | 24 +++++-------- onionshare_gui/settings_dialog.py | 45 +++++++++++-------------- onionshare_gui/tor_connection_dialog.py | 14 ++++---- 4 files changed, 42 insertions(+), 48 deletions(-) diff --git a/onionshare/onion.py b/onionshare/onion.py index adfb4dbf..364bcb89 100644 --- a/onionshare/onion.py +++ b/onionshare/onion.py @@ -375,11 +375,16 @@ class Onion(object): # ephemeral stealth onion services are not supported self.supports_stealth = False + def is_authenticated(self): """ Returns True if the Tor connection is still working, or False otherwise. """ - return self.c.is_authenticated() + if self.c: + return self.c.is_authenticated() + else: + return False + def start_onion_service(self, port): """ diff --git a/onionshare_gui/onionshare_gui.py b/onionshare_gui/onionshare_gui.py index f6ebf6be..b7783738 100644 --- a/onionshare_gui/onionshare_gui.py +++ b/onionshare_gui/onionshare_gui.py @@ -142,10 +142,10 @@ class OnionShareGui(QtWidgets.QMainWindow): self.set_server_active(False) # Start the "Connecting to Tor" dialog, which calls onion.connect() - tor_con = TorConnectionDialog(self.qtapp, self.settings, self.onion) - tor_con.canceled.connect(self._tor_connection_canceled) - tor_con.open_settings.connect(self._tor_connection_open_settings) - tor_con.start() + self.tor_con = TorConnectionDialog(self.qtapp, self.settings, self.onion) + self.tor_con.canceled.connect(self._tor_connection_canceled) + self.tor_con.open_settings.connect(self._tor_connection_open_settings) + self.tor_con.start() # After connecting to Tor, check for updates self.check_for_updates() @@ -394,17 +394,11 @@ class OnionShareGui(QtWidgets.QMainWindow): """ self.update() - # Have we lost connection to Tor somehow? - try: - # Tor Browser may not even have been open when we started OnionShare, - # in which case onion.is_authenticated() throws a NoneType error - self.onion - if not self.onion.is_authenticated(): - self.timer.stop() - self.start_server_error(strings._('error_tor_protocol_error')) - self._tor_connection_canceled() - except: - pass + # Has the Tor Connection dialog finished, but we've since lost connection to Tor somehow? + if not self.onion.is_authenticated() and self.tor_con.t.isFinished(): + self.timer.stop() + self.start_server_error(strings._('error_tor_protocol_error')) + self._tor_connection_canceled() # scroll to the bottom of the dl progress bar log pane # if a new download has been added diff --git a/onionshare_gui/settings_dialog.py b/onionshare_gui/settings_dialog.py index 7ead3f1b..15d8f634 100644 --- a/onionshare_gui/settings_dialog.py +++ b/onionshare_gui/settings_dialog.py @@ -459,31 +459,26 @@ class SettingsDialog(QtWidgets.QDialog): # If Tor isn't connected, or if Tor settings have changed, Reinitialize # the Onion object reboot_onion = False - try: - self.onion - if self.onion.is_authenticated(): - def changed(s1, s2, keys): - """ - Compare the Settings objects s1 and s2 and return true if any values - have changed for the given keys. - """ - for key in keys: - if s1.get(key) != s2.get(key): - return True - return False + if self.onion.is_authenticated(): + def changed(s1, s2, keys): + """ + Compare the Settings objects s1 and s2 and return true if any values + have changed for the given keys. + """ + for key in keys: + if s1.get(key) != s2.get(key): + return True + return False - if changed(settings, self.old_settings, [ - 'connection_type', 'control_port_address', - 'control_port_port', 'socks_address', 'socks_port', - 'socket_file_path', 'auth_type', 'auth_password']): + if changed(settings, self.old_settings, [ + 'connection_type', 'control_port_address', + 'control_port_port', 'socks_address', 'socks_port', + 'socket_file_path', 'auth_type', 'auth_password']): - reboot_onion = True - - else: - # Tor isn't connected, so try connecting reboot_onion = True - except: - # We definitely aren't connected, as the onion object had no attribute is_authenticated() + + else: + # Tor isn't connected, so try connecting reboot_onion = True # Do we need to reinitialize Tor? @@ -497,7 +492,7 @@ class SettingsDialog(QtWidgets.QDialog): common.log('SettingsDialog', 'save_clicked', 'Onion done rebooting, connected to Tor: {}'.format(self.onion.connected_to_tor)) - if self.onion.connected_to_tor and not tor_con.wasCanceled(): + if self.onion.is_authenticated() and not tor_con.wasCanceled(): self.settings_saved.emit() self.close() @@ -510,7 +505,7 @@ class SettingsDialog(QtWidgets.QDialog): Cancel button clicked. """ common.log('SettingsDialog', 'cancel_clicked') - if not self.onion.connected_to_tor: + if not self.onion.is_authenticated(): Alert(strings._('gui_tor_connection_canceled', True), QtWidgets.QMessageBox.Warning) sys.exit() else: @@ -565,7 +560,7 @@ class SettingsDialog(QtWidgets.QDialog): common.log('SettingsDialog', 'closeEvent') # On close, if Tor isn't connected, then quit OnionShare altogether - if not self.onion.connected_to_tor: + if not self.onion.is_authenticated(): common.log('SettingsDialog', 'closeEvent', 'Closing while not connected to Tor') # Wait 1ms for the event loop to finish, then quit diff --git a/onionshare_gui/tor_connection_dialog.py b/onionshare_gui/tor_connection_dialog.py index fa4c7860..1785f391 100644 --- a/onionshare_gui/tor_connection_dialog.py +++ b/onionshare_gui/tor_connection_dialog.py @@ -57,12 +57,12 @@ class TorConnectionDialog(QtWidgets.QProgressDialog): def start(self): common.log('TorConnectionDialog', 'start') - t = TorConnectionThread(self, self.settings, self.onion) - t.tor_status_update.connect(self._tor_status_update) - t.connected_to_tor.connect(self._connected_to_tor) - t.canceled_connecting_to_tor.connect(self._canceled_connecting_to_tor) - t.error_connecting_to_tor.connect(self._error_connecting_to_tor) - t.start() + self.t = TorConnectionThread(self, self.settings, self.onion) + self.t.tor_status_update.connect(self._tor_status_update) + self.t.connected_to_tor.connect(self._connected_to_tor) + self.t.canceled_connecting_to_tor.connect(self._canceled_connecting_to_tor) + self.t.error_connecting_to_tor.connect(self._error_connecting_to_tor) + self.t.start() # The main thread needs to remain active, and checkign for Qt events, # until the thread is finished. Otherwise it won't be able to handle @@ -126,7 +126,7 @@ class TorConnectionThread(QtCore.QThread): # Connect to the Onion try: self.onion.connect(self.settings, False, self._tor_status_update) - if self.onion.connected_to_tor: + if self.onion.is_authenticated(): self.connected_to_tor.emit() else: self.canceled_connecting_to_tor.emit()