Twisted

Merge lp://staging/~spiv/twisted/lineReceiver-bug-3277 into lp://staging/twisted

  1. lineReceiver-bug-3277
  2. Merge into trunk
Proposed by Andrew Bennetts
Status: Needs review
Proposed branch: lp://staging/~spiv/twisted/lineReceiver-bug-3277
Merge into: lp://staging/twisted
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~spiv/twisted/lineReceiver-bug-3277
Reviewer Review Type Date Requested Status
Twisted-dev Pending
Review via email: mp+8220@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Andrew Bennetts (spiv) wrote :

I'm largely submitting this Launchpad merge proposal to experiment with
Launchpad, although I think the patch is good.)

To run the tests in this branch requires lp:testscenarios to be installed. I'm
not sure what to do about that, but the easy improvement in test coverage is
probably worth doing something to accomodate that.

This patch simply adds a TestCase that defines tests that both LineReceiver and
LineOnlyReceiver should meet. It turns out that actually there are some
inconsistencies in how these two classes behave for their supposedly common
features, so this patch also fixes the code so that the tests pass.

One unfixed issue I noticed is that LineOnlyReceiver does not implement the
IPushProducer interface, but LineReceiver does. I'm not sure if this is
intentional or not, and adding the feature to LineOnlyReceiver wasn't completely
trivial so I've punted on this issue and made that particular scenario skip.

