2889 tub port listen #437

warner · 2017-08-10T19:42:31Z

tahoe.cfg: add tub.port=listen:i2p (and/or listen:tor)

This delegates the construction of the server Endpoint object to the i2p/tor
Provider, which can use the i2p/tor section of the config file to add options
which would be awkward to express as text in an endpoint descriptor string.

refs ticket:2889 (but note this merely makes room for a function to be
written that can process I2CP options, it does not actually handle such
options, so it does not close this ticket yet)

codecov-io · 2017-08-10T21:27:07Z

Codecov Report

Merging #437 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #437      +/-   ##
==========================================
- Coverage   89.85%   89.85%   -0.01%     
==========================================
  Files         144      144              
  Lines       27090    27105      +15     
  Branches     3893     3895       +2     
==========================================
+ Hits        24343    24355      +12     
- Misses       2017     2019       +2     
- Partials      730      731       +1

Impacted Files	Coverage Δ
src/allmydata/scripts/create_node.py	`98.98% <ø> (ø)`	⬆️
src/allmydata/node.py	`94.6% <100%> (+0.07%)`	⬆️
src/allmydata/util/i2p_provider.py	`95.76% <100%> (+0.22%)`	⬆️
src/allmydata/util/tor_provider.py	`98.3% <100%> (+0.03%)`	⬆️
src/allmydata/mutable/servermap.py	`93.7% <0%> (-0.49%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 856425f...a171108. Read the comment docs.

exarkun · 2017-08-15T17:45:42Z

src/allmydata/node.py

@@ -400,7 +400,13 @@ def create_main_tub(self):
            for port in tubport.split(","):
                if port in ("0", "tcp:0"):
                    raise ValueError("tub.port cannot be 0: you must choose")
-                self.tub.listenOn(port)
+                if port == "listen:i2p":
+                    port_or_endpoint = self._i2p_provider.get_listener()


get_listener could do with some docs

exarkun · 2017-08-15T17:46:11Z

src/allmydata/test/test_i2p_provider.py

@@ -294,6 +294,28 @@ def test_handler_default(self):
        self.assertIs(h, handler)
        i2p.default.assert_called_with(reactor, keyfile=None)

+class Provider_Listener(unittest.TestCase):


Funny name. ProviderListener matches the naming convention better, no?

exarkun · 2017-08-15T17:46:26Z

src/allmydata/test/test_i2p_provider.py

@@ -294,6 +294,28 @@ def test_handler_default(self):
        self.assertIs(h, handler)
        i2p.default.assert_called_with(reactor, keyfile=None)

+class Provider_Listener(unittest.TestCase):
+    def test_listener(self):


How about a docstring for this test method?

exarkun · 2017-08-15T17:47:08Z

src/allmydata/test/test_i2p_provider.py

+                                      reactor)
+            endpoint_or_description = p.get_listener()
+        self.assertEqual(endpoint_or_description,
+                         "i2p:%s:3457:api=SAM:apiEndpoint=goodport" % privkeyfile)


There should be another similar test that exercises the necessary escaping logic.

Naw, the whole point of this exercise is to avoid the need for escaping anything. In this early state, the get_listener() method doesn't return anything complicated enough to require escaping. In the later state (which I hope @str4d will write), it will return an Endpoint object instead of a (string) endpoint descriptor, so it won't need escaping either.

There is some escaping in the implementation, though. Maybe you want to remove that?

Whoops, you're absolutely right. I just pushed a patch to exercise that escaping.

When @str4d rewrites this to build an endpoint directly, he can remove that.

exarkun · 2017-08-15T17:47:50Z

src/allmydata/test/test_tor_provider.py

@@ -393,6 +393,20 @@ def test_handler_default(self):
        self.assertIs(h, handler)
        tor.default_socks.assert_called_with()

+class Provider_Listener(unittest.TestCase):


That funny name again

exarkun · 2017-08-15T17:49:56Z

src/allmydata/util/tor_provider.py

+    def get_listener(self):
+        local_port = self._get_tor_config("onion.local_port")
+        tor_port = "tcp:%s:interface=127.0.0.1" % local_port
+        return tor_port


It's pretty easy to make the endpoint object instead here:

from twisted.internet.endpoints import TCP4ServerEndpoint return TCP4ServerEndpoint(self._reactor, tor_port, interface="127.0.0.1")

warner · 2017-08-16T01:04:12Z

I think that addresses everything. I changed tor_provider to return an Endpoint (even though it's pretty simple, and a string descriptor would be sufficient) to serve as an example, and to exercise the property that it's supposed to be allowed to return either. Thanks for the suggestion!

This delegates the construction of the server Endpoint object to the i2p/tor Provider, which can use the i2p/tor section of the config file to add options which would be awkward to express as text in an endpoint descriptor string. refs ticket:2889 (but note this merely makes room for a function to be written that can process I2CP options, it does not actually handle such options, so it does not close this ticket yet)

…criptor get_listener() is allowed to return either, and the Tor provider is currently simple enough to not really need more than a basic descriptor, but have it return a full Endpoint for use as an example of what I2P can do later.

warner force-pushed the 2889-tub-port-listen branch from 0185cec to a68024c Compare August 10, 2017 20:19

exarkun reviewed Aug 15, 2017

View reviewed changes

warner force-pushed the 2889-tub-port-listen branch from 726af4f to 286c33a Compare August 17, 2017 06:19

warner force-pushed the 2889-tub-port-listen branch 2 times, most recently from 8f2b322 to d09849f Compare September 21, 2017 07:31

warner added 5 commits November 3, 2017 00:24

tor/i2p: rename create_onion/create_dest to create_config

d1fd43a

address review feedback: docstrings, better test class name

d713e0e

test_i2p_provider: exercise escaping of apiEndpoint attribute

a171108

warner force-pushed the 2889-tub-port-listen branch from d09849f to a171108 Compare November 3, 2017 07:25

warner merged commit a171108 into tahoe-lafs:master Nov 3, 2017

warner deleted the 2889-tub-port-listen branch November 3, 2017 07:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2889 tub port listen #437

2889 tub port listen #437

warner commented Aug 10, 2017

codecov-io commented Aug 10, 2017 •

edited

Loading

exarkun Aug 15, 2017

exarkun Aug 15, 2017

exarkun Aug 15, 2017

exarkun Aug 15, 2017

warner Aug 16, 2017

exarkun Aug 16, 2017

warner Aug 17, 2017

exarkun Aug 15, 2017

exarkun Aug 15, 2017

warner commented Aug 16, 2017

2889 tub port listen #437

2889 tub port listen #437

Conversation

warner commented Aug 10, 2017

codecov-io commented Aug 10, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

warner commented Aug 16, 2017

codecov-io commented Aug 10, 2017 •

edited

Loading