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

Proposed by Alejandro J. Cura on 2012-02-28
Status: Merged
Approved by: Alejandro J. Cura on 2012-03-09
Approved revision: 1212
Merged at revision: 1203
Proposed branch: lp:~alecu/ubuntuone-client/proxy-tunnel-client
Merge into: lp:ubuntuone-client
Prerequisite: lp:~alecu/ubuntuone-client/proxy-tunnel-server
Diff against target: 264 lines (+247/-0)
3 files modified
tests/proxy/test_tunnel_client.py (+144/-0)
tests/proxy/test_tunnel_server.py (+2/-0)
ubuntuone/proxy/tunnel_client.py (+101/-0)
To merge this branch: bzr merge lp:~alecu/ubuntuone-client/proxy-tunnel-client
Reviewer Review Type Date Requested Status
Natalia Bidart 2012-02-28 Approve on 2012-03-09
Manuel de la Peña (community) 2012-02-28 Approve on 2012-03-09
Review via email: mp+95077@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
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) wrote :

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) wrote :

* 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
1212. By Alejandro J. Cura on 2012-03-09

fixes requested on the reviews

Alejandro J. Cura (alecu) wrote :

> 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) wrote :

> * 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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'tests/proxy/test_tunnel_client.py'
2--- tests/proxy/test_tunnel_client.py 1970-01-01 00:00:00 +0000
3+++ tests/proxy/test_tunnel_client.py 2012-03-09 14:41:44 +0000
4@@ -0,0 +1,144 @@
5+# -*- coding: utf8 -*-
6+#
7+# Copyright 2012 Canonical Ltd.
8+#
9+# This program is free software: you can redistribute it and/or modify it
10+# under the terms of the GNU General Public License version 3, as published
11+# by the Free Software Foundation.
12+#
13+# This program is distributed in the hope that it will be useful, but
14+# WITHOUT ANY WARRANTY; without even the implied warranties of
15+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
16+# PURPOSE. See the GNU General Public License for more details.
17+#
18+# You should have received a copy of the GNU General Public License along
19+# with this program. If not, see <http://www.gnu.org/licenses/>.
20+"""Tests for the proxy tunnel."""
21+
22+from twisted.internet import defer, protocol, ssl
23+from twisted.trial.unittest import TestCase
24+from twisted.web import client
25+
26+from ubuntuone.devtools.testcases.squid import SquidTestCase
27+
28+from tests.proxy import (
29+ FakeTransport,
30+ MockWebServer,
31+ SAMPLE_CONTENT,
32+ SIMPLERESOURCE,
33+)
34+from ubuntuone.proxy import tunnel_client
35+from ubuntuone.proxy.tunnel_client import CRLF, TunnelClient
36+from ubuntuone.proxy.tunnel_server import TunnelServer
37+
38+
39+FAKE_HEADER = (
40+ "HTTP/1.0 200 Connected!" + CRLF +
41+ "Header1: value1" + CRLF +
42+ "Header2: value2" + CRLF +
43+ CRLF
44+)
45+
46+
47+class SavingProtocol(protocol.Protocol):
48+ """A protocol that saves all that it receives."""
49+
50+ def __init__(self):
51+ """Initialize this protocol."""
52+ self.saved_data = None
53+
54+ def connectionMade(self):
55+ """The connection was made, start saving."""
56+ self.saved_data = []
57+
58+ def dataReceived(self, data):
59+ """Save the data received."""
60+ self.saved_data.append(data)
61+
62+ @property
63+ def content(self):
64+ """All the content so far."""
65+ return "".join(self.saved_data)
66+
67+
68+class TunnelClientProtocolTestCase(TestCase):
69+ """Tests for the client side tunnel protocol."""
70+
71+ timeout = 3
72+
73+ @defer.inlineCallbacks
74+ def setUp(self):
75+ """Initialize this testcase."""
76+ yield super(TunnelClientProtocolTestCase, self).setUp()
77+ self.host, self.port = "9.9.9.9", 8765
78+ fake_addr = object()
79+ self.other_proto = SavingProtocol()
80+ other_factory = protocol.ClientFactory()
81+ other_factory.buildProtocol = lambda _addr: self.other_proto
82+ tunnel_client_factory = tunnel_client.TunnelClientFactory(self.host,
83+ self.port, other_factory)
84+ tunnel_client_proto = tunnel_client_factory.buildProtocol(fake_addr)
85+ tunnel_client_proto.transport = FakeTransport()
86+ tunnel_client_proto.connectionMade()
87+ self.tunnel_client_proto = tunnel_client_proto
88+
89+ def test_sends_connect_request(self):
90+ """Sends the expected CONNECT request."""
91+ expected = tunnel_client.METHOD_LINE.format(self.host, self.port)
92+ written = self.tunnel_client_proto.transport.getvalue()
93+ first_line = written.split(CRLF)[0]
94+ self.assertEqual(first_line + CRLF, expected)
95+ self.assertTrue(written.endswith(CRLF * 2),
96+ "Ends with a double CRLF")
97+
98+ def test_handles_successful_connection(self):
99+ """A successful connection is handled."""
100+ self.tunnel_client_proto.dataReceived(FAKE_HEADER)
101+ self.assertEqual(self.tunnel_client_proto.status_code, "200")
102+
103+ def test_protocol_is_switched(self):
104+ """The protocol is switched after the headers are received."""
105+ expected = (SAMPLE_CONTENT + CRLF) * 2
106+ self.tunnel_client_proto.dataReceived(FAKE_HEADER + SAMPLE_CONTENT)
107+ self.other_proto.dataReceived(CRLF + SAMPLE_CONTENT + CRLF)
108+ self.assertEqual(self.other_proto.content, expected)
109+
110+
111+class TunnelClientTestCase(SquidTestCase):
112+ """Test the client for the tunnel."""
113+
114+ timeout = 3
115+
116+ @defer.inlineCallbacks
117+ def setUp(self):
118+ """Initialize this testcase."""
119+ yield super(TunnelClientTestCase, self).setUp()
120+ self.ws = MockWebServer()
121+ self.addCleanup(self.ws.stop)
122+ self.dest_url = self.ws.get_iri().encode("utf-8") + SIMPLERESOURCE
123+ self.dest_ssl_url = (self.ws.get_ssl_iri().encode("utf-8") +
124+ SIMPLERESOURCE)
125+ self.tunnel_server = TunnelServer()
126+ self.addCleanup(self.tunnel_server.shutdown)
127+
128+ @defer.inlineCallbacks
129+ def test_connects_right(self):
130+ """Uses the CONNECT method on the tunnel."""
131+ tunnel_client = TunnelClient("0.0.0.0", self.tunnel_server.port)
132+ factory = client.HTTPClientFactory(self.dest_url)
133+ scheme, host, port, path = client._parse(self.dest_url)
134+ tunnel_client.connectTCP(host, port, factory)
135+ result = yield factory.deferred
136+ self.assertEqual(result, SAMPLE_CONTENT)
137+
138+ @defer.inlineCallbacks
139+ def test_starts_tls_connection(self):
140+ """TLS is started after connecting; control passed to the client."""
141+ tunnel_client = TunnelClient("0.0.0.0",
142+ self.tunnel_server.port)
143+ factory = client.HTTPClientFactory(self.dest_ssl_url)
144+ scheme, host, port, path = client._parse(self.dest_ssl_url)
145+ context_factory = ssl.ClientContextFactory()
146+ tunnel_client.connectSSL(host, port, factory, context_factory)
147+ result = yield factory.deferred
148+ self.assertEqual(result, SAMPLE_CONTENT)
149
150=== modified file 'tests/proxy/test_tunnel_server.py'
151--- tests/proxy/test_tunnel_server.py 2012-03-01 23:13:50 +0000
152+++ tests/proxy/test_tunnel_server.py 2012-03-09 14:41:44 +0000
153@@ -1,3 +1,5 @@
154+# -*- coding: utf8 -*-
155+#
156 # Copyright 2012 Canonical Ltd.
157 #
158 # This program is free software: you can redistribute it and/or modify it
159
160=== added file 'ubuntuone/proxy/tunnel_client.py'
161--- ubuntuone/proxy/tunnel_client.py 1970-01-01 00:00:00 +0000
162+++ ubuntuone/proxy/tunnel_client.py 2012-03-09 14:41:44 +0000
163@@ -0,0 +1,101 @@
164+# -*- coding: utf8 -*-
165+#
166+# Copyright 2012 Canonical Ltd.
167+#
168+# This program is free software: you can redistribute it and/or modify it
169+# under the terms of the GNU General Public License version 3, as published
170+# by the Free Software Foundation.
171+#
172+# This program is distributed in the hope that it will be useful, but
173+# WITHOUT ANY WARRANTY; without even the implied warranties of
174+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
175+# PURPOSE. See the GNU General Public License for more details.
176+#
177+# You should have received a copy of the GNU General Public License along
178+# with this program. If not, see <http://www.gnu.org/licenses/>.
179+"""Client for the tunnel protocol."""
180+
181+from twisted.internet import protocol
182+
183+from ubuntuone.proxy.common import BaseTunnelProtocol, CRLF
184+
185+METHOD_LINE = "CONNECT {0}:{1} HTTP/1.0" + CRLF
186+
187+
188+class TunnelClientProtocol(BaseTunnelProtocol):
189+ """Client protocol for the handshake part of the tunnel."""
190+
191+ def connectionMade(self):
192+ """The connection to the tunnel was made so send request."""
193+ method_line = METHOD_LINE.format(self.factory.tunnel_host,
194+ self.factory.tunnel_port)
195+ headers = {
196+ "User-Agent": "Ubuntu One tunnel client"
197+ }
198+ self.transport.write(method_line +
199+ self.format_headers(headers) +
200+ CRLF)
201+
202+ def handle_first_line(self, line):
203+ """The first line received is the status line."""
204+ try:
205+ proto_version, self.status_code, description = line.split(" ", 2)
206+ except ValueError:
207+ self.transport.loseConnection()
208+
209+ def headers_done(self):
210+ """All the headers have arrived. Time to switch protocols."""
211+ remaining_data = self.clearLineBuffer()
212+ if self.status_code != "200":
213+ self.transport.loseConnection()
214+ return
215+ addr = self.transport.getPeer()
216+ other_protocol = self.factory.other_factory.buildProtocol(addr)
217+ self.transport.protocol = other_protocol
218+ other_protocol.transport = self.transport
219+ self.transport = None
220+ if self.factory.context_factory:
221+ other_protocol.transport.startTLS(self.factory.context_factory)
222+ other_protocol.connectionMade()
223+ if remaining_data:
224+ other_protocol.dataReceived(remaining_data)
225+
226+
227+class TunnelClientFactory(protocol.ClientFactory):
228+ """A factory for Tunnel Client Protocols."""
229+
230+ protocol = TunnelClientProtocol
231+
232+ def __init__(self, tunnel_host, tunnel_port, other_factory,
233+ context_factory=None):
234+ """Initialize this factory."""
235+ self.tunnel_host = tunnel_host
236+ self.tunnel_port = tunnel_port
237+ self.other_factory = other_factory
238+ self.context_factory = context_factory
239+
240+
241+class TunnelClient(object):
242+ """A client for the proxy tunnel."""
243+
244+ def __init__(self, tunnel_host, tunnel_port):
245+ """Initialize this client."""
246+ self.tunnel_host = tunnel_host
247+ self.tunnel_port = tunnel_port
248+ # TODO: use tcp activation to start a tunnel process if not running
249+
250+ def connectTCP(self, host, port, other_factory, *args, **kwargs):
251+ """A connectTCP going thru the tunnel."""
252+ factory = TunnelClientFactory(host, port, other_factory)
253+ from twisted.internet import reactor
254+ return reactor.connectTCP(self.tunnel_host, self.tunnel_port,
255+ factory, *args, **kwargs)
256+
257+ def connectSSL(self, host, port, other_factory,
258+ context_factory, *args, **kwargs):
259+ """A connectSSL going thru the tunnel."""
260+ factory = TunnelClientFactory(host, port, other_factory,
261+ context_factory)
262+ from twisted.internet import reactor
263+ return reactor.connectTCP(self.tunnel_host, self.tunnel_port,
264+ factory, *args, **kwargs)

Subscribers

People subscribed via source and target branches