Merge lp:~alecu/ubuntuone-client/proxy-tunnel-client into lp:ubuntuone-client

[Alejandro J. Cura]

- A client for the proxy tunnel, to be used by syncdaemon (LP: #929207).

- A client for the proxy tunnel, to be used by syncdaemon (LP: #929207)

[lp:~alecu/ubuntuone-client/proxy-tunnel-client]

1208. By Alejandro J. Cura on 2012-02-29

merged with source branch

1209. By Alejandro J. Cura on 2012-03-02

Merged proxy-tunnel-server into proxy-tunnel-client.

1210. By Alejandro J. Cura on 2012-03-06

Merged proxy-tunnel-server into proxy-tunnel-client.

1211. By Alejandro J. Cura on 2012-03-07

Merged proxy-tunnel-server into proxy-tunnel-client.

[Manuel de la Peña (mandel)]

In TunnelClientProtocolTestCase I see that we do a number of times the following:

75 + other_proto = protocol.Protocol()
76 + other_factory = protocol.ClientFactory()
77 + other_factory.buildProtocol = lambda _addr: other_proto
78 + tunnel_client_factory = tunnel_client.TunnelClientFactory(host, port,
79 + other_factory)
80 + tunnel_client_proto = tunnel_client_factory.buildProtocol(fake_addr)
81 + tunnel_client_proto.transport = FakeTransport()
82 + tunnel_client_proto.connectionMade()

Would't it be a good idea to move that to the setUp?

In TunnelClientTestCase you refactor the tests as follows:

             @defer.inlineCallbacks
             def _assert_connection(self, is_ssl=False):
                 """Asseert the connection."""
                 tunnel_client = TunnelClient("0.0.0.0", self.tunnel_server.port)
                 factory = client.HTTPClientFactory(self.dest_url)
                 scheme, host, port, path = client._parse(self.dest_url)
                 if is_ssl:
                     context_factory = ssl.ClientContextFactory()
                     tunnel_client.connectSSL(host, port, factory, context_factory)
                 else:
                     tunnel_client.connectTCP(host, port, factory)
                 result = yield factory.deferred
                 self.assertEqual(result, SAMPLE_CONTENT)

            def test_connects_right(self):
                """Uses the CONNECT method on the tunnel."""
                self._assert_connection()

            def test_starts_tls_connection(self):
                """TLS is started after connecting; control passed to the client."""
                self._assert_connection(is_ssl=True)

Although if you think it does not bring anything to the code you can leave the tests as they are.

Since all the above comments are just about tests and writing less code I'll approve and will trust you to apply or not the comments if you feel they are needed :)

review: Approve

[Natalia Bidart (nataliabidart)]

* Can you please use the same encoding definition header from old files in the new file? Also, can you add the encoding header to tests/proxy/test_tunnel_client.py?

The rest looks good!

review: Approve

[lp:~alecu/ubuntuone-client/proxy-tunnel-client]

1212. By Alejandro J. Cura on 2012-03-09

fixes requested on the reviews

[Alejandro J. Cura (alecu)]

> In TunnelClientProtocolTestCase I see that we do a number of times the
> following:
>
> 75 + other_proto = protocol.Protocol()
> 76 + other_factory = protocol.ClientFactory()
> 77 + other_factory.buildProtocol = lambda _addr: other_proto
> 78 + tunnel_client_factory =
> tunnel_client.TunnelClientFactory(host, port,
> 79 +
> other_factory)
> 80 + tunnel_client_proto =
> tunnel_client_factory.buildProtocol(fake_addr)
> 81 + tunnel_client_proto.transport = FakeTransport()
> 82 + tunnel_client_proto.connectionMade()
>
> Would't it be a good idea to move that to the setUp?

Good idea, I've moved it to the setUp.

> In TunnelClientTestCase you refactor the tests as follows:
>
> @defer.inlineCallbacks
> def _assert_connection(self, is_ssl=False):
> """Asseert the connection."""
> tunnel_client = TunnelClient("0.0.0.0",
> self.tunnel_server.port)
> factory = client.HTTPClientFactory(self.dest_url)
> scheme, host, port, path = client._parse(self.dest_url)
> if is_ssl:
> context_factory = ssl.ClientContextFactory()
> tunnel_client.connectSSL(host, port, factory,
> context_factory)
> else:
> tunnel_client.connectTCP(host, port, factory)
> result = yield factory.deferred
> self.assertEqual(result, SAMPLE_CONTENT)
>
> def test_connects_right(self):
> """Uses the CONNECT method on the tunnel."""
> self._assert_connection()
>
> def test_starts_tls_connection(self):
> """TLS is started after connecting; control passed to the
> client."""
> self._assert_connection(is_ssl=True)
>
> Although if you think it does not bring anything to the code you can leave the
> tests as they are.

This case is a bit more delicate, because not only the connectXXX method is different and needs to have an if like you've put, but also the url is different too. I think it's not worth it to refactor the test on this particular test.

thanks the review!

[Alejandro J. Cura (alecu)]

> * Can you please use the same encoding definition header from old files in the
> new file? Also, can you add the encoding header to
> tests/proxy/test_tunnel_client.py?

Fixed, thanks for pointing it out.

=== added file 'tests/proxy/test_tunnel_client.py'
--- tests/proxy/test_tunnel_client.py	1970-01-01 00:00:00 +0000
+++ tests/proxy/test_tunnel_client.py	2012-03-09 14:41:44 +0000
@@ -0,0 +1,144 @@
+# -*- coding: utf8 -*-
+#
+# Copyright 2012 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+"""Tests for the proxy tunnel."""
+
+from twisted.internet import defer, protocol, ssl
+from twisted.trial.unittest import TestCase
+from twisted.web import client
+
+from ubuntuone.devtools.testcases.squid import SquidTestCase
+
+from tests.proxy import (
+    FakeTransport,
+    MockWebServer,
+    SAMPLE_CONTENT,
+    SIMPLERESOURCE,
+)
+from ubuntuone.proxy import tunnel_client
+from ubuntuone.proxy.tunnel_client import CRLF, TunnelClient
+from ubuntuone.proxy.tunnel_server import TunnelServer
+
+
+FAKE_HEADER = (
+    "HTTP/1.0 200 Connected!" + CRLF +
+    "Header1: value1" + CRLF +
+    "Header2: value2" + CRLF +
+    CRLF
+)
+
+
+class SavingProtocol(protocol.Protocol):
+    """A protocol that saves all that it receives."""
+
+    def __init__(self):
+        """Initialize this protocol."""
+        self.saved_data = None
+
+    def connectionMade(self):
+        """The connection was made, start saving."""
+        self.saved_data = []
+
+    def dataReceived(self, data):
+        """Save the data received."""
+        self.saved_data.append(data)
+
+    @property
+    def content(self):
+        """All the content so far."""
+        return "".join(self.saved_data)
+
+
+class TunnelClientProtocolTestCase(TestCase):
+    """Tests for the client side tunnel protocol."""
+
+    timeout = 3
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Initialize this testcase."""
+        yield super(TunnelClientProtocolTestCase, self).setUp()
+        self.host, self.port = "9.9.9.9", 8765
+        fake_addr = object()
+        self.other_proto = SavingProtocol()
+        other_factory = protocol.ClientFactory()
+        other_factory.buildProtocol = lambda _addr: self.other_proto
+        tunnel_client_factory = tunnel_client.TunnelClientFactory(self.host,
+                                                      self.port, other_factory)
+        tunnel_client_proto = tunnel_client_factory.buildProtocol(fake_addr)
+        tunnel_client_proto.transport = FakeTransport()
+        tunnel_client_proto.connectionMade()
+        self.tunnel_client_proto = tunnel_client_proto
+
+    def test_sends_connect_request(self):
+        """Sends the expected CONNECT request."""
+        expected = tunnel_client.METHOD_LINE.format(self.host, self.port)
+        written = self.tunnel_client_proto.transport.getvalue()
+        first_line = written.split(CRLF)[0]
+        self.assertEqual(first_line + CRLF, expected)
+        self.assertTrue(written.endswith(CRLF * 2),
+                        "Ends with a double CRLF")
+
+    def test_handles_successful_connection(self):
+        """A successful connection is handled."""
+        self.tunnel_client_proto.dataReceived(FAKE_HEADER)
+        self.assertEqual(self.tunnel_client_proto.status_code, "200")
+
+    def test_protocol_is_switched(self):
+        """The protocol is switched after the headers are received."""
+        expected = (SAMPLE_CONTENT + CRLF) * 2
+        self.tunnel_client_proto.dataReceived(FAKE_HEADER + SAMPLE_CONTENT)
+        self.other_proto.dataReceived(CRLF + SAMPLE_CONTENT + CRLF)
+        self.assertEqual(self.other_proto.content, expected)
+
+
+class TunnelClientTestCase(SquidTestCase):
+    """Test the client for the tunnel."""
+
+    timeout = 3
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        """Initialize this testcase."""
+        yield super(TunnelClientTestCase, self).setUp()
+        self.ws = MockWebServer()
+        self.addCleanup(self.ws.stop)
+        self.dest_url = self.ws.get_iri().encode("utf-8") + SIMPLERESOURCE
+        self.dest_ssl_url = (self.ws.get_ssl_iri().encode("utf-8") +
+                             SIMPLERESOURCE)
+        self.tunnel_server = TunnelServer()
+        self.addCleanup(self.tunnel_server.shutdown)
+
+    @defer.inlineCallbacks
+    def test_connects_right(self):
+        """Uses the CONNECT method on the tunnel."""
+        tunnel_client = TunnelClient("0.0.0.0", self.tunnel_server.port)
+        factory = client.HTTPClientFactory(self.dest_url)
+        scheme, host, port, path = client._parse(self.dest_url)
+        tunnel_client.connectTCP(host, port, factory)
+        result = yield factory.deferred
+        self.assertEqual(result, SAMPLE_CONTENT)
+
+    @defer.inlineCallbacks
+    def test_starts_tls_connection(self):
+        """TLS is started after connecting; control passed to the client."""
+        tunnel_client = TunnelClient("0.0.0.0",
+                                             self.tunnel_server.port)
+        factory = client.HTTPClientFactory(self.dest_ssl_url)
+        scheme, host, port, path = client._parse(self.dest_ssl_url)
+        context_factory = ssl.ClientContextFactory()
+        tunnel_client.connectSSL(host, port, factory, context_factory)
+        result = yield factory.deferred
+        self.assertEqual(result, SAMPLE_CONTENT)
 
=== modified file 'tests/proxy/test_tunnel_server.py'
--- tests/proxy/test_tunnel_server.py	2012-03-01 23:13:50 +0000
+++ tests/proxy/test_tunnel_server.py	2012-03-09 14:41:44 +0000
@@ -1,3 +1,5 @@
+# -*- coding: utf8 -*-
+#
 # Copyright 2012 Canonical Ltd.
 #
 # This program is free software: you can redistribute it and/or modify it
 
=== added file 'ubuntuone/proxy/tunnel_client.py'
--- ubuntuone/proxy/tunnel_client.py	1970-01-01 00:00:00 +0000
+++ ubuntuone/proxy/tunnel_client.py	2012-03-09 14:41:44 +0000
@@ -0,0 +1,101 @@
+# -*- coding: utf8 -*-
+#
+# Copyright 2012 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program.  If not, see <http://www.gnu.org/licenses/>.
+"""Client for the tunnel protocol."""
+
+from twisted.internet import protocol
+
+from ubuntuone.proxy.common import BaseTunnelProtocol, CRLF
+
+METHOD_LINE = "CONNECT {0}:{1} HTTP/1.0" + CRLF
+
+
+class TunnelClientProtocol(BaseTunnelProtocol):
+    """Client protocol for the handshake part of the tunnel."""
+
+    def connectionMade(self):
+        """The connection to the tunnel was made so send request."""
+        method_line = METHOD_LINE.format(self.factory.tunnel_host,
+                                         self.factory.tunnel_port)
+        headers = {
+            "User-Agent": "Ubuntu One tunnel client"
+        }
+        self.transport.write(method_line +
+                             self.format_headers(headers) +
+                             CRLF)
+
+    def handle_first_line(self, line):
+        """The first line received is the status line."""
+        try:
+            proto_version, self.status_code, description = line.split(" ", 2)
+        except ValueError:
+            self.transport.loseConnection()
+
+    def headers_done(self):
+        """All the headers have arrived. Time to switch protocols."""
+        remaining_data = self.clearLineBuffer()
+        if self.status_code != "200":
+            self.transport.loseConnection()
+            return
+        addr = self.transport.getPeer()
+        other_protocol = self.factory.other_factory.buildProtocol(addr)
+        self.transport.protocol = other_protocol
+        other_protocol.transport = self.transport
+        self.transport = None
+        if self.factory.context_factory:
+            other_protocol.transport.startTLS(self.factory.context_factory)
+        other_protocol.connectionMade()
+        if remaining_data:
+            other_protocol.dataReceived(remaining_data)
+
+
+class TunnelClientFactory(protocol.ClientFactory):
+    """A factory for Tunnel Client Protocols."""
+
+    protocol = TunnelClientProtocol
+
+    def __init__(self, tunnel_host, tunnel_port, other_factory,
+                 context_factory=None):
+        """Initialize this factory."""
+        self.tunnel_host = tunnel_host
+        self.tunnel_port = tunnel_port
+        self.other_factory = other_factory
+        self.context_factory = context_factory
+
+
+class TunnelClient(object):
+    """A client for the proxy tunnel."""
+
+    def __init__(self, tunnel_host, tunnel_port):
+        """Initialize this client."""
+        self.tunnel_host = tunnel_host
+        self.tunnel_port = tunnel_port
+        # TODO: use tcp activation to start a tunnel process if not running
+
+    def connectTCP(self, host, port, other_factory, *args, **kwargs):
+        """A connectTCP going thru the tunnel."""
+        factory = TunnelClientFactory(host, port, other_factory)
+        from twisted.internet import reactor
+        return reactor.connectTCP(self.tunnel_host, self.tunnel_port,
+                                  factory, *args, **kwargs)
+
+    def connectSSL(self, host, port, other_factory,
+                   context_factory, *args, **kwargs):
+        """A connectSSL going thru the tunnel."""
+        factory = TunnelClientFactory(host, port, other_factory,
+                                      context_factory)
+        from twisted.internet import reactor
+        return reactor.connectTCP(self.tunnel_host, self.tunnel_port,
+                                  factory, *args, **kwargs)