* [Buildroot] [PATCH 1/1] python-websockets: backport fix for upstream issue #350 @ 2018-05-07 20:57 Joseph Kogut 2018-05-07 21:26 ` Thomas Petazzoni 2018-05-07 22:18 ` [Buildroot] [PATCH v2 " Joseph Kogut 0 siblings, 2 replies; 9+ messages in thread From: Joseph Kogut @ 2018-05-07 20:57 UTC (permalink / raw) To: buildroot Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com> --- package/python-websockets/python-websockets.hash | 1 + package/python-websockets/python-websockets.mk | 1 + 2 files changed, 2 insertions(+) diff --git a/package/python-websockets/python-websockets.hash b/package/python-websockets/python-websockets.hash index 18947cc60a..4fa2fba1e1 100644 --- a/package/python-websockets/python-websockets.hash +++ b/package/python-websockets/python-websockets.hash @@ -2,3 +2,4 @@ md5 9e8c6b3c70def4146d75fbb0f52bdfc7 websockets-4.0.1.tar.gz sha256 da4d4fbe059b0453e726d6d993760065d69b823a27efc3040402a6fcfe6a1ed9 websockets-4.0.1.tar.gz sha256 2cd4d416e432ca7fda2c103b38b852f8d3cb327d70c3db44410b9fe97e6c4d73 LICENSE +sha256 c43360df2209d8af756297af7d8d1ff0a98483fc2d6551be80f309a6a6671ece 402059e4a46a764632eba8a669f5b012f173ee7b.patch diff --git a/package/python-websockets/python-websockets.mk b/package/python-websockets/python-websockets.mk index 7c5cf8c4e7..e0a701af43 100644 --- a/package/python-websockets/python-websockets.mk +++ b/package/python-websockets/python-websockets.mk @@ -10,5 +10,6 @@ PYTHON_WEBSOCKETS_SITE = https://pypi.python.org/packages/b6/12/6194aac840c65253 PYTHON_WEBSOCKETS_SETUP_TYPE = setuptools PYTHON_WEBSOCKETS_LICENSE = BSD-3-Clause PYTHON_WEBSOCKETS_LICENSE_FILES = LICENSE +PYTHON_WEBSOCKETS_PATCH += https://github.com/aaugustin/websockets/commit/402059e4a46a764632eba8a669f5b012f173ee7b.patch $(eval $(python-package)) -- 2.17.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH 1/1] python-websockets: backport fix for upstream issue #350 2018-05-07 20:57 [Buildroot] [PATCH 1/1] python-websockets: backport fix for upstream issue #350 Joseph Kogut @ 2018-05-07 21:26 ` Thomas Petazzoni 2018-05-07 22:07 ` Joseph Kogut 2018-05-07 22:18 ` [Buildroot] [PATCH v2 " Joseph Kogut 1 sibling, 1 reply; 9+ messages in thread From: Thomas Petazzoni @ 2018-05-07 21:26 UTC (permalink / raw) To: buildroot Hello, On Mon, 7 May 2018 13:57:33 -0700, Joseph Kogut wrote: > Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com> > --- > package/python-websockets/python-websockets.hash | 1 + > package/python-websockets/python-websockets.mk | 1 + > 2 files changed, 2 insertions(+) > > diff --git a/package/python-websockets/python-websockets.hash b/package/python-websockets/python-websockets.hash > index 18947cc60a..4fa2fba1e1 100644 > --- a/package/python-websockets/python-websockets.hash > +++ b/package/python-websockets/python-websockets.hash > @@ -2,3 +2,4 @@ > md5 9e8c6b3c70def4146d75fbb0f52bdfc7 websockets-4.0.1.tar.gz > sha256 da4d4fbe059b0453e726d6d993760065d69b823a27efc3040402a6fcfe6a1ed9 websockets-4.0.1.tar.gz > sha256 2cd4d416e432ca7fda2c103b38b852f8d3cb327d70c3db44410b9fe97e6c4d73 LICENSE > +sha256 c43360df2209d8af756297af7d8d1ff0a98483fc2d6551be80f309a6a6671ece 402059e4a46a764632eba8a669f5b012f173ee7b.patch > diff --git a/package/python-websockets/python-websockets.mk b/package/python-websockets/python-websockets.mk > index 7c5cf8c4e7..e0a701af43 100644 > --- a/package/python-websockets/python-websockets.mk > +++ b/package/python-websockets/python-websockets.mk > @@ -10,5 +10,6 @@ PYTHON_WEBSOCKETS_SITE = https://pypi.python.org/packages/b6/12/6194aac840c65253 > PYTHON_WEBSOCKETS_SETUP_TYPE = setuptools > PYTHON_WEBSOCKETS_LICENSE = BSD-3-Clause > PYTHON_WEBSOCKETS_LICENSE_FILES = LICENSE > +PYTHON_WEBSOCKETS_PATCH += https://github.com/aaugustin/websockets/commit/402059e4a46a764632eba8a669f5b012f173ee7b.patch Please store the patch locally in Buildroot's package folder. Downloading patches from Github doesn't work because they don't have stable contents, and therefore a stable hash. Thanks! Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH 1/1] python-websockets: backport fix for upstream issue #350 2018-05-07 21:26 ` Thomas Petazzoni @ 2018-05-07 22:07 ` Joseph Kogut 0 siblings, 0 replies; 9+ messages in thread From: Joseph Kogut @ 2018-05-07 22:07 UTC (permalink / raw) To: buildroot Hi Thomas, On Mon, May 7, 2018 at 2:26 PM, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > Hello, > > On Mon, 7 May 2018 13:57:33 -0700, Joseph Kogut wrote: >> Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com> >> --- >> package/python-websockets/python-websockets.hash | 1 + >> package/python-websockets/python-websockets.mk | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/package/python-websockets/python-websockets.hash b/package/python-websockets/python-websockets.hash >> index 18947cc60a..4fa2fba1e1 100644 >> --- a/package/python-websockets/python-websockets.hash >> +++ b/package/python-websockets/python-websockets.hash >> @@ -2,3 +2,4 @@ >> md5 9e8c6b3c70def4146d75fbb0f52bdfc7 websockets-4.0.1.tar.gz >> sha256 da4d4fbe059b0453e726d6d993760065d69b823a27efc3040402a6fcfe6a1ed9 websockets-4.0.1.tar.gz >> sha256 2cd4d416e432ca7fda2c103b38b852f8d3cb327d70c3db44410b9fe97e6c4d73 LICENSE >> +sha256 c43360df2209d8af756297af7d8d1ff0a98483fc2d6551be80f309a6a6671ece 402059e4a46a764632eba8a669f5b012f173ee7b.patch >> diff --git a/package/python-websockets/python-websockets.mk b/package/python-websockets/python-websockets.mk >> index 7c5cf8c4e7..e0a701af43 100644 >> --- a/package/python-websockets/python-websockets.mk >> +++ b/package/python-websockets/python-websockets.mk >> @@ -10,5 +10,6 @@ PYTHON_WEBSOCKETS_SITE = https://pypi.python.org/packages/b6/12/6194aac840c65253 >> PYTHON_WEBSOCKETS_SETUP_TYPE = setuptools >> PYTHON_WEBSOCKETS_LICENSE = BSD-3-Clause >> PYTHON_WEBSOCKETS_LICENSE_FILES = LICENSE >> +PYTHON_WEBSOCKETS_PATCH += https://github.com/aaugustin/websockets/commit/402059e4a46a764632eba8a669f5b012f173ee7b.patch > > Please store the patch locally in Buildroot's package folder. > Downloading patches from Github doesn't work because they don't have > stable contents, and therefore a stable hash. Thanks for the heads up, I'll submit a revised patch. > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) > Embedded Linux and Kernel engineering > https://bootlin.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH v2 1/1] python-websockets: backport fix for upstream issue #350 2018-05-07 20:57 [Buildroot] [PATCH 1/1] python-websockets: backport fix for upstream issue #350 Joseph Kogut 2018-05-07 21:26 ` Thomas Petazzoni @ 2018-05-07 22:18 ` Joseph Kogut 2018-05-08 3:48 ` Baruch Siach 2018-05-10 22:17 ` [Buildroot] [PATCH v3 " Joseph Kogut 1 sibling, 2 replies; 9+ messages in thread From: Joseph Kogut @ 2018-05-07 22:18 UTC (permalink / raw) To: buildroot Fetch from: https://github.com/aaugustin/websockets/commit/402059e4a46a764632eba8a669f5b012f173ee7b.patch Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com> --- Changes v1 -> v2: - Include patch locally (suggested by Thomas) ...59e4a46a764632eba8a669f5b012f173ee7b.patch | 256 ++++++++++++++++++ 1 file changed, 256 insertions(+) create mode 100644 package/python-websockets/402059e4a46a764632eba8a669f5b012f173ee7b.patch diff --git a/package/python-websockets/402059e4a46a764632eba8a669f5b012f173ee7b.patch b/package/python-websockets/402059e4a46a764632eba8a669f5b012f173ee7b.patch new file mode 100644 index 0000000000..62e93bdd2e --- /dev/null +++ b/package/python-websockets/402059e4a46a764632eba8a669f5b012f173ee7b.patch @@ -0,0 +1,256 @@ +From 402059e4a46a764632eba8a669f5b012f173ee7b Mon Sep 17 00:00:00 2001 +From: Aymeric Augustin <aymeric.augustin@m4x.org> +Date: Tue, 1 May 2018 17:05:05 +0200 +Subject: [PATCH] Fix behavior of recv() in the CLOSING state. + +The behavior wasn't tested correctly: in some test cases, the connection +had already moved to the CLOSED state, where the close code and reason +are already known. + +Refactor half_close_connection_{local,remote} to allow multiple runs of +the event loop while remaining in the CLOSING state. Refactor affected +tests accordingly. + +I verified that all tests in the CLOSING state were behaving is intended +by inserting debug statements in recv/send/ping/pong and running: + +$ PYTHONASYNCIODEBUG=1 python -m unittest -v websockets.test_protocol.{Client,Server}Tests.test_{recv,send,ping,pong}_on_closing_connection_{local,remote} + +Fix #317, #327, #350, #357. +--- + websockets/protocol.py | 10 +++--- + websockets/test_protocol.py | 78 +++++++++++++++++++++++++++++++++++---------- + 2 files changed, 66 insertions(+), 22 deletions(-) + +diff --git a/websockets/protocol.py b/websockets/protocol.py +index f8121a1..7583fe9 100644 +--- a/websockets/protocol.py ++++ b/websockets/protocol.py +@@ -303,7 +303,7 @@ def recv(self): + # Don't yield from self.ensure_open() here because messages could be + # received before the closing frame even if the connection is closing. + +- # Wait for a message until the connection is closed ++ # Wait for a message until the connection is closed. + next_message = asyncio_ensure_future( + self.messages.get(), loop=self.loop) + try: +@@ -315,15 +315,15 @@ def recv(self): + next_message.cancel() + raise + +- # Now there's no need to yield from self.ensure_open(). Either a +- # message was received or the connection was closed. +- + if next_message in done: + return next_message.result() + else: + next_message.cancel() + if not self.legacy_recv: +- raise ConnectionClosed(self.close_code, self.close_reason) ++ assert self.state in [State.CLOSING, State.CLOSED] ++ # Wait until the connection is closed to raise ++ # ConnectionClosed with the correct code and reason. ++ yield from self.ensure_open() + + @asyncio.coroutine + def send(self, data): +diff --git a/websockets/test_protocol.py b/websockets/test_protocol.py +index 70348fb..bfd4e3b 100644 +--- a/websockets/test_protocol.py ++++ b/websockets/test_protocol.py +@@ -105,7 +105,7 @@ def run_loop_once(self): + self.loop.call_soon(self.loop.stop) + self.loop.run_forever() + +- def make_drain_slow(self, delay=3 * MS): ++ def make_drain_slow(self, delay=MS): + # Process connection_made in order to initialize self.protocol.writer. + self.run_loop_once() + +@@ -174,6 +174,8 @@ def close_connection(self, code=1000, reason='close'): + # Empty the outgoing data stream so we can make assertions later on. + self.assertOneFrameSent(True, OP_CLOSE, close_frame_data) + ++ assert self.protocol.state is State.CLOSED ++ + def half_close_connection_local(self, code=1000, reason='close'): + """ + Start a closing handshake but do not complete it. +@@ -181,31 +183,56 @@ def half_close_connection_local(self, code=1000, reason='close'): + The main difference with `close_connection` is that the connection is + left in the CLOSING state until the event loop runs again. + ++ The current implementation returns a task that must be awaited or ++ cancelled, else asyncio complains about destroying a pending task. ++ + """ + close_frame_data = serialize_close(code, reason) +- # Trigger the closing handshake from the local side. +- self.ensure_future(self.protocol.close(code, reason)) ++ # Trigger the closing handshake from the local endpoint. ++ close_task = self.ensure_future(self.protocol.close(code, reason)) + self.run_loop_once() # wait_for executes + self.run_loop_once() # write_frame executes + # Empty the outgoing data stream so we can make assertions later on. + self.assertOneFrameSent(True, OP_CLOSE, close_frame_data) +- # Prepare the response to the closing handshake from the remote side. +- self.loop.call_soon( +- self.receive_frame, Frame(True, OP_CLOSE, close_frame_data)) +- self.loop.call_soon(self.receive_eof_if_client) ++ ++ assert self.protocol.state is State.CLOSING ++ ++ # Complete the closing sequence at 1ms intervals so the test can run ++ # at each point even it goes back to the event loop several times. ++ self.loop.call_later( ++ MS, self.receive_frame, Frame(True, OP_CLOSE, close_frame_data)) ++ self.loop.call_later(2 * MS, self.receive_eof_if_client) ++ ++ # This task must be awaited or cancelled by the caller. ++ return close_task + + def half_close_connection_remote(self, code=1000, reason='close'): + """ +- Receive a closing handshake. ++ Receive a closing handshake but do not complete it. + + The main difference with `close_connection` is that the connection is + left in the CLOSING state until the event loop runs again. + + """ ++ # On the server side, websockets completes the closing handshake and ++ # closes the TCP connection immediately. Yield to the event loop after ++ # sending the close frame to run the test while the connection is in ++ # the CLOSING state. ++ if not self.protocol.is_client: ++ self.make_drain_slow() ++ + close_frame_data = serialize_close(code, reason) +- # Trigger the closing handshake from the remote side. ++ # Trigger the closing handshake from the remote endpoint. + self.receive_frame(Frame(True, OP_CLOSE, close_frame_data)) +- self.receive_eof_if_client() ++ self.run_loop_once() # read_frame executes ++ # Empty the outgoing data stream so we can make assertions later on. ++ self.assertOneFrameSent(True, OP_CLOSE, close_frame_data) ++ ++ assert self.protocol.state is State.CLOSING ++ ++ # Complete the closing sequence at 1ms intervals so the test can run ++ # at each point even it goes back to the event loop several times. ++ self.loop.call_later(2 * MS, self.receive_eof_if_client) + + def process_invalid_frames(self): + """ +@@ -335,11 +362,13 @@ def test_recv_binary(self): + self.assertEqual(data, b'tea') + + def test_recv_on_closing_connection_local(self): +- self.half_close_connection_local() ++ close_task = self.half_close_connection_local() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.recv()) + ++ self.loop.run_until_complete(close_task) # cleanup ++ + def test_recv_on_closing_connection_remote(self): + self.half_close_connection_remote() + +@@ -421,24 +450,29 @@ def test_send_type_error(self): + self.assertNoFrameSent() + + def test_send_on_closing_connection_local(self): +- self.half_close_connection_local() ++ close_task = self.half_close_connection_local() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.send('foobar')) ++ + self.assertNoFrameSent() + ++ self.loop.run_until_complete(close_task) # cleanup ++ + def test_send_on_closing_connection_remote(self): + self.half_close_connection_remote() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.send('foobar')) +- self.assertOneFrameSent(True, OP_CLOSE, serialize_close(1000, 'close')) ++ ++ self.assertNoFrameSent() + + def test_send_on_closed_connection(self): + self.close_connection() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.send('foobar')) ++ + self.assertNoFrameSent() + + # Test the ping coroutine. +@@ -466,24 +500,29 @@ def test_ping_type_error(self): + self.assertNoFrameSent() + + def test_ping_on_closing_connection_local(self): +- self.half_close_connection_local() ++ close_task = self.half_close_connection_local() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.ping()) ++ + self.assertNoFrameSent() + ++ self.loop.run_until_complete(close_task) # cleanup ++ + def test_ping_on_closing_connection_remote(self): + self.half_close_connection_remote() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.ping()) +- self.assertOneFrameSent(True, OP_CLOSE, serialize_close(1000, 'close')) ++ ++ self.assertNoFrameSent() + + def test_ping_on_closed_connection(self): + self.close_connection() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.ping()) ++ + self.assertNoFrameSent() + + # Test the pong coroutine. +@@ -506,24 +545,29 @@ def test_pong_type_error(self): + self.assertNoFrameSent() + + def test_pong_on_closing_connection_local(self): +- self.half_close_connection_local() ++ close_task = self.half_close_connection_local() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.pong()) ++ + self.assertNoFrameSent() + ++ self.loop.run_until_complete(close_task) # cleanup ++ + def test_pong_on_closing_connection_remote(self): + self.half_close_connection_remote() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.pong()) +- self.assertOneFrameSent(True, OP_CLOSE, serialize_close(1000, 'close')) ++ ++ self.assertNoFrameSent() + + def test_pong_on_closed_connection(self): + self.close_connection() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.pong()) ++ + self.assertNoFrameSent() + + # Test the protocol's logic for acknowledging pings with pongs. -- 2.17.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH v2 1/1] python-websockets: backport fix for upstream issue #350 2018-05-07 22:18 ` [Buildroot] [PATCH v2 " Joseph Kogut @ 2018-05-08 3:48 ` Baruch Siach 2018-05-10 22:17 ` [Buildroot] [PATCH v3 " Joseph Kogut 1 sibling, 0 replies; 9+ messages in thread From: Baruch Siach @ 2018-05-08 3:48 UTC (permalink / raw) To: buildroot Hi Joseph, On Mon, May 07, 2018 at 03:18:23PM -0700, Joseph Kogut wrote: > Fetch from: https://github.com/aaugustin/websockets/commit/402059e4a46a764632eba8a669f5b012f173ee7b.patch > > Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com> > --- > Changes v1 -> v2: > - Include patch locally (suggested by Thomas) > > ...59e4a46a764632eba8a669f5b012f173ee7b.patch | 256 ++++++++++++++++++ > 1 file changed, 256 insertions(+) > create mode 100644 package/python-websockets/402059e4a46a764632eba8a669f5b012f173ee7b.patch > > diff --git a/package/python-websockets/402059e4a46a764632eba8a669f5b012f173ee7b.patch b/package/python-websockets/402059e4a46a764632eba8a669f5b012f173ee7b.patch The patch file name should be numbered. See the patch naming conversion in section 18.1.2 of the manual: https://buildroot.org/downloads/manual/manual.html#_providing_patches You can easily generate this patch using 'git format-patch': git format-patch -N --no-thread -s -1 402059e4a46a76 > new file mode 100644 > index 0000000000..62e93bdd2e > --- /dev/null > +++ b/package/python-websockets/402059e4a46a764632eba8a669f5b012f173ee7b.patch > @@ -0,0 +1,256 @@ > +From 402059e4a46a764632eba8a669f5b012f173ee7b Mon Sep 17 00:00:00 2001 > +From: Aymeric Augustin <aymeric.augustin@m4x.org> > +Date: Tue, 1 May 2018 17:05:05 +0200 > +Subject: [PATCH] Fix behavior of recv() in the CLOSING state. > + > +The behavior wasn't tested correctly: in some test cases, the connection > +had already moved to the CLOSED state, where the close code and reason > +are already known. > + > +Refactor half_close_connection_{local,remote} to allow multiple runs of > +the event loop while remaining in the CLOSING state. Refactor affected > +tests accordingly. > + > +I verified that all tests in the CLOSING state were behaving is intended > +by inserting debug statements in recv/send/ping/pong and running: > + > +$ PYTHONASYNCIODEBUG=1 python -m unittest -v websockets.test_protocol.{Client,Server}Tests.test_{recv,send,ping,pong}_on_closing_connection_{local,remote} > + > +Fix #317, #327, #350, #357. Your sign-off is missing. The -s option for git format-patch adds that for you. > +--- > + websockets/protocol.py | 10 +++--- > + websockets/test_protocol.py | 78 +++++++++++++++++++++++++++++++++++---------- > + 2 files changed, 66 insertions(+), 22 deletions(-) baruch -- http://baruch.siach.name/blog/ ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il - ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH v3 1/1] python-websockets: backport fix for upstream issue #350 2018-05-07 22:18 ` [Buildroot] [PATCH v2 " Joseph Kogut 2018-05-08 3:48 ` Baruch Siach @ 2018-05-10 22:17 ` Joseph Kogut 2018-05-11 21:21 ` Thomas Petazzoni 2018-05-28 14:23 ` Peter Korsgaard 1 sibling, 2 replies; 9+ messages in thread From: Joseph Kogut @ 2018-05-10 22:17 UTC (permalink / raw) To: buildroot Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com> Changes v2 -> v3: - Fix missing Signed-off-by for included patch (suggested by Baruch) - Fix patch naming (suggested by Baruch) Changes v1 -> v2: - Include patch locally (suggested by Thomas) --- ...ehavior-of-recv-in-the-CLOSING-state.patch | 261 ++++++++++++++++++ 1 file changed, 261 insertions(+) create mode 100644 package/python-websockets/0001-Fix-behavior-of-recv-in-the-CLOSING-state.patch diff --git a/package/python-websockets/0001-Fix-behavior-of-recv-in-the-CLOSING-state.patch b/package/python-websockets/0001-Fix-behavior-of-recv-in-the-CLOSING-state.patch new file mode 100644 index 0000000000..cb97b36bde --- /dev/null +++ b/package/python-websockets/0001-Fix-behavior-of-recv-in-the-CLOSING-state.patch @@ -0,0 +1,261 @@ +From 402059e4a46a764632eba8a669f5b012f173ee7b Mon Sep 17 00:00:00 2001 +From: Aymeric Augustin <aymeric.augustin@m4x.org> +Date: Tue, 1 May 2018 17:05:05 +0200 +Subject: [PATCH] Fix behavior of recv() in the CLOSING state. + +The behavior wasn't tested correctly: in some test cases, the connection +had already moved to the CLOSED state, where the close code and reason +are already known. + +Refactor half_close_connection_{local,remote} to allow multiple runs of +the event loop while remaining in the CLOSING state. Refactor affected +tests accordingly. + +I verified that all tests in the CLOSING state were behaving is intended +by inserting debug statements in recv/send/ping/pong and running: + +$ PYTHONASYNCIODEBUG=1 python -m unittest -v websockets.test_protocol.{Client,Server}Tests.test_{recv,send,ping,pong}_on_closing_connection_{local,remote} + +Fix #317, #327, #350, #357. + +Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com> +--- + websockets/protocol.py | 10 ++--- + websockets/test_protocol.py | 78 +++++++++++++++++++++++++++++-------- + 2 files changed, 66 insertions(+), 22 deletions(-) + +diff --git a/websockets/protocol.py b/websockets/protocol.py +index f8121a1..7583fe9 100644 +--- a/websockets/protocol.py ++++ b/websockets/protocol.py +@@ -303,7 +303,7 @@ class WebSocketCommonProtocol(asyncio.StreamReaderProtocol): + # Don't yield from self.ensure_open() here because messages could be + # received before the closing frame even if the connection is closing. + +- # Wait for a message until the connection is closed ++ # Wait for a message until the connection is closed. + next_message = asyncio_ensure_future( + self.messages.get(), loop=self.loop) + try: +@@ -315,15 +315,15 @@ class WebSocketCommonProtocol(asyncio.StreamReaderProtocol): + next_message.cancel() + raise + +- # Now there's no need to yield from self.ensure_open(). Either a +- # message was received or the connection was closed. +- + if next_message in done: + return next_message.result() + else: + next_message.cancel() + if not self.legacy_recv: +- raise ConnectionClosed(self.close_code, self.close_reason) ++ assert self.state in [State.CLOSING, State.CLOSED] ++ # Wait until the connection is closed to raise ++ # ConnectionClosed with the correct code and reason. ++ yield from self.ensure_open() + + @asyncio.coroutine + def send(self, data): +diff --git a/websockets/test_protocol.py b/websockets/test_protocol.py +index 70348fb..bfd4e3b 100644 +--- a/websockets/test_protocol.py ++++ b/websockets/test_protocol.py +@@ -105,7 +105,7 @@ class CommonTests: + self.loop.call_soon(self.loop.stop) + self.loop.run_forever() + +- def make_drain_slow(self, delay=3 * MS): ++ def make_drain_slow(self, delay=MS): + # Process connection_made in order to initialize self.protocol.writer. + self.run_loop_once() + +@@ -174,6 +174,8 @@ class CommonTests: + # Empty the outgoing data stream so we can make assertions later on. + self.assertOneFrameSent(True, OP_CLOSE, close_frame_data) + ++ assert self.protocol.state is State.CLOSED ++ + def half_close_connection_local(self, code=1000, reason='close'): + """ + Start a closing handshake but do not complete it. +@@ -181,31 +183,56 @@ class CommonTests: + The main difference with `close_connection` is that the connection is + left in the CLOSING state until the event loop runs again. + ++ The current implementation returns a task that must be awaited or ++ cancelled, else asyncio complains about destroying a pending task. ++ + """ + close_frame_data = serialize_close(code, reason) +- # Trigger the closing handshake from the local side. +- self.ensure_future(self.protocol.close(code, reason)) ++ # Trigger the closing handshake from the local endpoint. ++ close_task = self.ensure_future(self.protocol.close(code, reason)) + self.run_loop_once() # wait_for executes + self.run_loop_once() # write_frame executes + # Empty the outgoing data stream so we can make assertions later on. + self.assertOneFrameSent(True, OP_CLOSE, close_frame_data) +- # Prepare the response to the closing handshake from the remote side. +- self.loop.call_soon( +- self.receive_frame, Frame(True, OP_CLOSE, close_frame_data)) +- self.loop.call_soon(self.receive_eof_if_client) ++ ++ assert self.protocol.state is State.CLOSING ++ ++ # Complete the closing sequence at 1ms intervals so the test can run ++ # at each point even it goes back to the event loop several times. ++ self.loop.call_later( ++ MS, self.receive_frame, Frame(True, OP_CLOSE, close_frame_data)) ++ self.loop.call_later(2 * MS, self.receive_eof_if_client) ++ ++ # This task must be awaited or cancelled by the caller. ++ return close_task + + def half_close_connection_remote(self, code=1000, reason='close'): + """ +- Receive a closing handshake. ++ Receive a closing handshake but do not complete it. + + The main difference with `close_connection` is that the connection is + left in the CLOSING state until the event loop runs again. + + """ ++ # On the server side, websockets completes the closing handshake and ++ # closes the TCP connection immediately. Yield to the event loop after ++ # sending the close frame to run the test while the connection is in ++ # the CLOSING state. ++ if not self.protocol.is_client: ++ self.make_drain_slow() ++ + close_frame_data = serialize_close(code, reason) +- # Trigger the closing handshake from the remote side. ++ # Trigger the closing handshake from the remote endpoint. + self.receive_frame(Frame(True, OP_CLOSE, close_frame_data)) +- self.receive_eof_if_client() ++ self.run_loop_once() # read_frame executes ++ # Empty the outgoing data stream so we can make assertions later on. ++ self.assertOneFrameSent(True, OP_CLOSE, close_frame_data) ++ ++ assert self.protocol.state is State.CLOSING ++ ++ # Complete the closing sequence at 1ms intervals so the test can run ++ # at each point even it goes back to the event loop several times. ++ self.loop.call_later(2 * MS, self.receive_eof_if_client) + + def process_invalid_frames(self): + """ +@@ -335,11 +362,13 @@ class CommonTests: + self.assertEqual(data, b'tea') + + def test_recv_on_closing_connection_local(self): +- self.half_close_connection_local() ++ close_task = self.half_close_connection_local() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.recv()) + ++ self.loop.run_until_complete(close_task) # cleanup ++ + def test_recv_on_closing_connection_remote(self): + self.half_close_connection_remote() + +@@ -421,24 +450,29 @@ class CommonTests: + self.assertNoFrameSent() + + def test_send_on_closing_connection_local(self): +- self.half_close_connection_local() ++ close_task = self.half_close_connection_local() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.send('foobar')) ++ + self.assertNoFrameSent() + ++ self.loop.run_until_complete(close_task) # cleanup ++ + def test_send_on_closing_connection_remote(self): + self.half_close_connection_remote() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.send('foobar')) +- self.assertOneFrameSent(True, OP_CLOSE, serialize_close(1000, 'close')) ++ ++ self.assertNoFrameSent() + + def test_send_on_closed_connection(self): + self.close_connection() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.send('foobar')) ++ + self.assertNoFrameSent() + + # Test the ping coroutine. +@@ -466,24 +500,29 @@ class CommonTests: + self.assertNoFrameSent() + + def test_ping_on_closing_connection_local(self): +- self.half_close_connection_local() ++ close_task = self.half_close_connection_local() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.ping()) ++ + self.assertNoFrameSent() + ++ self.loop.run_until_complete(close_task) # cleanup ++ + def test_ping_on_closing_connection_remote(self): + self.half_close_connection_remote() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.ping()) +- self.assertOneFrameSent(True, OP_CLOSE, serialize_close(1000, 'close')) ++ ++ self.assertNoFrameSent() + + def test_ping_on_closed_connection(self): + self.close_connection() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.ping()) ++ + self.assertNoFrameSent() + + # Test the pong coroutine. +@@ -506,24 +545,29 @@ class CommonTests: + self.assertNoFrameSent() + + def test_pong_on_closing_connection_local(self): +- self.half_close_connection_local() ++ close_task = self.half_close_connection_local() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.pong()) ++ + self.assertNoFrameSent() + ++ self.loop.run_until_complete(close_task) # cleanup ++ + def test_pong_on_closing_connection_remote(self): + self.half_close_connection_remote() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.pong()) +- self.assertOneFrameSent(True, OP_CLOSE, serialize_close(1000, 'close')) ++ ++ self.assertNoFrameSent() + + def test_pong_on_closed_connection(self): + self.close_connection() + + with self.assertRaises(ConnectionClosed): + self.loop.run_until_complete(self.protocol.pong()) ++ + self.assertNoFrameSent() + + # Test the protocol's logic for acknowledging pings with pongs. +-- +2.17.0 + -- 2.17.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH v3 1/1] python-websockets: backport fix for upstream issue #350 2018-05-10 22:17 ` [Buildroot] [PATCH v3 " Joseph Kogut @ 2018-05-11 21:21 ` Thomas Petazzoni 2018-05-11 21:26 ` Joseph Kogut 2018-05-28 14:23 ` Peter Korsgaard 1 sibling, 1 reply; 9+ messages in thread From: Thomas Petazzoni @ 2018-05-11 21:21 UTC (permalink / raw) To: buildroot Hello Joseph, On Thu, 10 May 2018 15:17:12 -0700, Joseph Kogut wrote: > Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com> > > Changes v2 -> v3: > - Fix missing Signed-off-by for included patch (suggested by > Baruch) > - Fix patch naming (suggested by Baruch) > > Changes v1 -> v2: > - Include patch locally (suggested by Thomas) The changelog is good, but it should be... > --- ... below this sign. Otherwise the changelog is preserved in the commit description when applying the patch, which is not what we want. I've fixed that up and applied to master. Thanks! Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH v3 1/1] python-websockets: backport fix for upstream issue #350 2018-05-11 21:21 ` Thomas Petazzoni @ 2018-05-11 21:26 ` Joseph Kogut 0 siblings, 0 replies; 9+ messages in thread From: Joseph Kogut @ 2018-05-11 21:26 UTC (permalink / raw) To: buildroot Hi Thomas, On Fri, May 11, 2018 at 2:21 PM, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > Hello Joseph, > > On Thu, 10 May 2018 15:17:12 -0700, Joseph Kogut wrote: >> Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com> >> >> Changes v2 -> v3: >> - Fix missing Signed-off-by for included patch (suggested by >> Baruch) >> - Fix patch naming (suggested by Baruch) >> >> Changes v1 -> v2: >> - Include patch locally (suggested by Thomas) > > The changelog is good, but it should be... > >> --- > > ... below this sign. > Whoops, I'm not sure how I missed that. > Otherwise the changelog is preserved in the commit description when > applying the patch, which is not what we want. > > I've fixed that up and applied to master. Thanks! > Thanks. > Thomas > -- > Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) > Embedded Linux and Kernel engineering > https://bootlin.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Buildroot] [PATCH v3 1/1] python-websockets: backport fix for upstream issue #350 2018-05-10 22:17 ` [Buildroot] [PATCH v3 " Joseph Kogut 2018-05-11 21:21 ` Thomas Petazzoni @ 2018-05-28 14:23 ` Peter Korsgaard 1 sibling, 0 replies; 9+ messages in thread From: Peter Korsgaard @ 2018-05-28 14:23 UTC (permalink / raw) To: buildroot >>>>> "Joseph" == Joseph Kogut <joseph.kogut@gmail.com> writes: > Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com> > Changes v2 -> v3: > - Fix missing Signed-off-by for included patch (suggested by > Baruch) > - Fix patch naming (suggested by Baruch) > Changes v1 -> v2: > - Include patch locally (suggested by Thomas) Committed to 2018.02.x, thanks. -- Bye, Peter Korsgaard ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-05-28 14:23 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-05-07 20:57 [Buildroot] [PATCH 1/1] python-websockets: backport fix for upstream issue #350 Joseph Kogut 2018-05-07 21:26 ` Thomas Petazzoni 2018-05-07 22:07 ` Joseph Kogut 2018-05-07 22:18 ` [Buildroot] [PATCH v2 " Joseph Kogut 2018-05-08 3:48 ` Baruch Siach 2018-05-10 22:17 ` [Buildroot] [PATCH v3 " Joseph Kogut 2018-05-11 21:21 ` Thomas Petazzoni 2018-05-11 21:26 ` Joseph Kogut 2018-05-28 14:23 ` Peter Korsgaard
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.