Skip to content
This repository has been archived by the owner on May 4, 2021. It is now read-only.

Bug28692 #316

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 37 additions & 9 deletions sbws/util/stem.py
@@ -1,3 +1,5 @@
from collections import OrderedDict

from stem.control import (Controller, Listener)
from stem import (SocketError, InvalidRequest, UnsatisfiableRequest,
OperationFailed, ControllerError, InvalidArguments,
Expand All @@ -16,6 +18,9 @@
stream_building_lock = RLock()


TORRC_OPTIONS_CAN_FAIL = OrderedDict({'ConnectionPadding': '0'})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's put all the options in the same place.
You could put this constant in globals.py after TORRC_STARTING_POINT.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.



def attach_stream_to_circuit_listener(controller, circ_id):
''' Returns a function that should be given to add_event_listener(). It
looks for newly created streams and attaches them to the given circ_id '''
Expand Down Expand Up @@ -117,6 +122,33 @@ def _init_controller_socket(socket):
return c


def set_torrc_runtime_options(cont):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please call this function on line 74 as well, to remove duplicate code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, i forgot

"""Set torrc options at runtime."""
try:
cont.set_conf('__DisablePredictedCircuits', '1')
cont.set_conf('__LeaveStreamsUnattached', '1')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you refactor these options into a dictionary constant?
And let's put all the options in the same place.
You could put the new constant in globals.py after TORRC_STARTING_POINT.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my initial idea (https://trac.torproject.org/projects/tor/ticket/28692#comment:8):

>  Even if only stem.py uses these options, i think they should be in globals.py, since we might need to change them and globals.py should be the place where to change sbws defaults.

Let's leave any refactoring until #28737.

# Only the first option that fails will be logged here.
# Just log stem's exceptions.
except (ControllerError, InvalidRequest, InvalidArguments) as e:
log.exception(e)
exit(1)


def set_torrc_options_can_fail(controller):
"""Set options that can fail, at runtime.
They can be set at launch, but since the may fail because they are not
supported in some Tor versions, it's easier to try one by one at runtime
and ignore the ones that fail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: extra newline?

"""
for k, v in TORRC_OPTIONS_CAN_FAIL.items():
try:
controller.set_conf(k, v)
except InvalidArguments as error:
log.debug('Ignoring option not supported by this Tor version. %s',
error)


def launch_tor(conf):
assert isinstance(conf, ConfigParser)
os.makedirs(conf.getpath('tor', 'datadir'), mode=0o700, exist_ok=True)
Expand Down Expand Up @@ -194,15 +226,11 @@ def launch_tor(conf):
fail_hard('Error trying to launch tor: %s', e)
# And return a controller to it
cont = _init_controller_socket(conf.getpath('tor', 'control_socket'))
# Because we build things by hand and can't set these before Tor bootstraps
try:
cont.set_conf('__DisablePredictedCircuits', '1')
cont.set_conf('__LeaveStreamsUnattached', '1')
except (ControllerError, InvalidArguments, InvalidRequest) as e:
log.exception("Error trying to launch tor: %s. "
"Maybe the tor directory is being used by other "
"sbws instance?", e)
exit(1)
# Set options that can fail at runtime
set_torrc_options_can_fail(cont)
# Set runtime options
set_torrc_runtime_options(cont)

log.info('Started and connected to Tor %s via %s', cont.get_version(),
conf.getpath('tor', 'control_socket'))
return cont
Expand Down
22 changes: 22 additions & 0 deletions tests/integration/util/test_stem.py
Expand Up @@ -4,3 +4,25 @@
def test_launch_and_okay(persistent_launch_tor):
cont = persistent_launch_tor
assert stem_utils.is_bootstrapped(cont)


def test_set_torrc_runtime_option_succesful(persistent_launch_tor):
controller = persistent_launch_tor
runtime_options = controller.get_conf_map(['__LeaveStreamsUnattached'])
assert runtime_options == {'__LeaveStreamsUnattached': ['1']}


def test_set_torrc_runtime_invalidrequest_option_fail(persistent_launch_tor):
controller = persistent_launch_tor
try:
controller.set_conf('ControlSocket', '/tmp/dummy')
except stem_utils.InvalidRequest as e:
assert "Unable to set option" in e.message


def test_set_torrc_options_can_fail_option_fail(persistent_launch_tor):
controller = persistent_launch_tor
try:
controller.set_conf('BadOption', '0')
except stem_utils.InvalidArguments as e:
assert "Unknown option" in e.message