Reply

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'twisted/protocols/basic.py'
2--- twisted/protocols/basic.py 2008-12-14 21:41:04 +0000
3+++ twisted/protocols/basic.py 2009-07-04 14:02:58 +0000
4@@ -133,7 +133,8 @@
5
6 def dataReceived(self, data):
7 """Translates bytes into lines, and calls lineReceived."""
8- lines = (self._buffer+data).split(self.delimiter)
9+ dataSoFar = self._buffer + data
10+ lines = dataSoFar.split(self.delimiter)
11 self._buffer = lines.pop(-1)
12 for line in lines:
13 if self.transport.disconnecting:
14@@ -143,7 +144,7 @@
15 # the one that told it to close.
16 return
17 if len(line) > self.MAX_LENGTH:
18- return self.lineLengthExceeded(line)
19+ return self.lineLengthExceeded(dataSoFar)
20 else:
21 self.lineReceived(line)
22 if len(self._buffer) > self.MAX_LENGTH:
23@@ -163,6 +164,7 @@
24 """Called when the maximum line length has been reached.
25 Override if it needs to be dealt with in some special way.
26 """
27+ self.transport.loseConnection()
28 return error.ConnectionLost('Line length exceeded')
29
30
31@@ -232,7 +234,7 @@
32 else:
33 linelength = len(line)
34 if linelength > self.MAX_LENGTH:
35- exceeded = line + self.__buffer
36+ exceeded = line + self.delimiter + self.__buffer
37 self.__buffer = ''
38 return self.lineLengthExceeded(exceeded)
39 why = self.lineReceived(line)
40@@ -291,7 +293,8 @@
41 be more than one line, or may be only the initial portion of the
42 line.
43 """
44- return self.transport.loseConnection()
45+ self.transport.loseConnection()
46+ return error.ConnectionLost('Line length exceeded')
47
48
49 class StringTooLongError(AssertionError):
50
51=== modified file 'twisted/test/test_protocols.py'
52--- twisted/test/test_protocols.py 2009-02-14 21:04:19 +0000
53+++ twisted/test/test_protocols.py 2009-07-04 14:15:06 +0000
54@@ -7,7 +7,7 @@
55
56 import struct
57
58-from twisted.trial import unittest
59+from twisted.trial import unittest, runner
60 from twisted.protocols import basic, wire, portforward
61 from twisted.internet import reactor, protocol, defer, task, error
62 from twisted.test import proto_helpers
63@@ -85,25 +85,6 @@
64 self.setLineMode(line[self.MAX_LENGTH + 1:])
65
66
67-class LineOnlyTester(basic.LineOnlyReceiver):
68- """
69- A buffering line only receiver.
70- """
71- delimiter = '\n'
72- MAX_LENGTH = 64
73-
74- def connectionMade(self):
75- """
76- Create/clean data received on connection.
77- """
78- self.received = []
79-
80- def lineReceived(self, line):
81- """
82- Save received data.
83- """
84- self.received.append(line)
85-
86 class WireTestCase(unittest.TestCase):
87 """
88 Test wire protocols.
89@@ -300,7 +281,10 @@
90
91 class LineOnlyReceiverTestCase(unittest.TestCase):
92 """
93- Test line only receiveer.
94+ Test line only receiver interface.
95+
96+ This test case is applied to both LineReceiver and LineOnlyReceiver by the
97+ test_suite function.
98 """
99 buffer = """foo
100 bleakness
101@@ -308,26 +292,103 @@
102 plastic forks
103 """
104
105+ scenarios = [
106+ ('LineReceiver', {'klass': basic.LineReceiver}),
107+ ('LineOnlyReceiver', {'klass': basic.LineOnlyReceiver})]
108+
109+ def makeLineReceiver(self):
110+ """
111+ Construct a line receiver for testing with.
112+
113+ It will be a simple subclass of whatever self.lineReceiverClass is.
114+ The subclass will log calls to lineReceived and lineLengthExceeded to a
115+ '.calls' attribute.
116+ """
117+ transport = protocol.FileWrapper(proto_helpers.StringIOWithoutClosing())
118+ baseClass = self.klass
119+ class LoggingLineReceiver(baseClass):
120+ """
121+ A line receiver subclass that records calls made to it, but
122+ otherwise behaves like its base class.
123+ """
124+ delimiter = '\n'
125+ MAX_LENGTH = 64
126+
127+ def connectionMade(self):
128+ self.calls = []
129+ return baseClass.connectionMade(self)
130+
131+ def lineReceived(self, line):
132+ self.calls.append(('lineReceived', line))
133+
134+ def lineLengthExceeded(self, line):
135+ self.calls.append(('lineLengthExceeded', line))
136+ return baseClass.lineLengthExceeded(self, line)
137+
138+ lineReceiver = LoggingLineReceiver()
139+ lineReceiver.makeConnection(transport)
140+ return lineReceiver
141+
142 def test_buffer(self):
143 """
144 Test buffering over line protocol: data received should match buffer.
145 """
146- t = proto_helpers.StringTransport()
147- a = LineOnlyTester()
148- a.makeConnection(t)
149+ lineReceiver = self.makeLineReceiver()
150 for c in self.buffer:
151- a.dataReceived(c)
152- self.assertEquals(a.received, self.buffer.split('\n')[:-1])
153+ lineReceiver.dataReceived(c)
154+ expectedLines = self.buffer.split('\n')[:-1]
155+ expectedCalls = [('lineReceived', line) for line in expectedLines]
156+ self.assertEquals(expectedCalls, lineReceiver.calls)
157
158 def test_lineTooLong(self):
159 """
160- Test sending a line too long: it should close the connection.
161+ When a line greater than MAX_LENGTH is received, lineLengthExceeded is
162+ called. The default implementation lineLengthExceeded closes the
163+ connection, and returns a ConnectionLost error.
164 """
165- t = proto_helpers.StringTransport()
166- a = LineOnlyTester()
167- a.makeConnection(t)
168- res = a.dataReceived('x'*200)
169+ lineReceiver = self.makeLineReceiver()
170+ res = lineReceiver.dataReceived('x'*200)
171+ self.failUnlessEqual(
172+ [('lineLengthExceeded', 'x'*200)], lineReceiver.calls)
173+ self.assertTrue(lineReceiver.transport.closed)
174 self.assertIsInstance(res, error.ConnectionLost)
175+ self.assertEqual(('Line length exceeded',), res.args)
176+
177+ def testLongLineWithDelimiter(self):
178+ """
179+ When MAX_LENGTH is exceeded *and* a delimiter has been received,
180+ lineLengthExceeded is called with the right bytes.
181+
182+ See http://twistedmatrix.com/trac/ticket/3277
183+ """
184+ # Set up a line receiver with a short MAX_LENGTH that logs
185+ # lineLengthExceeded events.
186+ lineReceiver = self.makeLineReceiver()
187+ lineReceiver.MAX_LENGTH = 10
188+ # Call dataReceived with two lines, the first longer than MAX_LENGTH.
189+ longLine = ('x' * 11) + '\n'
190+ nextLine = 'next line\n'
191+ lineReceiver.dataReceived(longLine + nextLine)
192+ # We expect lineLengthExceeded to be called with exactly what we just
193+ # passed dataReceived. lineReceived is not called.
194+ expectedCalls = [('lineLengthExceeded', longLine + nextLine)]
195+ self.assertEqual(expectedCalls, lineReceiver.calls)
196+
197+ def testLineReceiverAsProducer(self):
198+ """
199+ Test produce/unproduce in receiving.
200+ """
201+ lineReceiver = self.makeLineReceiver()
202+ if isinstance(lineReceiver, basic.LineOnlyReceiver):
203+ raise unittest.SkipTest(
204+ 'LineOnlyReceiver does not implement IPushProducer')
205+ lineReceiver.transport.registerProducer(lineReceiver, False)
206+ lineReceiver.dataReceived('hello world\n')
207+ lineReceiver.transport.unregisterProducer()
208+ lineReceiver.dataReceived('goodbye\n')
209+ self.assertEquals(
210+ [('lineReceived', 'hello world'), ('lineReceived', 'goodbye')],
211+ lineReceiver.calls)
212
213
214
215@@ -778,3 +839,15 @@
216 """
217 s = proto_helpers.StringTransport()
218 self.assertRaises(TypeError, s.write, u'foo')
219+
220+
221+def test_suite():
222+ import twisted.test.test_protocols as this_module
223+ from testscenarios import generate_scenarios
224+ loader = runner.TestLoader()
225+ result = unittest.TestSuite()
226+ tests = []
227+ for testClass in loader.findTestClasses(this_module):
228+ tests.append(loader.loadClass(testClass))
229+ result.addTests(generate_scenarios(tests))
230+ return result

Subscribers

People subscribed via source and target branches