From 816ee1c33a7c9f3db917dc1a1db9f2dd0c6e565b Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Mon, 16 May 2016 20:28:49 -0700 Subject: [PATCH 1/9] Small updates to changes.rst --- docs/source/changes.rst | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/source/changes.rst b/docs/source/changes.rst index 38ce8df..51eea69 100644 --- a/docs/source/changes.rst +++ b/docs/source/changes.rst @@ -3,8 +3,8 @@ History of changes .. currentmodule:: h11 -vNEXT ------ +vNEXT (????-??-??) +------------------ Backwards **in**\compatible changes: @@ -18,13 +18,13 @@ Backwards compatible changes: triggered by our peer being in the :data:`ERROR` state. * Split :exc:`ProtocolError` into :exc:`LocalProtocolError` and - :exc:`RemoteProtocolError`. Use case: HTTP servers want to be able - to distinguish between an error that originates locally (which - produce a 500 status code) versus errors caused by remote - misbehavior (which produce a 4xx status code). + :exc:`RemoteProtocolError` (see :ref:`error-handling`). Use case: HTTP + servers want to be able to distinguish between an error that + originates locally (which produce a 500 status code) versus errors + caused by remote misbehavior (which produce a 4xx status code). -v0.5.0 ------- +v0.5.0 (2016-05-14) +------------------- * Initial release. From 4affece216a48a187eed4ff8a97c2f6776694b72 Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Mon, 16 May 2016 20:28:55 -0700 Subject: [PATCH 2/9] s/~./~/ in the docs I'm not sure why I thought the Sphinx syntax for short link text was like :meth:`~.Class.foo`, but let's switch to the more standard :meth:`~Class.foo`. (Apparently both ways work?) (ref: http://www.sphinx-doc.org/en/stable/markup/inline.html#cross-referencing-syntax) --- docs/source/api.rst | 44 ++++++++++++++++++------------------- docs/source/basic-usage.rst | 4 ++-- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/docs/source/api.rst b/docs/source/api.rst index 33f9b56..433953a 100644 --- a/docs/source/api.rst +++ b/docs/source/api.rst @@ -47,9 +47,9 @@ All events behave in essentially similar ways. Let's take :class:`Request` as an example. Like all events, this is a "final" class -- you cannot subclass it. And like all events, it has several fields. For :class:`Request`, there are four of them: -:attr:`~.Request.method`, :attr:`~.Request.target``, -:attr:`~.Request.headers``, and -:attr:`~.Request.http_version``. :attr:`~.Request.http_version`` +:attr:`~Request.method`, :attr:`~Request.target``, +:attr:`~Request.headers``, and +:attr:`~Request.http_version``. :attr:`~Request.http_version`` defaults to ``b"1.1"``; the rest have no default, so to create a :class:`Request` you have to specify their values: @@ -430,7 +430,7 @@ There are four cases where these exceptions might be raised: don't know what they're doing and we cannot safely proceed. :attr:`Connection.their_state` immediately becomes :data:`ERROR`, and all further calls to - :meth:`~.Connection.receive_data` will also raise + :meth:`~Connection.receive_data` will also raise :exc:`ProtocolError`. :meth:`Connection.send` still works as normal, so if you're implementing a server and this happens then you have an opportunity to send back a 400 Bad Request response. Your only other @@ -443,7 +443,7 @@ There are four cases where these exceptions might be raised: what you're doing, its internal state may be inconsistent, and we cannot safely proceed. :attr:`Connection.our_state` immediately becomes :data:`ERROR`, and all further calls to - :meth:`~.Connection.send` will also raise :exc:`ProtocolError`. The + :meth:`~Connection.send` will also raise :exc:`ProtocolError`. The only thing you can reasonably due at this point is to close your socket and make a new connection. @@ -567,7 +567,7 @@ lack of protocol support, one of the sides just unilaterally closed the connection -- then the state machines will skip past the :data:`DONE` state directly to the :data:`MUST_CLOSE` or :data:`CLOSED` states. In this case, trying to call -:meth:`~.Connection.prepare_to_use` will raise an error, and the only +:meth:`~Connection.prepare_to_use` will raise an error, and the only thing you can legally do is to close this connection and make a new one. @@ -580,17 +580,17 @@ because it makes things like error recovery very complicated. As a client, h11 does not support pipelining. This is enforced by the structure of the state machine: after sending one :class:`Request`, you can't send another until after calling -:meth:`~.Connection.prepare_to_reuse`, and you can't call -:meth:`~.Connection.prepare_to_reuse` until the server has entered the +:meth:`~Connection.prepare_to_reuse`, and you can't call +:meth:`~Connection.prepare_to_reuse` until the server has entered the :data:`DONE` state, which requires reading the server's full response. As a server, h11 provides the minimal support for pipelining required to comply with the HTTP/1.1 standard: if the client sends multiple pipelined requests, then we the first request until we reach the -:data:`DONE` state, and then :meth:`~.Connection.receive_data` will +:data:`DONE` state, and then :meth:`~Connection.receive_data` will pause and refuse to parse any more events until the response is -completed and :meth:`~.Connection.prepare_to_reuse` is called. See the +completed and :meth:`~Connection.prepare_to_reuse` is called. See the next section for more details. @@ -600,8 +600,8 @@ Flow control ------------ h11 always does the absolute minimum of buffering that it can get away -with: :meth:`~.Connection.send` always returns the full data to send -immediately, and :meth:`~.Connection.recieve_data` always greedily +with: :meth:`~Connection.send` always returns the full data to send +immediately, and :meth:`~Connection.recieve_data` always greedily parses and returns as many events as possible from its current buffer. So you can be sure that no data or events will suddenly appear and need processing, except when you call these methods. And @@ -644,8 +644,8 @@ waiting to read data, always execute some code like: The other mechanism h11 provides to help you manage read flow control is the :class:`Paused` pseudo-event. Unlike other events, the :class:`Paused` event doesn't contain information sent from the remote -peer; if :meth:`~.Connection.receive_data` returns one of these, it -means that :meth:`~.Connection.receive_data` has stopped processing +peer; if :meth:`~Connection.receive_data` returns one of these, it +means that :meth:`~Connection.receive_data` has stopped processing its data buffer and isn't going to process any more until the remote peer's state (:attr:`Connection.their_state`) changes to something different. @@ -655,10 +655,10 @@ There are three possible reasons to enter a paused state: * The remote peer is in the :data:`DONE` state, but sent more data, i.e., a client is attempting to :ref:`pipeline requests `. In the :data:`DONE` state, - :meth:`~.Connection.receive_data` can return :class:`ConnectionClosed` + :meth:`~Connection.receive_data` can return :class:`ConnectionClosed` events, but if any actual data is received then it will pause, and stay that way until a successful call to - :meth:`~.Connection.prepare_to_reuse`. + :meth:`~Connection.prepare_to_reuse`. * The remote client is in the :data:`MIGHT_SWITCH_PROTOCOL` state (see :ref:`switching-protocols`). This really shouldn't happen, because @@ -673,18 +673,18 @@ There are three possible reasons to enter a paused state: us. If this happens, we pause. Once the connection has entered a paused state, then it's safe to keep -calling :meth:`~.Connection.receive_data` -- it will just keep +calling :meth:`~Connection.receive_data` -- it will just keep returning new :class:`Paused` events -- but instead you should probably stop reading from the network; all you're going to accomplish is to shove more and more data into our internal buffers, where it's just going to there using more and more memory. (And we do *not* enforce the regular maximum buffer size limits when in a paused state -- if we did then you might go over the limit in a single call to -:meth:`~.Connection.receive_data`, not because you or the remote peer +:meth:`~Connection.receive_data`, not because you or the remote peer did anything wrong, but just because a fair amount of data all came in at the same time we entered the paused state.) And simply reading more data will never trigger an unpause -- for that something external has -to happen, usually a call to :meth:`~.Connection.prepare_to_reuse`. +to happen, usually a call to :meth:`~Connection.prepare_to_reuse`. And that's the other tricky moment: when you come out of a paused state, you shouldn't immediately read from the network. Consider the @@ -724,7 +724,7 @@ data now, then we'll be waiting forever. That would be bad. Instead, what we have to do after unpausing is make an explicit call -to :meth:`~.Connection.receive_data` with ``None`` as the argument, +to :meth:`~Connection.receive_data` with ``None`` as the argument, which means "I don't have any more data for you, but could you check the data you already have buffered in case there's anything else you can parse now that you couldn't before?". And once we've done this and @@ -748,13 +748,13 @@ Closing connections h11 represents a connection shutdown with the special event type :class:`ConnectionClosed`. You can send this event, in which case -:meth:`~.Connection.send` will simply update the state machine and +:meth:`~Connection.send` will simply update the state machine and then return ``None``. You can receive this event, if you call ``conn.receive_data(b"")``. (The actual receipt might be delayed if the connection is :ref:`paused `.) It's safe and legal to call ``conn.receive_data(b"")`` multiple times, and once you've done this once, then all future calls to -:meth:`~.Connection.receive_data` will also return +:meth:`~Connection.receive_data` will also return ``ConnectionClosed()``: .. ipython:: python diff --git a/docs/source/basic-usage.rst b/docs/source/basic-usage.rst index 2086a0e..1f01e30 100644 --- a/docs/source/basic-usage.rst +++ b/docs/source/basic-usage.rst @@ -80,7 +80,7 @@ send -- in this case, a :class:`h11.Request`: target="/xml", headers=[("Host", "httpbin.org")]) -Next, we pass this to our connection's :meth:`~.Connection.send` +Next, we pass this to our connection's :meth:`~Connection.send` method, which gives us back the bytes corresponding to this message: .. ipython:: python @@ -152,7 +152,7 @@ earlier and has the response's status code (200 OK) and headers; a :class:`EndOfMessage` object. This similarity between what we send and what we receive isn't accidental: if we were using h11 to write an HTTP server, then these are the objects we would have created and passed to -:meth:`~.Connection.send` -- h11 in client and server mode has an API +:meth:`~Connection.send` -- h11 in client and server mode has an API that's almost exactly symmetric. From 2f68f1772a412796b217eaf67c9ac37b7587dab9 Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Mon, 16 May 2016 20:25:16 -0700 Subject: [PATCH 3/9] Get rid of Paused pseudo-event Discovered that it made writing the curio server example rather tricky and difficult -- my server code already paused reading at the appropriate times, but then I had to watch out for these Paused events cluttering up my event stream... --- docs/source/api.rst | 226 ++++++++++++++++++++------------- docs/source/changes.rst | 3 + docs/source/supported-http.rst | 14 ++ fuzz/README.rst | 2 +- h11/_connection.py | 44 +++---- h11/_events.py | 30 ----- h11/tests/test_connection.py | 90 +++++-------- h11/tests/test_events.py | 3 - 8 files changed, 204 insertions(+), 208 deletions(-) diff --git a/docs/source/api.rst b/docs/source/api.rst index 433953a..a35e8ad 100644 --- a/docs/source/api.rst +++ b/docs/source/api.rst @@ -18,11 +18,10 @@ directly at the top level: In [3]: h11. h11.CLIENT h11.MIGHT_SWITCH_PROTOCOL h11.CLOSED h11.MUST_CLOSE - h11.Connection h11.Paused - h11.ConnectionClosed h11.PRODUCT_ID - h11.Data h11.ProtocolError - h11.DONE h11.RemoteProtocolError - h11.egg-info/ h11.Request + h11.Connection h11.PRODUCT_ID + h11.ConnectionClosed h11.ProtocolError + h11.Data h11.RemoteProtocolError + h11.DONE h11.Request h11.EndOfMessage h11.Response h11.ERROR h11.SEND_BODY h11.IDLE h11.SEND_RESPONSE @@ -160,8 +159,6 @@ Here's the complete set of events supported by h11: .. autoclass:: ConnectionClosed -.. autoclass:: Paused - .. _state-machine: @@ -599,95 +596,94 @@ next section for more details. Flow control ------------ -h11 always does the absolute minimum of buffering that it can get away -with: :meth:`~Connection.send` always returns the full data to send -immediately, and :meth:`~Connection.recieve_data` always greedily -parses and returns as many events as possible from its current -buffer. So you can be sure that no data or events will suddenly appear -and need processing, except when you call these methods. And -presumably you know when you want to send things. But there is one -thing you still need to know: you don't want to read data from the -remote peer if it can't be processed (i.e., you want to apply -backpressure and avoid building arbitrarily large buffers), and you -definitely don't want to block waiting on data from the remote peer at -the same time that it's blocked waiting for you, because that will -cause a deadlock. - -We assume that if you're implementing a client then you're clever -enough not to sit around trying to read more data from the server when -there's no response pending. But there are a few more subtle ways that -reading in HTTP can go wrong, and h11 provides two ways to help you -avoid these situations. - -First, it keeps track of the `client's ``Expect: 100-continue`` status -`_. you can read -the spec for details, but basically the way this works if that -sometimes clients will send a :class:`Request` with an ``Expect: -100-continue`` header, and then they will stop there, before sending -the body, until they see some response from the server (or possibly -some timeout occurs). The server's response can be an -:class:`InformationalResponse` with status ``100 Continue``, or -anything really (e.g. a full :class:`Response` with an error +h11 always does the absolute minimum of buffering that it can manage: +:meth:`~Connection.send` and +:meth:`~Connection.send_with_data_passthrough` always return the full +data to send immediately, and :meth:`~Connection.recieve_data` always +greedily parses and returns as many events as possible from its +current buffer (with one exception discussed below). So you can be +sure that no data or events will suddenly appear and need processing, +except when you call these methods. And presumably you know when you +want to send things. But there is one thing you still need to know, +which is when it's safe to read from the remote peer: you don't want +to read data from the remote peer if it can't be processed (i.e., you +want to apply backpressure and avoid building arbitrarily large +in-memory buffers), and you definitely don't want to block waiting on +data from the remote peer at the same time that it's blocked waiting +for you, because that will cause a deadlock. + +For a client, this is simple: send your request, then read the +response, then stop reading until after you've sent another request. + +For a server, there are three more subtle points you need to keep +track of. + +**First**, you have to be prepared to handle :class:`Request`\s with `an +``Expect: 100-continue`` header +`_. You can read +the spec for details, but basically what this header means is that +after sending the :class:`Request`, the client plans to pause and wait +until they see some response from the server before sending any +:class:`Data` (or possibly some timeout occurs). The server's response +can be an :class:`InformationalResponse` with status ``100 Continue``, +or anything really (e.g. a full :class:`Response` with an error code). The crucial thing as a server, though, is that you should never block trying to read a request body if the client is blocked waiting for you to tell them to send the request body. -The simple way to avoid this is to make sure that before you block -waiting to read data, always execute some code like: +Fortunately, h11 makes this easy, because it tracks whether the client +is blocked, and exposes this as +:attr:`Connection.they_are_waiting_for_100_continue`. So you don't +have to pay attention to the ``Expect`` header yourself; you just have +to make sure that before you block waiting to read a request body, you +execute some code like: .. code-block:: python if conn.they_are_waiting_for_100_continue: - send(conn, h11.InformationalResponse(100, headers=[...])) + do_send(conn, h11.InformationalResponse(100, headers=[...])) do_read(...) -The other mechanism h11 provides to help you manage read flow control -is the :class:`Paused` pseudo-event. Unlike other events, the -:class:`Paused` event doesn't contain information sent from the remote -peer; if :meth:`~Connection.receive_data` returns one of these, it -means that :meth:`~Connection.receive_data` has stopped processing -its data buffer and isn't going to process any more until the remote -peer's state (:attr:`Connection.their_state`) changes to something -different. - -There are three possible reasons to enter a paused state: - -* The remote peer is in the :data:`DONE` state, but sent more data, - i.e., a client is attempting to :ref:`pipeline requests - `. In the :data:`DONE` state, - :meth:`~Connection.receive_data` can return :class:`ConnectionClosed` - events, but if any actual data is received then it will pause, and - stay that way until a successful call to - :meth:`~Connection.prepare_to_reuse`. - -* The remote client is in the :data:`MIGHT_SWITCH_PROTOCOL` state (see - :ref:`switching-protocols`). This really shouldn't happen, because - they don't know yet whether the protocol switch will actually happen, - but OTOH it certainly isn't correct for us to go ahead and parse the - data they sent as if it were HTTP, when it might not be. So if this - happens, we pause. - -* The remote peer is in the :data:`SWITCHED_PROTOCOL` state (see - :ref:`switching-protocols`). We certainly aren't going to try to - parse their data -- it's not HTTP, or at least not HTTP directed at - us. If this happens, we pause. - -Once the connection has entered a paused state, then it's safe to keep -calling :meth:`~Connection.receive_data` -- it will just keep -returning new :class:`Paused` events -- but instead you should -probably stop reading from the network; all you're going to accomplish -is to shove more and more data into our internal buffers, where it's -just going to there using more and more memory. (And we do *not* -enforce the regular maximum buffer size limits when in a paused state --- if we did then you might go over the limit in a single call to -:meth:`~Connection.receive_data`, not because you or the remote peer -did anything wrong, but just because a fair amount of data all came in -at the same time we entered the paused state.) And simply reading more -data will never trigger an unpause -- for that something external has -to happen, usually a call to :meth:`~Connection.prepare_to_reuse`. - -And that's the other tricky moment: when you come out of a paused -state, you shouldn't immediately read from the network. Consider the +In fact, if you're lazy (and what programmer isn't?) then you can just +do this check before all reads -- it's mandatory before blocking to +read a request body, but it's safe at any time. + +**Second**, if you're using some sort of I/O setup where you read all +the time (e.g. in a dedicated thread or some sort of proactor), then +you should stop reading when you get each request's +:class:`EndOfMessage`, and don't start again until after you call +:meth:`~Connection.prepare_to_reuse`. (Of course you should also +follow this rule if you're using blocking I/O, but hopefully that is +obvious -- what's not so obvious is that this applies also to threaded +reads.) If you ignore this rule, then nothing particularly bad will +happen -- but nothing useful will happen, either. After seeing the +:class:`EndOfMessage`, then :meth:`~Connection.receive_data` will +enter a "paused" state: you can call it, and it'll add any data that +you give it to its internal buffer, but it won't actually attempt to +parse that buffer or return any events (unless the next data you give +it is ``b""`` indicating that the client has closed their connection, +in which case it will return :class:`ConnectionClosed`). This state +also does another thing: normally, as an anti-denial-of-service +protection, h11 will error out if its internal receive buffer gets too +big; but when we are in this paused state, this protection is +disabled. (This is important because without it, we'd run the risk of +treating a perfectly innocent client as if they were a DoS attacker, +just because they sent a number of pipelined requests that all arrived +at the same time.) + +So really it's best if you just stop reading from the remote socket +during this period. If you're lucky this might even provide some +back-pressure to the remote client to stop sending stuff so fast. + +(Note: There isn't currently any API exposed to tell you whether +receiving is paused; you're expected to "just know" based on what +you've recently done. So far this seems to work fine for the servers +I've written, but if you'd find this useful then please let us know -- +it wouldn't be hard to add.) + +And **third**, coming out of the paused state requires a bit of +care. After you call :meth:`~Connection.prepare_to_reuse`, you +*must not* immediately start reading from the network. Consider the situation where a client sends two pipelined requests, and then blocks waiting for the two responses. It's possible the two requests will arrive together, and be enqueued into our receive buffer together: @@ -701,7 +697,7 @@ arrive together, and be enqueued into our receive buffer together: ) Notice how we get back only the first :class:`Request` and its (empty) -body, then a :class:`Paused` event. +body. We process the first request: @@ -735,10 +731,58 @@ processed the events we get back, we can continue as normal: conn.receive_data(None) It is always safe to call ``conn.receive_data(None)``; if there aren't -any new events to return, it will simply return ``[]``, and if the -connection is paused, it will return a :class:`Paused` event. If you -want to be conservative, you can defensively call this immediately -before issuing any blocking read. +any new events to return, it will simply return ``[]``. If you want to +be conservative, you can defensively call this immediately before +issuing any blocking read. + + +Technical details on how the receive_data pause state is implemented +.................................................................... + +You probably don't need to know this to write a client or server, but +perhaps you're curious. + +Internally, the pause logic doesn't distinguish between whether we're +implementing a client or a server, and there are actually three +possible reasons we might enter the paused state: + +* The remote peer is in the :data:`DONE` state, but sent more data, + i.e., a client is attempting to :ref:`pipeline requests + `. In the :data:`DONE` state, + :meth:`~Connection.receive_data` can return :class:`ConnectionClosed` + events, but if any actual data is received then it will pause, and + stay that way until a successful call to + :meth:`~Connection.prepare_to_reuse`. + +* The remote client is in the :data:`MIGHT_SWITCH_PROTOCOL` state (see + :ref:`switching-protocols`). This really shouldn't happen, because + they don't know yet whether the protocol switch will actually happen, + but OTOH it certainly isn't correct for us to go ahead and parse the + data they sent as if it were HTTP, when it might not be. So if this + happens, we pause. + +* The remote peer is in the :data:`SWITCHED_PROTOCOL` state (see + :ref:`switching-protocols`). When this happens we certainly + shouldn't try to parse their data. So if this happens we pause. + +How does this relate to the advice given above? + +You don't really need to worry about the :data:`MIGHT_SWITCH_PROTOCOL` +case, because the only way a client can get into this state is by +entering :data:`DONE`, and the only way it can get out is by entering +:data:`DONE` or :data:`SWITCHED_PROTOCOL`. So if you handle those +cases correctly then you'll automatically handle this case too. And +handling of :data:`SWITCHED_PROTOCOL` should be obvious, because if +you've just negotiated a protocol switch you hopefully know that and +won't try to keep using the :class:`Connection` object, and this state +is permanent. So the only case that requires careful handling by your +flow control logic is :data:`DONE`. And if you're implementing a +client, even this isn't something you need to worry about, because it +would be very weird for a server that was in the :data:`DONE` state to +suddenly send another response in a row out-of-the-blue, before you've +sent another request, so the simple rule of "only read when waiting +for a response" covers this case as well. So really it's only servers +that need special flow-control logic to handle pipelining. .. _closing: diff --git a/docs/source/changes.rst b/docs/source/changes.rst index 51eea69..d215208 100644 --- a/docs/source/changes.rst +++ b/docs/source/changes.rst @@ -12,6 +12,9 @@ Backwards **in**\compatible changes: :attr:`Connection.client_state`, :attr:`Connection.server_state` with the new :attr:`Connection.states`. +* Removed the :class:`Paused` pseudo-event -- see :ref:`flow-control` + for the new way things work. + Backwards compatible changes: * State machine: added a :data:`DONE` -> :data:`MUST_CLOSE` transition diff --git a/docs/source/supported-http.rst b/docs/source/supported-http.rst index 5fc33b8..b62a42f 100644 --- a/docs/source/supported-http.rst +++ b/docs/source/supported-http.rst @@ -74,3 +74,17 @@ sure whether anyone actually needs it. Unfortunately a few major implementations that I spot-checked (node.js, go) do still seem to support reading such headers (but not generating them), so it might or might not be obsolete in practice -- it's hard to know. + + +.. _flow-control-details: + +Flow control details +-------------------- + +The :ref:`flow-control` section in the main API docs gives a +user-level explanation of what they need to know about flow control +and the :meth:`Connection.receive_data` "paused" state. The internal +implementation is slightly more involved. + +First, pause handling is actually identical for both the client and +server diff --git a/fuzz/README.rst b/fuzz/README.rst index 7ddfea1..821d549 100644 --- a/fuzz/README.rst +++ b/fuzz/README.rst @@ -53,7 +53,7 @@ Ideas for further additions examples? pipelining and protocol switch examples? * check that the all-at-once and byte-by-byte processing give the same - event stream (modulo ``Paused``, and data splits) + event stream (modulo data splits) * maybe should split apart fancy checks versus non-fancy checks b/c speed is important diff --git a/h11/_connection.py b/h11/_connection.py index 7c5cde0..965a13d 100644 --- a/h11/_connection.py +++ b/h11/_connection.py @@ -375,30 +375,24 @@ def receive_data(self, data): # Read out all the events we can events = [] while True: - event = self._next_receive_event() + event = self._extract_next_receive_event() if event is None: break events.append(event) - # The Paused pseudo-event doesn't go through the state - # machine, because it's purely a local signal. - if type(event) is Paused: - break self._process_event(self.their_role, event) if type(event) is ConnectionClosed: break # Buffer maintainence self._receive_buffer.compress() - if events and type(events[-1]) is Paused: - # We don't enforce buffer size limits when Paused, because - # avoiding ever-growing buffers here indicates a problem with - # the user code, not with the remote client (and otherwise - # it's entirely possible that a single receive_data call all - # by itself could put us over the limit, with no real way to - # avoid it) - pass - else: - if len(self._receive_buffer) > self._max_buffer_size: + if len(self._receive_buffer) > self._max_buffer_size: + # We don't enforce buffer size limits when paused, because + # ever-growing buffers here would indicate a problem with + # the user code, not with the remote client -- and it's + # entirely possible that a single receive_data call all by + # itself could put us over this limit, with no real way to + # avoid it. + if not self._paused(): # 431 is "Request header fields too large" which is pretty # much the only situation where we can get here raise RemoteProtocolError("Receive buffer too long", @@ -409,8 +403,9 @@ def receive_data(self, data): # delivered that ConnectionClosed -- we don't want to hang forever # waiting for data that never arrives. if self._receive_buffer_closed: - FINAL_EVENTS = {Paused, ConnectionClosed} - if not events or type(events[-1]) not in FINAL_EVENTS: + if not (self._paused() + or (events + and type(events[-1]) is ConnectionClosed)): raise RemoteProtocolError( "peer unexpectedly closed connection") @@ -418,21 +413,26 @@ def receive_data(self, data): return events except BaseException as exc: self._process_error(self.their_role) - if type(exc) is LocalProtocolError: + if isinstance(exc, LocalProtocolError): exc._reraise_as_remote_protocol_error() else: raise - def _next_receive_event(self): + def _paused(self): state = self.their_state # We don't pause immediately when they enter DONE, because even in # DONE state we can still process a ConnectionClosed() event. But # if we have data in our buffer, then we definitely aren't getting # a ConnectionClosed() immediately and we need to pause. if state is DONE and self._receive_buffer: - return Paused(reason=state) - if state is MIGHT_SWITCH_PROTOCOL or state is SWITCHED_PROTOCOL: - return Paused(reason=state) + return True + if state in {MIGHT_SWITCH_PROTOCOL, SWITCHED_PROTOCOL}: + return True + return False + + def _extract_next_receive_event(self): + if self._paused(): + return None assert self._reader is not None event = self._reader(self._receive_buffer) if event is None: diff --git a/h11/_events.py b/h11/_events.py index 406853c..d3570c6 100644 --- a/h11/_events.py +++ b/h11/_events.py @@ -16,7 +16,6 @@ "Data", "EndOfMessage", "ConnectionClosed", - "Paused", ] @@ -244,32 +243,3 @@ class ConnectionClosed(_EventBundle): No fields. """ pass - -class Paused(_EventBundle): - """A pseudo-event used for flow control. - - If :meth:`Connection.receive_data` returns this event, it means that the - HTTP parser is in a paused condition, and won't process any new data until - after the condition is resolved. See :ref:`flow-control` for details. - - .. attribute:: reason - - The remote peer's state that triggered the pause. One of: - - * :data:`h11.DONE`: a client has started sending another request before - we finished responding to their first request. Cleared by finishing - the response and then calling :meth:`Connection.prepare_to_reuse`. - - * :data:`MIGHT_SWITCH_PROTOCOL`: a client is in the - :data:`MIGHT_SWITCH_PROTOCOL` state, and is waiting for the server to - either accept or reject the proposed protocol switch. See - :ref:`switching-protocols` for details. - - * :data:`SWITCHED_PROTOCOL`: the remote peer is the - :data:`SWITCHED_PROTOCOL` state. h11 isn't going to parse any more - data that they send, because they're no longer speaking HTTP. See - :ref:`switching-protocols` for details. - - """ - - _fields = ["reason"] diff --git a/h11/tests/test_connection.py b/h11/tests/test_connection.py index c8163a9..3e2db09 100644 --- a/h11/tests/test_connection.py +++ b/h11/tests/test_connection.py @@ -341,25 +341,25 @@ def test_max_buffer_size_countermeasure(): # Exceeding the max buffer size is fine if we are paused c = Connection(SERVER, max_buffer_size=100) - # Two pipelined requests in a big big buffer + # Two pipelined requests to create a way-too-big receive buffer... but + # it's fine because we're not checking assert (c.receive_data(b"GET /1 HTTP/1.1\r\nHost: a\r\n\r\n" b"GET /2 HTTP/1.1\r\nHost: b\r\n\r\n" + b"X" * 1000) == [Request(method="GET", target="/1", headers=[("host", "a")]), - EndOfMessage(), - Paused(reason=DONE)]) - # Even more data comes in, no problem - assert c.receive_data(b"X" * 1000) + EndOfMessage()]) + # Even more data comes in, still no problem + c.receive_data(b"X" * 1000) # We can respond and reuse to get the second pipelined request c.send(Response(status_code=200, headers=[])) c.send(EndOfMessage()) c.prepare_to_reuse() assert (c.receive_data(None) == [Request(method="GET", target="/2", headers=[("host", "b")]), - EndOfMessage(), - Paused(reason=DONE)]) - # But once we unpause and try to read the next message, the buffer size is - # enforced again + EndOfMessage()]) + # But once we unpause and try to read the next message, and find that it's + # incomplete and the buffer is *still* way too large, then *that's* a + # problem: c.send(Response(status_code=200, headers=[])) c.send(EndOfMessage()) c.prepare_to_reuse() @@ -401,13 +401,11 @@ def test_pipelining(): headers=[("Host", "a.com"), ("Content-Length", "5")]), Data(data=b"12345"), EndOfMessage(), - Paused(reason=DONE), ] assert c.their_state is DONE assert c.our_state is SEND_RESPONSE - # Pause pseudo-events are re-emitted each time through: - assert c.receive_data(None) == [Paused(reason=DONE)] + assert c.receive_data(None) == [] c.send(Response(status_code=200, headers=[])) c.send(EndOfMessage()) @@ -422,7 +420,6 @@ def test_pipelining(): headers=[("Host", "a.com"), ("Content-Length", "5")]), Data(data=b"67890"), EndOfMessage(), - Paused(reason=DONE), ] c.send(Response(status_code=200, headers=[])) c.send(EndOfMessage()) @@ -440,12 +437,12 @@ def test_pipelining(): # Arrival of more data triggers pause assert c.receive_data(None) == [] - assert c.receive_data(b"SADF") == [Paused(reason=DONE)] + assert c.receive_data(b"SADF") == [] assert c.trailing_data == (b"SADF", False) - assert c.receive_data(b"") == [Paused(reason=DONE)] + assert c.receive_data(b"") == [] assert c.trailing_data == (b"SADF", True) - assert c.receive_data(None) == [Paused(reason=DONE)] - assert c.receive_data(b"") == [Paused(reason=DONE)] + assert c.receive_data(None) == [] + assert c.receive_data(b"") == [] # Can't call receive_data with non-empty buf after closing it with pytest.raises(RuntimeError): c.receive_data(b"FDSA") @@ -494,16 +491,10 @@ def setup(): # finish the request before that kicks in for conn in p.conns: assert conn.states[CLIENT] is SEND_BODY - p.send(CLIENT, - [Data(data=b"1"), EndOfMessage()], - expect=[Data(data=b"1"), - EndOfMessage(), - Paused(reason=MIGHT_SWITCH_PROTOCOL)]) + p.send(CLIENT, [Data(data=b"1"), EndOfMessage()]) for conn in p.conns: assert conn.states[CLIENT] is MIGHT_SWITCH_PROTOCOL - assert p.conn[SERVER].receive_data(None) == [ - Paused(reason=MIGHT_SWITCH_PROTOCOL), - ] + assert p.conn[SERVER].receive_data(None) == [] return p # Test deny case @@ -518,17 +509,12 @@ def setup(): # Test accept case p = setup() - p.send(SERVER, accept, - expect=[accept, Paused(reason=SWITCHED_PROTOCOL)]) + p.send(SERVER, accept) for conn in p.conns: assert conn.states == {CLIENT: SWITCHED_PROTOCOL, SERVER: SWITCHED_PROTOCOL} - assert conn.receive_data(b"123") == [ - Paused(reason=SWITCHED_PROTOCOL), - ] - assert conn.receive_data(b"456") == [ - Paused(reason=SWITCHED_PROTOCOL), - ] + assert conn.receive_data(b"123") == [] + assert conn.receive_data(b"456") == [] assert conn.trailing_data == (b"123456", False) # Pausing in might-switch, then recovery @@ -537,17 +523,11 @@ def setup(): # logic) p = setup() sc = p.conn[SERVER] - assert sc.receive_data(b"GET / HTTP/1.0\r\n\r\n") == [ - Paused(reason=MIGHT_SWITCH_PROTOCOL), - ] - assert sc.receive_data(None) == [ - Paused(reason=MIGHT_SWITCH_PROTOCOL), - ] + assert sc.receive_data(b"GET / HTTP/1.0\r\n\r\n") == [] + assert sc.receive_data(None) == [] assert sc.trailing_data == (b"GET / HTTP/1.0\r\n\r\n", False) sc.send(deny) - assert sc.receive_data(None) == [ - Paused(reason=DONE), - ] + assert sc.receive_data(None) == [] sc.send(EndOfMessage()) sc.prepare_to_reuse() assert sc.receive_data(None) == [ @@ -560,28 +540,17 @@ def setup(): # switched, we don't. p = setup() sc = p.conn[SERVER] - assert sc.receive_data(b"") == [ - Paused(reason=MIGHT_SWITCH_PROTOCOL), - ] - assert sc.receive_data(None) == [ - Paused(reason=MIGHT_SWITCH_PROTOCOL), - ] + assert sc.receive_data(b"") == [] + assert sc.receive_data(None) == [] assert sc.trailing_data == (b"", True) - p.send(SERVER, accept, - expect=[accept, Paused(reason=SWITCHED_PROTOCOL)]) - assert sc.receive_data(None) == [ - Paused(reason=SWITCHED_PROTOCOL), - ] + p.send(SERVER, accept) + assert sc.receive_data(None) == [] p = setup() sc = p.conn[SERVER] - assert sc.receive_data(b"") == [ - Paused(reason=MIGHT_SWITCH_PROTOCOL), - ] + assert sc.receive_data(b"") == [] sc.send(deny) - assert sc.receive_data(None) == [ - ConnectionClosed(), - ] + assert sc.receive_data(None) == [ConnectionClosed()] # You can't send after switching protocols, or while waiting for a # protocol switch @@ -590,8 +559,7 @@ def setup(): p.conn[CLIENT].send( Request(method="GET", target="/", headers=[("Host", "a")])) p = setup() - p.send(SERVER, accept, - expect=[accept, Paused(reason=SWITCHED_PROTOCOL)]) + p.send(SERVER, accept) with pytest.raises(LocalProtocolError): p.conn[SERVER].send(Data(data=b"123")) diff --git a/h11/tests/test_events.py b/h11/tests/test_events.py index 01447b1..c9a349a 100644 --- a/h11/tests/test_events.py +++ b/h11/tests/test_events.py @@ -99,6 +99,3 @@ def test_events(): cc = ConnectionClosed() assert repr(cc) == "ConnectionClosed()" - - paused = Paused(reason="some-reason") - assert paused.reason == "some-reason" From b12d31dfeb4c202f58509cdcaf22a22a1235d88f Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sun, 15 May 2016 23:26:12 -0700 Subject: [PATCH 4/9] WIP --- examples/curio-server.py | 147 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 examples/curio-server.py diff --git a/examples/curio-server.py b/examples/curio-server.py new file mode 100644 index 0000000..eba4198 --- /dev/null +++ b/examples/curio-server.py @@ -0,0 +1,147 @@ +# A simple HTTP server implemented using h11 and Curio: +# http://curio.readthedocs.org/ +# (so requires python 3.5+). +# +# All requests get echoed back a JSON document containing information about +# the request. + +import socket +from collections import deque +from wsgiref.handlers import format_date_time +import json + +import curio +import h11 + +MAX_RECV = 2 ** 16 +TIMEOUT = 10 + +class CurioServerTransport: + def __init__(self, sock): + self.sock = sock + self.conn = h11.Connection(h11.SERVER) + self.ident = " ".join([ + "h11-demo-curio-server/{}".format(h11.__version__), + h11.PRODUCT_ID, + ]).encode("ascii") + self.received = deque() + + async def send(self, event): + data = self.conn.send(event) + if data is None: + with self.sock.blocking() as real_sock: + # Curio bug: doesn't expose shutdown() + real_sock.shutdown(socket.SHUT_WR) + else: + await self.sock.sendall(data) + + def basic_headers(self): + return [("Server", self.ident), + ("Date", format_date_time(None).encode("ascii")), + ] + + async def receive_next_event(self): + while True: + if self.received: + return self.received.pop_left() + # In case we just got un-paused, check for new events before doing + # a blocking read + self.received.extend(self.conn.receive_data(None)) + if self.received: + return self.received.pop_left() + # And if the client is blocked waiting for 100 Continue, we better + # tell them to go ahead before we block waiting for them, or else + # we'll deadlock. + if self.conn.they_are_waiting_for_100_continue: + go_ahead = h11.InformationalResponse( + status_code=100, + headers=self.basic_headers()) + await self.send(go_ahead) + data = await self.sock.recv(MAX_RECV) + self.received.extend(self.conn.receive_data(data)) + +async def send_error_response(transport, exc): + if isinstance(exc, h11.RemoteProtocolError): + status_code = exc.error_status_hint + else: + status_code = 500 + body = str(exc).encode("utf-8") + headers = transport.basic_headers() + headers.append(("Content-Length", len(body))) + headers.append(("Content-Type", "text/plain; charset=utf-8")) + res = h11.Response(status_code=status_code, headers=headers) + await transport.send(res) + await transport.send(h11.Data(data=body)) + await transport.send(h11.EndOfMessage()) + +async def wait_for_shutdown(transport): + assert transport.our_state is h11.CLOSED + # Wait for a bit to give them a chance to see that we closed things, but + # eventually give up and just close the socket. + with curio.ignore_after(TIMEOUT): + while transport.their_state is not h11.CLOSED: + await transport.receive_next_event() + +def handle_next_request(transport): + while True: + request = await conn.receive_next_event() + if type(request) is h11.Paused: + continue + +def http_serve(sock, addr): + transport = CurioServerTransport(sock) + while True: + assert transport.conn.states == { + h11.CLIENT: h11.IDLE, h11.SERVER: h11.IDLE} + + + event = await conn.receive_next_event() + if type(event) is h11.Request: + try: + await respond(event, t) + except Exception as exc: + print("Oops:", exc) + try_send_error(transport, exc) + if transport.conn.our_state is h11.MUST_CLOSE: + await transport.send(h11.ConnectionClosed()) + if transport.conn.our_state is h11.CLOSED: + await wait_for_shutdown(transport) + await sock.close() + return + if transport.conn.states == { + h11.CLIENT: h11.DONE, h11.SERVER: h11.DONE}: + # Ready to re-use; loop back around + transport.conn.prepare_to_reuse() + else: + # something has gone wrong + try_send_error(RuntimeError( + "unexpected states {}".format(transport.conn.states))) + +async def send_echo_response(request, transport): + response_json = { + "method": request.method.decode("ascii"), + "target": request.target.decode("ascii"), + "headers": [(name.decode("ascii"), value.decode("ascii")) + for (name, value) in request.headers], + "body": "", + } + while True: + event = await transport.receive_next_event() + if type(event) is h11.EndOfMessage: + break + assert type(event) is h11.Data + response_json["body"] += event.data.decode("ascii") + response_body_unicode = json.dumps(response_json, + sort_keys=True, + indent=4, + separators=(",", ": ")) + response_body_bytes = response_body_unicode.encode("utf-8") + response_headers = transport.basic_headers() + response_headers += [ + ("Content-Length", len(response_body_bytes)), + ("Content-Type", "application/json; charset=utf-8"), + ] + await transport.send( + h11.Response(status_code=200, headers=response_headers)) + await transport.send(h11.Data(data=response_body_bytes)) + await transport.send(h11.EndOfMessage()) From 5671ed0ebb26ebe9eece8b58d6660a21236c013e Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Tue, 17 May 2016 17:59:37 -0700 Subject: [PATCH 5/9] Small doc text cleanups --- docs/source/api.rst | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/source/api.rst b/docs/source/api.rst index a35e8ad..d25051e 100644 --- a/docs/source/api.rst +++ b/docs/source/api.rst @@ -618,15 +618,15 @@ response, then stop reading until after you've sent another request. For a server, there are three more subtle points you need to keep track of. -**First**, you have to be prepared to handle :class:`Request`\s with `an -``Expect: 100-continue`` header -`_. You can read -the spec for details, but basically what this header means is that -after sending the :class:`Request`, the client plans to pause and wait -until they see some response from the server before sending any -:class:`Data` (or possibly some timeout occurs). The server's response -can be an :class:`InformationalResponse` with status ``100 Continue``, -or anything really (e.g. a full :class:`Response` with an error +**First**, you have to be prepared to handle :class:`Request`\s with +an ``Expect: 100-continue`` header. You can `read the spec +`_ for the full +details, but basically what this header means is that after sending +the :class:`Request`, the client plans to pause and wait until they +see some response from the server before sending any +:class:`Data`. The server's response can be an +:class:`InformationalResponse` with status ``100 Continue``, or +anything really (e.g. a full :class:`Response` with an error code). The crucial thing as a server, though, is that you should never block trying to read a request body if the client is blocked waiting for you to tell them to send the request body. @@ -666,7 +666,7 @@ in which case it will return :class:`ConnectionClosed`). This state also does another thing: normally, as an anti-denial-of-service protection, h11 will error out if its internal receive buffer gets too big; but when we are in this paused state, this protection is -disabled. (This is important because without it, we'd run the risk of +disabled. (This is important because otherwise we'd run the risk of treating a perfectly innocent client as if they were a DoS attacker, just because they sent a number of pipelined requests that all arrived at the same time.) From d19984f45dbf01278d5e84bf0d1a255cacd880ec Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Tue, 17 May 2016 18:00:10 -0700 Subject: [PATCH 6/9] First working curio-server commit! --- examples/curio-server.py | 158 +++++++++++++++++++++++++-------------- 1 file changed, 102 insertions(+), 56 deletions(-) diff --git a/examples/curio-server.py b/examples/curio-server.py index eba4198..1139020 100644 --- a/examples/curio-server.py +++ b/examples/curio-server.py @@ -9,6 +9,7 @@ from collections import deque from wsgiref.handlers import format_date_time import json +from itertools import count import curio import h11 @@ -17,6 +18,8 @@ TIMEOUT = 10 class CurioServerTransport: + _next_id = count() + def __init__(self, sock): self.sock = sock self.conn = h11.Connection(h11.SERVER) @@ -25,30 +28,32 @@ def __init__(self, sock): h11.PRODUCT_ID, ]).encode("ascii") self.received = deque() + self._obj_id = next(CurioServerTransport._next_id) async def send(self, event): + # The code below doesn't send ConnectionClosed, so we don't bother + # handling it here either -- it would require that we do something + # appropriate when 'data' is None. + assert type(event) is not h11.ConnectionClosed data = self.conn.send(event) - if data is None: - with self.sock.blocking() as real_sock: - # Curio bug: doesn't expose shutdown() - real_sock.shutdown(socket.SHUT_WR) - else: - await self.sock.sendall(data) + await self.sock.sendall(data) def basic_headers(self): - return [("Server", self.ident), - ("Date", format_date_time(None).encode("ascii")), - ] + # Required in all responses: + return [ + ("Date", format_date_time(None).encode("ascii")), + ("Server", self.ident), + ] async def receive_next_event(self): while True: if self.received: - return self.received.pop_left() + return self.received.popleft() # In case we just got un-paused, check for new events before doing # a blocking read self.received.extend(self.conn.receive_data(None)) if self.received: - return self.received.pop_left() + return self.received.popleft() # And if the client is blocked waiting for 100 Continue, we better # tell them to go ahead before we block waiting for them, or else # we'll deadlock. @@ -57,67 +62,108 @@ async def receive_next_event(self): status_code=100, headers=self.basic_headers()) await self.send(go_ahead) - data = await self.sock.recv(MAX_RECV) + try: + data = await self.sock.recv(MAX_RECV) + except ConnectionError: + # I don't know why BrokenPipeError, ConnectionResetError, + # etc. are different from remote EOF, but apparently they + # are. We just pretend the remote returned EOF. + data = b"" self.received.extend(self.conn.receive_data(data)) -async def send_error_response(transport, exc): - if isinstance(exc, h11.RemoteProtocolError): - status_code = exc.error_status_hint - else: - status_code = 500 - body = str(exc).encode("utf-8") + async def shutdown_and_clean_up(self): + # When this method is called, it's because we definitely want to kill + # this connection, either as a clean shutdown or because of some kind + # of error or loss-of-sync bug, and we don't care if that violates the + # protocol or not. So we ignore the state of self.conn, and just go + # ahead and do the shutdown on the socket directly. + # Curio bug: doesn't expose shutdown() + with self.sock.blocking() as real_sock: + try: + real_sock.shutdown(socket.SHUT_WR) + except OSError: + # Already closed, I guess + return + # Wait for a bit to give them a chance to see that we closed things, but + # eventually give up and just close the socket. + async with curio.ignore_after(TIMEOUT): + try: + while True: + # Attempt to read until EOF + got = await self.sock.recv(MAX_RECV) + if not got: + break + finally: + await self.sock.close() + + def info(self, *args): + print(self._obj_id, *args) + +async def send_simple_response(transport, status_code, content_type, body): + transport.info("Sending", status_code, + "response with", len(body), "bytes") headers = transport.basic_headers() - headers.append(("Content-Length", len(body))) - headers.append(("Content-Type", "text/plain; charset=utf-8")) + headers.append(("Content-Type", content_type)) + headers.append(("Content-Length", str(len(body)))) res = h11.Response(status_code=status_code, headers=headers) await transport.send(res) await transport.send(h11.Data(data=body)) await transport.send(h11.EndOfMessage()) -async def wait_for_shutdown(transport): - assert transport.our_state is h11.CLOSED - # Wait for a bit to give them a chance to see that we closed things, but - # eventually give up and just close the socket. - with curio.ignore_after(TIMEOUT): - while transport.their_state is not h11.CLOSED: - await transport.receive_next_event() - -def handle_next_request(transport): - while True: - request = await conn.receive_next_event() - if type(request) is h11.Paused: - continue +async def maybe_send_error_response(transport, exc): + # If we can't send an error, oh well, nothing to be done + transport.info("trying to send error response...") + if transport.conn.our_state not in {h11.IDLE, h11.SEND_RESPONSE}: + transport.info("...but I can't, because our state is", + transport.conn.our_state) + return + if isinstance(exc, h11.RemoteProtocolError): + status_code = exc.error_status_hint + else: + status_code = 500 + body = str(exc).encode("utf-8") + await send_simple_response(transport, + status_code, + "text/plain; charset=utf-8", + body) -def http_serve(sock, addr): +async def http_serve(sock, addr): transport = CurioServerTransport(sock) while True: assert transport.conn.states == { h11.CLIENT: h11.IDLE, h11.SERVER: h11.IDLE} - - - event = await conn.receive_next_event() + transport.info("Entering server main loop; waiting for event") + event = await transport.receive_next_event() + transport.info("Server main loop got event:", event) if type(event) is h11.Request: try: - await respond(event, t) + await send_echo_response(transport, event) except Exception as exc: - print("Oops:", exc) - try_send_error(transport, exc) + transport.info("Oops:", exc) + await maybe_send_error_response(transport, exc) if transport.conn.our_state is h11.MUST_CLOSE: - await transport.send(h11.ConnectionClosed()) - if transport.conn.our_state is h11.CLOSED: - await wait_for_shutdown(transport) - await sock.close() + transport.info("connection is not reusable, so shutting down") + await transport.shutdown_and_clean_up() return if transport.conn.states == { h11.CLIENT: h11.DONE, h11.SERVER: h11.DONE}: # Ready to re-use; loop back around + transport.info("connection reusable and all is well, so looping") transport.conn.prepare_to_reuse() else: # something has gone wrong - try_send_error(RuntimeError( - "unexpected states {}".format(transport.conn.states))) + transport.info("Response handler left us with unexpected state", + transport.conn.states, + "bailing out") + await maybe_send_error_response( + transport, + RuntimeError( + "unexpected states {}".format(transport.conn.states))) + await transport.shutdown_and_clean_up() + return -async def send_echo_response(request, transport): +async def send_echo_response(transport, request): + transport.info("Preparing echo response") response_json = { "method": request.method.decode("ascii"), "target": request.target.decode("ascii"), @@ -136,12 +182,12 @@ async def send_echo_response(request, transport): indent=4, separators=(",", ": ")) response_body_bytes = response_body_unicode.encode("utf-8") - response_headers = transport.basic_headers() - response_headers += [ - ("Content-Length", len(response_body_bytes)), - ("Content-Type", "application/json; charset=utf-8"), - ] - await transport.send( - h11.Response(status_code=200, headers=response_headers)) - await transport.send(h11.Data(data=response_body_bytes)) - await transport.send(h11.EndOfMessage()) + await send_simple_response(transport, + 200, + "application/json; charset=utf-8", + response_body_bytes) + +if __name__ == "__main__": + kernel = curio.Kernel() + print("Listening on http://localhost:8080") + kernel.run(curio.tcp_server("localhost", 8080, http_serve)) From 1d9cd6eb00e3de634e1ff426894cf78f40ac7e31 Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Tue, 17 May 2016 20:05:44 -0700 Subject: [PATCH 7/9] Split receive_data into receive_data + next_event So now receive_data no longer returns a list of events; instead, it just appends to the internal buffer, and then you have to call next_event to get it. (So next_event is like the old receive_data(None).) Maybe some other small changes snuck in, but that's the main one... this involved a major rework of the code and docs. The big advantage of this is that when trying to solve the really hard problem of implementing a HTTP server that handles errors gracefully, life is *much* simpler if you can take advantage of h11's knowledge of the connection state. But with the old way of doing receive_data, you might get back multiple events at once, and then while you're processing the first event, conn.states is already updated to reflect the future events you haven't processed yet. So you couldn't really use the state, because in general it'd be out-of-sync with your processing. In the new model, we return events on at a time, and we only update the state machine as we return them, so now the API follows causal law again. Down with spooky time travel! I'm also *much* happier with the flow control section of the docs, which now explicitly talks about how to use these primitives to implement both push-style and pull-style servers. Plus: super-awesome new example curio-server.py, which easily scales to thousands of requests/second, thousands of concurrent connections, and has robust error handling without spaghetti (!!). --- docs/source/_examples/myclient.py | 14 +- docs/source/api.rst | 403 +++++++++++++------------- docs/source/basic-usage.rst | 113 +++++--- docs/source/changes.rst | 13 +- examples/curio-server.py | 311 +++++++++++++------- examples/tiny-client-demo.py | 47 ++- h11/_connection.py | 212 +++++++------- h11/_receivebuffer.py | 4 +- h11/tests/helpers.py | 25 +- h11/tests/test_against_stdlib_http.py | 48 +-- h11/tests/test_connection.py | 199 +++++++------ 11 files changed, 795 insertions(+), 594 deletions(-) diff --git a/docs/source/_examples/myclient.py b/docs/source/_examples/myclient.py index 6ba1f57..2207a56 100644 --- a/docs/source/_examples/myclient.py +++ b/docs/source/_examples/myclient.py @@ -18,6 +18,14 @@ def send(self, *events): else: self.sock.sendall(data) - # max_bytes set intentionally small for pedagogical purposes - def receive(self, max_bytes=200): - return self.conn.receive_data(self.sock.recv(max_bytes)) + # max_bytes_per_recv intentionally set low for pedagogical purposes + def next_event(self, max_bytes_per_recv=200): + while True: + # If we already have a complete event buffered internally, just + # return that. Otherwise, read some data, add it to the internal + # buffer, and then try again. + event = self.conn.next_event() + if event is h11.NEED_DATA: + self.conn.receive_data(self.sock.recv(max_bytes_per_recv)) + continue + return event diff --git a/docs/source/api.rst b/docs/source/api.rst index d25051e..f6a9290 100644 --- a/docs/source/api.rst +++ b/docs/source/api.rst @@ -16,17 +16,18 @@ directly at the top level: @verbatim In [3]: h11. - h11.CLIENT h11.MIGHT_SWITCH_PROTOCOL - h11.CLOSED h11.MUST_CLOSE - h11.Connection h11.PRODUCT_ID - h11.ConnectionClosed h11.ProtocolError - h11.Data h11.RemoteProtocolError - h11.DONE h11.Request - h11.EndOfMessage h11.Response - h11.ERROR h11.SEND_BODY - h11.IDLE h11.SEND_RESPONSE - h11.InformationalResponse h11.SERVER - h11.LocalProtocolError h11.SWITCHED_PROTOCOL + h11.CLIENT h11.MUST_CLOSE + h11.CLOSED h11.NEED_DATA + h11.Connection h11.PAUSED + h11.ConnectionClosed h11.PRODUCT_ID + h11.Data h11.ProtocolError + h11.DONE h11.RemoteProtocolError + h11.EndOfMessage h11.Request + h11.ERROR h11.Response + h11.IDLE h11.SEND_BODY + h11.InformationalResponse h11.SEND_RESPONSE + h11.LocalProtocolError h11.SERVER + h11.MIGHT_SWITCH_PROTOCOL h11.SWITCHED_PROTOCOL These symbols fall into three main categories: event classes, special constants used to track different connection states, and the @@ -288,7 +289,14 @@ the code, while the labels in upright text correspond to actual objects in the public API. You've already seen the event objects like :class:`Request` and :class:`Response`; there are also a set of opaque sentinel values that you can use to track and query the client and -server's states: +server's states. + + +Special constants +----------------- + +h11 exposes some special constants corresponding to the different +states described above. .. data:: IDLE .. data:: SEND_RESPONSE @@ -326,23 +334,31 @@ And we can test these values directly using constants like :data:`SEND_BODY`: This shows how the :class:`Connection` type tracks these state machines and lets you query their current state. - -The Connection object ---------------------- - -There are two special constants used to indicate the two different -roles that a peer can play in an HTTP connection: +The above also showed the special constants that can be used to +indicate the two different roles that a peer can play in an HTTP +connection: .. data:: CLIENT .. data:: SERVER -When creating a :class:`Connection` object, you need to pass one of -these constants to indicate which side of the HTTP conversation you -want to implement: +And finally, there are also two special constants that can be returned +from :meth:`Connection.next_event`: + +.. data:: NEED_DATA +.. data:: PAUSED + +All of these behave the same, and their behavior is modeled after +:data:`None`: they're opaque singletons, their :meth:`__repr__` is +their name, and you compare them with ``is``. + + +The Connection object +--------------------- .. autoclass:: Connection .. automethod:: receive_data + .. automethod:: next_event .. automethod:: send .. automethod:: send_with_data_passthrough @@ -421,17 +437,18 @@ There are four cases where these exceptions might be raised: the :data:`DONE` state. The :class:`Connection` object remains usable, and you can try again later. -* When calling :meth:`Connection.receive_data` +* When calling :meth:`Connection.next_event` (:exc:`RemoteProtocolError`): This indicates that the remote peer has violated our protocol assumptions. This is unrecoverable -- we don't know what they're doing and we cannot safely proceed. :attr:`Connection.their_state` immediately becomes :data:`ERROR`, and all further calls to - :meth:`~Connection.receive_data` will also raise - :exc:`ProtocolError`. :meth:`Connection.send` still works as normal, - so if you're implementing a server and this happens then you have an - opportunity to send back a 400 Bad Request response. Your only other - real option is to close your socket and make a new connection. + :meth:`~Connection.next_event` will also raise + :exc:`RemoteProtocolError`. :meth:`Connection.send` still works as + normal, so if you're implementing a server and this happens then you + have an opportunity to send back a 400 Bad Request response. But + aside from that, your only real option is to close your socket and + make a new connection. * When calling :meth:`Connection.send` or :meth:`Connection.send_with_data_passthrough` @@ -440,9 +457,9 @@ There are four cases where these exceptions might be raised: what you're doing, its internal state may be inconsistent, and we cannot safely proceed. :attr:`Connection.our_state` immediately becomes :data:`ERROR`, and all further calls to - :meth:`~Connection.send` will also raise :exc:`ProtocolError`. The - only thing you can reasonably due at this point is to close your - socket and make a new connection. + :meth:`~Connection.send` will also raise + :exc:`LocalProtocolError`. The only thing you can reasonably due at + this point is to close your socket and make a new connection. .. _framing: @@ -471,19 +488,19 @@ On requests, this means: * ``Transfer-Encoding: chunked``: You're going to send a variable / not yet known number of bytes. - Note 1: only HTTP/1.1 servers are required to supported - ``Transfer-Encoding: chunked``, and as a client you have to either - send this header or not before you get to see what protocol version - the server is using. + Note 1: only HTTP/1.1 servers are required to support + ``Transfer-Encoding: chunked``, and as a client you have to decide + whether to send this header before you get to see what protocol + version the server is using. Note 2: even though HTTP/1.1 servers are required to support - ``Transfer-Encoding: chunked``, this doesn't mean that they actually - do -- e.g., applications using Python's standard WSGI API cannot - accept chunked requests. + ``Transfer-Encoding: chunked``, this doesn't necessarily mean that + they actually do -- e.g., applications using Python's standard WSGI + API cannot accept chunked requests. Nonetheless, this is the only way to send request where you don't - know the size of the body ahead of time, so you might as well go - ahead and hope. + know the size of the body ahead of time, so if that's the situation + you find yourself in then you might as well try it and hope. On responses, things are a bit more subtle. There are effectively two cases: @@ -501,7 +518,7 @@ cases: keep-alive, allows trailing headers, clearly distinguishes between successful completion and network errors), but requires an HTTP/1.1 client; for HTTP/1.0 clients the only option is the no-headers - close-socket-to-indicate-completion approach. + approach where you have to close the socket to indicate completion. Since this is (almost) entirely a wire-level-encoding concern, h11 abstracts it: when sending a response you can set either @@ -559,10 +576,10 @@ explicitly call :meth:`Connection.prepare_to_reuse` to reset both sides back to the :data:`IDLE` state. This makes sure that the client and server remain synched up. -If keep-alive is disabled for whatever reason -- explicit headers, -lack of protocol support, one of the sides just unilaterally closed -the connection -- then the state machines will skip past the -:data:`DONE` state directly to the :data:`MUST_CLOSE` or +If keep-alive is disabled for whatever reason -- someone set +``Connection: close``, lack of protocol support, one of the sides just +unilaterally closed the connection -- then the state machines will +skip past the :data:`DONE` state directly to the :data:`MUST_CLOSE` or :data:`CLOSED` states. In this case, trying to call :meth:`~Connection.prepare_to_use` will raise an error, and the only thing you can legally do is to close this connection and make a new @@ -585,7 +602,7 @@ response. As a server, h11 provides the minimal support for pipelining required to comply with the HTTP/1.1 standard: if the client sends multiple pipelined requests, then we the first request until we reach the -:data:`DONE` state, and then :meth:`~Connection.receive_data` will +:data:`DONE` state, and then :meth:`~Connection.next_event` will pause and refuse to parse any more events until the response is completed and :meth:`~Connection.prepare_to_reuse` is called. See the next section for more details. @@ -596,43 +613,33 @@ next section for more details. Flow control ------------ -h11 always does the absolute minimum of buffering that it can manage: -:meth:`~Connection.send` and -:meth:`~Connection.send_with_data_passthrough` always return the full -data to send immediately, and :meth:`~Connection.recieve_data` always -greedily parses and returns as many events as possible from its -current buffer (with one exception discussed below). So you can be -sure that no data or events will suddenly appear and need processing, -except when you call these methods. And presumably you know when you -want to send things. But there is one thing you still need to know, -which is when it's safe to read from the remote peer: you don't want -to read data from the remote peer if it can't be processed (i.e., you -want to apply backpressure and avoid building arbitrarily large -in-memory buffers), and you definitely don't want to block waiting on -data from the remote peer at the same time that it's blocked waiting -for you, because that will cause a deadlock. - -For a client, this is simple: send your request, then read the -response, then stop reading until after you've sent another request. - -For a server, there are three more subtle points you need to keep -track of. - -**First**, you have to be prepared to handle :class:`Request`\s with -an ``Expect: 100-continue`` header. You can `read the spec +Presumably you know when you want to send things, and the +:meth:`~Connection.send` interface is very simple: it just immediately +returns all the data you need to send for the given event, so you can +apply whatever send buffer strategy you want. But reading from the +remote peer is a bit trickier: you don't want to read data from the +remote peer if it can't be processed (i.e., you want to apply +backpressure and avoid building arbitrarily large in-memory buffers), +and you definitely don't want to block waiting on data from the remote +peer at the same time that it's blocked waiting for you, because that +will cause a deadlock. + +One complication here is that if you're implementing a server, you +have to be prepared to handle :class:`Request`\s that have an +``Expect: 100-continue`` header. You can `read the spec `_ for the full details, but basically what this header means is that after sending the :class:`Request`, the client plans to pause and wait until they -see some response from the server before sending any -:class:`Data`. The server's response can be an -:class:`InformationalResponse` with status ``100 Continue``, or -anything really (e.g. a full :class:`Response` with an error -code). The crucial thing as a server, though, is that you should never -block trying to read a request body if the client is blocked waiting -for you to tell them to send the request body. +see some response from the server before they send that request's +:class:`Data`. The server's response would normally be an +:class:`InformationalResponse` with status ``100 Continue``, but it +could be anything really (e.g. a full :class:`Response` with a 4xx +status code). The crucial thing as a server, though, is that you +should never block trying to read a request body if the client is +blocked waiting for you to tell them to send the request body. Fortunately, h11 makes this easy, because it tracks whether the client -is blocked, and exposes this as +is in the waiting-for-100-continue state, and exposes this as :attr:`Connection.they_are_waiting_for_100_continue`. So you don't have to pay attention to the ``Expect`` header yourself; you just have to make sure that before you block waiting to read a request body, you @@ -648,141 +655,121 @@ In fact, if you're lazy (and what programmer isn't?) then you can just do this check before all reads -- it's mandatory before blocking to read a request body, but it's safe at any time. -**Second**, if you're using some sort of I/O setup where you read all -the time (e.g. in a dedicated thread or some sort of proactor), then -you should stop reading when you get each request's -:class:`EndOfMessage`, and don't start again until after you call -:meth:`~Connection.prepare_to_reuse`. (Of course you should also -follow this rule if you're using blocking I/O, but hopefully that is -obvious -- what's not so obvious is that this applies also to threaded -reads.) If you ignore this rule, then nothing particularly bad will -happen -- but nothing useful will happen, either. After seeing the -:class:`EndOfMessage`, then :meth:`~Connection.receive_data` will -enter a "paused" state: you can call it, and it'll add any data that -you give it to its internal buffer, but it won't actually attempt to -parse that buffer or return any events (unless the next data you give -it is ``b""`` indicating that the client has closed their connection, -in which case it will return :class:`ConnectionClosed`). This state -also does another thing: normally, as an anti-denial-of-service -protection, h11 will error out if its internal receive buffer gets too -big; but when we are in this paused state, this protection is -disabled. (This is important because otherwise we'd run the risk of -treating a perfectly innocent client as if they were a DoS attacker, -just because they sent a number of pipelined requests that all arrived -at the same time.) - -So really it's best if you just stop reading from the remote socket -during this period. If you're lucky this might even provide some -back-pressure to the remote client to stop sending stuff so fast. - -(Note: There isn't currently any API exposed to tell you whether -receiving is paused; you're expected to "just know" based on what -you've recently done. So far this seems to work fine for the servers -I've written, but if you'd find this useful then please let us know -- -it wouldn't be hard to add.) - -And **third**, coming out of the paused state requires a bit of -care. After you call :meth:`~Connection.prepare_to_reuse`, you -*must not* immediately start reading from the network. Consider the -situation where a client sends two pipelined requests, and then blocks -waiting for the two responses. It's possible the two requests will -arrive together, and be enqueued into our receive buffer together: - -.. ipython:: python - - conn = h11.Connection(our_role=h11.SERVER) - conn.receive_data( - b"GET /1 HTTP/1.1\r\nHost: example.com\r\n\r\n" - b"GET /1 HTTP/1.1\r\nHost: example.com\r\n\r\n" - ) - -Notice how we get back only the first :class:`Request` and its (empty) -body. - -We process the first request: - -.. ipython:: python - - conn.send(h11.Response(status_code=200, headers=[])) - conn.send(h11.EndOfMessage()) - -And then reset the connection to handle the next: - -.. ipython:: python - - conn.prepare_to_reuse() - -This has unpaused our receive buffer, so now we're ready to read more -data from the network right? Well, no-- the client is done sending -data, we already have all their data, so if we block waiting for more -data now, then we'll be waiting forever. - -That would be bad. +And the other thing you want to pay attention to is the special values +that :meth:`~Connection.next_event` might return: :data:`NEED_DATA` +and :data:`PAUSED`. + +:data:`NEED_DATA` is what it sounds like: it means that +:meth:`~Connection.next_event` is guaranteed not to return any more +real events until you've called :meth:`~Connection.receive_data` at +least once. + +:data:`PAUSED` is a little more subtle: it means that +:meth:`~Connection.next_event` is guaranteed not to return any more +real events until something else has happened to clear up the paused +state. There are three cases where this can happen: + +1) We received a full request/response from the remote peer, and then + we received some more data after that. (The main situation where + this might happen is a server responding to a pipelining client.) + The :data:`PAUSED` state will go away after you call + :meth:`~Connection.prepare_to_reuse`. + +2) A successful ``CONNECT`` or ``Upgrade:`` request has caused the + connection to switch to some other protocol (see + :ref:`switching-protocols`). This :data:`PAUSED` state is + permanent; you should abandon this :class:`Connection` and go do + whatever it is you're going to do with your new protocol. + +3) We're a server, and the client we're talking to proposed to switch + protocols (see :ref:`switching-protocols`), and now is waiting to + find out whether their request was successful or not. Once we + either accept or deny their request then this will turn into one of + the above two states, so you probably don't need to worry about + handling it specially. + +Putting all this together -- + +If your I/O is organized around a "pull" strategy, where your code +requests events as its ready to handle them (e.g. classic synchronous +code or `Curio `_), then you'll +probably want logic that looks something like: -Instead, what we have to do after unpausing is make an explicit call -to :meth:`~Connection.receive_data` with ``None`` as the argument, -which means "I don't have any more data for you, but could you check -the data you already have buffered in case there's anything else you -can parse now that you couldn't before?". And once we've done this and -processed the events we get back, we can continue as normal: +.. code-block:: python -.. ipython:: python + # Replace do_sendall and do_recv with your I/O code + def get_next_event(): + while True: + event = conn.next_event() + if event is h11.NEED_DATA: + if conn.they_are_waiting_for_100_continue: + do_sendall(conn, h11.InformationalResponse(100, ...)) + conn.receive_data(do_recv()) + continue + return event + +And then your code that calls this will need to make sure to call it +only at appropriate times (e.g., not immediately after receiving +:class:`EndOfMessage` or :data:`PAUSED`). + +If your I/O is organized around a "push" strategy, where the network +drives processing (e.g. you're using `Twisted +`_, or implementing an +:class:`asyncio.Protocol`), then you'll want to internally apply +back-pressure whenever you see :data:`PAUSED`, remove back-pressure +when you call :meth:`~Connection.prepare_to_reuse`, and otherwise just +deliver events as they arrive. Something like: - conn.receive_data(None) +.. code-block:: python -It is always safe to call ``conn.receive_data(None)``; if there aren't -any new events to return, it will simply return ``[]``. If you want to -be conservative, you can defensively call this immediately before -issuing any blocking read. - - -Technical details on how the receive_data pause state is implemented -.................................................................... - -You probably don't need to know this to write a client or server, but -perhaps you're curious. - -Internally, the pause logic doesn't distinguish between whether we're -implementing a client or a server, and there are actually three -possible reasons we might enter the paused state: - -* The remote peer is in the :data:`DONE` state, but sent more data, - i.e., a client is attempting to :ref:`pipeline requests - `. In the :data:`DONE` state, - :meth:`~Connection.receive_data` can return :class:`ConnectionClosed` - events, but if any actual data is received then it will pause, and - stay that way until a successful call to - :meth:`~Connection.prepare_to_reuse`. - -* The remote client is in the :data:`MIGHT_SWITCH_PROTOCOL` state (see - :ref:`switching-protocols`). This really shouldn't happen, because - they don't know yet whether the protocol switch will actually happen, - but OTOH it certainly isn't correct for us to go ahead and parse the - data they sent as if it were HTTP, when it might not be. So if this - happens, we pause. - -* The remote peer is in the :data:`SWITCHED_PROTOCOL` state (see - :ref:`switching-protocols`). When this happens we certainly - shouldn't try to parse their data. So if this happens we pause. - -How does this relate to the advice given above? - -You don't really need to worry about the :data:`MIGHT_SWITCH_PROTOCOL` -case, because the only way a client can get into this state is by -entering :data:`DONE`, and the only way it can get out is by entering -:data:`DONE` or :data:`SWITCHED_PROTOCOL`. So if you handle those -cases correctly then you'll automatically handle this case too. And -handling of :data:`SWITCHED_PROTOCOL` should be obvious, because if -you've just negotiated a protocol switch you hopefully know that and -won't try to keep using the :class:`Connection` object, and this state -is permanent. So the only case that requires careful handling by your -flow control logic is :data:`DONE`. And if you're implementing a -client, even this isn't something you need to worry about, because it -would be very weird for a server that was in the :data:`DONE` state to -suddenly send another response in a row out-of-the-blue, before you've -sent another request, so the simple rule of "only read when waiting -for a response" covers this case as well. So really it's only servers -that need special flow-control logic to handle pipelining. + class HTTPProtocol(asyncio.Protocol): + # Save the transport for later -- needed to access the + # backpressure API. + def connection_made(self, transport): + self._transport = transport + + # Internal helper function -- deliver all pending events + def _deliver_events(self): + while True: + event = self.conn.next_event() + if event is h11.NEED_DATA: + break + elif event is h11.PAUSED: + # Apply back-pressure + self._transport.pause_reading() + break + else: + self.event_received(event) + + # Called by "someone" whenever new data appears on our socket + def data_received(self, data): + self.conn.receive_data(data) + self._deliver_events() + + # Called by "someone" whenever the peer closes their socket + def eof_received(self): + self.conn.receive_data(b"") + self._deliver_events() + # asyncio will close our socket unless we return True here. + return True + + # Called by your code when its ready to start a new + # request/response cycle + def prepare_to_reuse(self): + self.conn.prepare_to_reuse() + # New events might have been buffered internally, and only + # become deliverable after calling prepare_to_reuse + self._deliver_events() + # Remove back-pressure + self._transport.resume_reading() + + # Fill in your code here + def event_received(self, event): + ... + +And your code that uses this will have to remember to check for +:attr:`~Connection.they_are_waiting_for_100_continue` at the +appropriate time. .. _closing: diff --git a/docs/source/basic-usage.rst b/docs/source/basic-usage.rst index 1f01e30..9c0c008 100644 --- a/docs/source/basic-usage.rst +++ b/docs/source/basic-usage.rst @@ -135,15 +135,18 @@ what that actually means in any given situation, so that you don't have to. Finally, we have to read the server's reply. By now you can probably -guess how this is done: we read some bytes from the network, then we -hand them to :meth:`Connection.receive_data` and it gives us back -high-level events from the server. +guess how this is done, at least in the general outline: we read some +bytes from the network, then we hand them to the connection (using +:meth:`Connection.receive_data`) and it converts them into events +(using :meth:`Connection.next_event`). .. ipython:: python bytes_received = sock.recv(1024) - events_received = conn.receive_data(bytes_received) - events_received + conn.receive_data(bytes_received) + conn.next_event() + conn.next_event() + conn.next_event() Here the server sent us three events: a :class:`Response` object, which is similar to the :class:`Request` object that we created @@ -155,13 +158,54 @@ server, then these are the objects we would have created and passed to :meth:`~Connection.send` -- h11 in client and server mode has an API that's almost exactly symmetric. +One thing we have to deal with, though, is that an entire response +doesn't always arrive in a single call to :meth:`socket.recv` -- +sometimes the network will decide to trickle it in at its own pace, in +multiple pieces. Let's try that again: + +.. ipython:: python + + import ssl, socket + import h11 + + ctx = ssl.create_default_context() + sock = ctx.wrap_socket(socket.create_connection(("httpbin.org", 443)), + server_hostname="httpbin.org") + + conn = h11.Connection(our_role=h11.CLIENT) + request = h11.Request(method="GET", + target="/xml", + headers=[("Host", "httpbin.org")]) + sock.sendall(conn.send(request)) + +and this time, we'll read in chunks of 200 bytes, to see how h11 +handles it: + +.. ipython:: python + + bytes_received = sock.recv(200) + conn.receive_data(bytes_received) + conn.next_event() + +:data:`NEED_DATA` is a special value that indicates that we, well, +need more data. h11 has buffered the first chunk of data; let's read +some more: + +.. ipython:: python + + bytes_received = sock.recv(200) + conn.receive_data(bytes_received) + conn.next_event() + +Now it's managed to read a complete :class:`Request`. + A basic client object --------------------- -To make this a little more convenient to play with, we can wrap up our -socket and :class:`Connection` into a single object with some -convenience methods: +Now let's use what we've learned to wrap up our socket and +:class:`Connection` into a single object with some convenience +methods: .. literalinclude:: _examples/myclient.py @@ -186,34 +230,26 @@ And read back the events: .. ipython:: python - client.receive() - -What happened here? We only read a max of 200 bytes from the socket -(see ``max_bytes=`` above), and it turns out that this wasn't enough -to form a complete event. This happens all the time in real life, due -to slow networks or whatever -- data trickles in at its own pace. When -this happens, h11 buffers the unprocessed data internally, and if you -keep reading then eventually you'll get a complete event: - -.. ipython:: python - - client.receive() + client.next_event() + client.next_event() Note here that we received a :class:`Data` event that only has *part* -of the response body -- h11 streams out data as it arrives, which -might mean that you receive multiple :class:`Data` events. (Of course, -if you're the one sending data, you can do the same thing: instead of -buffering all your data in one giant :class:`Data` event, you can send -multiple :class:`Data` events yourself to stream the data out -incrementally; just make sure that you set the appropriate -``Content-Length`` / ``Transfer-Encoding`` headers.) If we keep -reading, we'll see more :class:`Data` events, and then eventually the -:class:`EndOfMessage`: +of the response body -- this is another consequence of our reading in +small chunks. h11 tries to buffer as little as it can, so it streams +out data as it arrives, which might mean that a message body might be +split up into multiple :class:`Data` events. (Of course, if you're the +one sending data, you can do the same thing: instead of buffering all +your data in one giant :class:`Data` event, you can send multiple +:class:`Data` events yourself to stream the data out incrementally; +just make sure that you set the appropriate ``Content-Length`` / +``Transfer-Encoding`` headers.) If we keep reading, we'll see more +:class:`Data` events, and then eventually the :class:`EndOfMessage`: .. ipython:: python - client.receive() - client.receive() + client.next_event() + client.next_event() + client.next_event() Now we can see why :class:`EndOfMessage` is so important -- otherwise, we can't tell when we've received the end of the data. And since @@ -225,7 +261,7 @@ will just hang forever, unless we set a timeout or interrupt it: :okexcept: client.sock.settimeout(2) - client.receive() + client.next_event() Keep-alive @@ -239,8 +275,8 @@ can re-use this connection to send another request. There's a few ways we can tell. First, if it didn't, then it would have closed the connection already, and we would have gotten a :class:`ConnectionClosed` event on our last call to -:meth:`receive`. We can also tell by checking h11's internal idea of -what state the two sides of the conversation are in: +:meth:`~Connection.next_event`. We can also tell by checking h11's +internal idea of what state the two sides of the conversation are in: .. ipython:: python @@ -272,7 +308,7 @@ allowing us to send another :class:`Request`: client.send(h11.Request(method="GET", target="/get", headers=[("Host", "httpbin.org")]), h11.EndOfMessage()) - client.receive(max_bytes=4096) + client.next_event() What's next? @@ -291,7 +327,6 @@ Here's some ideas of things you might try: ("Content-Length", "10")]), h11.Data(data=b"1234567890"), h11.EndOfMessage()) - client.receive(max_bytes=4096) * Experiment with what happens if you try to violate the HTTP protocol by sending a :class:`Response` as a client, or sending two @@ -304,9 +339,9 @@ Here's some ideas of things you might try: * Adapt the above code to use your favorite non-blocking API -* Use h11 to write a simple HTTP server. (If you get stuck, there's - `an example in the test suite - `_.) +* Use h11 to write a simple HTTP server. (If you get stuck, `here's an + example + `_.) And of course, you'll want to read the :ref:`API-documentation` for all the details. diff --git a/docs/source/changes.rst b/docs/source/changes.rst index d215208..0105e61 100644 --- a/docs/source/changes.rst +++ b/docs/source/changes.rst @@ -6,15 +6,22 @@ History of changes vNEXT (????-??-??) ------------------ +This is the first release since we started using h11 to write +non-trivial server code, and this experience triggered a number of +substantial API changes. + Backwards **in**\compatible changes: +* Split the old :meth:`receive_data` into the new + :meth:`~Connection.receive_data` and + :meth:`~Connection.next_event`, and replaced the old :class:`Paused` + pseudo-event with the new :data:`NEED_DATA` and :data:`PAUSED` + sentinels. + * Simplified the API by replacing the old :meth:`Connection.state_of`, :attr:`Connection.client_state`, :attr:`Connection.server_state` with the new :attr:`Connection.states`. -* Removed the :class:`Paused` pseudo-event -- see :ref:`flow-control` - for the new way things work. - Backwards compatible changes: * State machine: added a :data:`DONE` -> :data:`MUST_CLOSE` transition diff --git a/examples/curio-server.py b/examples/curio-server.py index 1139020..1a2b135 100644 --- a/examples/curio-server.py +++ b/examples/curio-server.py @@ -4,12 +4,62 @@ # # All requests get echoed back a JSON document containing information about # the request. +# +# This is a rather involved example, since it attempts to both be +# fully-HTTP-compliant and also demonstrate error handling. +# +# The main difference between an HTTP client and an HTTP server is that in a +# client, if something goes wrong, you can just throw away that connection and +# make a new one. In a server, you're expected to handle all kinds of garbage +# input and internal errors and recover with grace and dignity. And that's +# what this code does. +# +# I recommend pushing on it to see how it works -- e.g. watch what happens if +# you visit http://localhost:8080 in a webbrowser that supports keep-alive, +# hit reload a few times, and then wait for the keep-alive to time out on the +# server. +# +# Or try using curl to start a chunked upload and then hit control-C in the +# middle of the upload: +# +# (for CHUNK in $(seq 10); do echo $CHUNK; sleep 1; done) \ +# | curl -T - http://localhost:8080/foo +# +# (Note that curl will send Expect: 100-Continue, too.) +# +# Or, heck, try letting curl complete successfully ;-). + +# Some potential improvements, if you wanted to try and extend this to a real +# general-purpose HTTP server: +# +# We should probably do something cleverer with buffering responses and +# TCP_CORK and suchlike. +# +# The timeout handling is rather crude -- we impose a flat 10 second timeout +# on each request (starting from the end of the previous response). Something +# cleverer would be better. Also, if a timeout is triggered we unconditionally +# send a 500 Internal Server Error; it would be better to keep track of +# whether the timeout is the client's fault, and if so send a 408 Request +# Timeout. +# +# The error handling policy here is somewhat crude as well. It handles a lot +# of cases perfectly, but there are corner cases where the ideal behavior is +# more debateable. For example, if a client starts uploading a large request, +# uses 100-Continue, and we send an error response, then we'll shut down the +# connection immediately (for well-behaved clients) or after spending TIMEOUT +# seconds reading and discarding their upload (for ill-behaved ones that go on +# and try to upload their request anyway). And for clients that do this +# without 100-Continue, we'll send the error response and then shut them down +# after TIMEOUT seconds. This might or might not be your preferred policy, +# though -- maybe you want to shut such clients down immediately (even if this +# risks their not seeing the response), or maybe you're happy to let them +# continue sending all the data and wasting your bandwidth if this is what it +# takes to guarantee that they see your error response. Up to you, really. -import socket -from collections import deque -from wsgiref.handlers import format_date_time import json from itertools import count +from socket import SHUT_WR +from wsgiref.handlers import format_date_time import curio import h11 @@ -17,18 +67,26 @@ MAX_RECV = 2 ** 16 TIMEOUT = 10 -class CurioServerTransport: +################################################################ +# I/O adapter +################################################################ + +# The core of this would work just as well for a curio-based HTTP client. +class CurioHTTPWrapper: _next_id = count() def __init__(self, sock): self.sock = sock self.conn = h11.Connection(h11.SERVER) + # Our Server: header self.ident = " ".join([ - "h11-demo-curio-server/{}".format(h11.__version__), + "h11-example-curio-server/{}".format(h11.__version__), h11.PRODUCT_ID, ]).encode("ascii") - self.received = deque() - self._obj_id = next(CurioServerTransport._next_id) + # A unique id for this connection, to include in debugging output + # (useful for understanding what's going on if there are multiple + # simultaneous clients). + self._obj_id = next(CurioHTTPWrapper._next_id) async def send(self, event): # The code below doesn't send ConnectionClosed, so we don't bother @@ -38,54 +96,46 @@ async def send(self, event): data = self.conn.send(event) await self.sock.sendall(data) - def basic_headers(self): - # Required in all responses: - return [ - ("Date", format_date_time(None).encode("ascii")), - ("Server", self.ident), - ] + async def _read_from_peer(self): + if self.conn.they_are_waiting_for_100_continue: + self.info("Sending 100 Continue") + go_ahead = h11.InformationalResponse( + status_code=100, + headers=self.basic_headers()) + await self.send(go_ahead) + try: + data = await self.sock.recv(MAX_RECV) + except ConnectionError: + # They've stopped listening. Not much we can do about it here. + data = b"" + self.conn.receive_data(data) - async def receive_next_event(self): + async def next_event(self): while True: - if self.received: - return self.received.popleft() - # In case we just got un-paused, check for new events before doing - # a blocking read - self.received.extend(self.conn.receive_data(None)) - if self.received: - return self.received.popleft() - # And if the client is blocked waiting for 100 Continue, we better - # tell them to go ahead before we block waiting for them, or else - # we'll deadlock. - if self.conn.they_are_waiting_for_100_continue: - go_ahead = h11.InformationalResponse( - status_code=100, - headers=self.basic_headers()) - await self.send(go_ahead) - try: - data = await self.sock.recv(MAX_RECV) - except ConnectionError: - # I don't know why BrokenPipeError, ConnectionResetError, - # etc. are different from remote EOF, but apparently they - # are. We just pretend the remote returned EOF. - data = b"" - self.received.extend(self.conn.receive_data(data)) + event = self.conn.next_event() + if event is h11.NEED_DATA: + await self._read_from_peer() + continue + return event async def shutdown_and_clean_up(self): # When this method is called, it's because we definitely want to kill # this connection, either as a clean shutdown or because of some kind - # of error or loss-of-sync bug, and we don't care if that violates the - # protocol or not. So we ignore the state of self.conn, and just go - # ahead and do the shutdown on the socket directly. + # of error or loss-of-sync bug, and we no longer care if that violates + # the protocol or not. So we ignore the state of self.conn, and just + # go ahead and do the shutdown on the socket directly. (If you're + # implementing a client you might prefer to send ConnectionClosed() + # and let it raise an exception if that violates the protocol.) + # # Curio bug: doesn't expose shutdown() with self.sock.blocking() as real_sock: try: - real_sock.shutdown(socket.SHUT_WR) + real_sock.shutdown(SHUT_WR) except OSError: - # Already closed, I guess + # They're already gone, nothing to do return - # Wait for a bit to give them a chance to see that we closed things, but - # eventually give up and just close the socket. + # Wait and read for a bit to give them a chance to see that we closed + # things, but eventually give up and just close the socket. async with curio.ignore_after(TIMEOUT): try: while True: @@ -96,74 +146,123 @@ async def shutdown_and_clean_up(self): finally: await self.sock.close() + def basic_headers(self): + # HTTP requires these headers in all responses (client would do + # something different here) + return [ + ("Date", format_date_time(None).encode("ascii")), + ("Server", self.ident), + ] + def info(self, *args): - print(self._obj_id, *args) + # Little debugging method + print("{}:".format(self._obj_id), *args) + +################################################################ +# Server main loop +################################################################ + +# General theory: +# +# If everything goes well: +# - we'll get a Request +# - our response handler will read the request body and send a full response +# - that will either leave us in MUST_CLOSE (if the client doesn't +# support keepalive) or DONE/DONE (if the client does). +# +# But then there are many, many different ways that things can go wrong +# here. For example: +# - we don't actually get a Request, but rather a ConnectionClosed +# - exception is raised from somewhere (naughty client, broken +# response handler, whatever) +# - depending on what went wrong and where, we might or might not be +# able to send an error response, and the connection might or +# might not be salvagable after that +# - response handler doesn't fully read the request or doesn't send a +# full response +# +# But these all have one thing in common: they involve us leaving the +# nice easy path up above. So we can just proceed on the assumption +# that the nice easy thing is what's happening, and whenever something +# goes wrong do our best to get back onto that path, and h11 will keep +# track of how successful we were and raise new errors if things don't work +# out. +async def http_serve(sock, addr): + wrapper = CurioHTTPWrapper(sock) + while True: + assert wrapper.conn.states == { + h11.CLIENT: h11.IDLE, h11.SERVER: h11.IDLE} + + try: + async with curio.timeout_after(TIMEOUT): + wrapper.info("Server main loop waiting for request") + event = await wrapper.next_event() + wrapper.info("Server main loop got event:", event) + if type(event) is h11.Request: + await send_echo_response(wrapper, event) + except Exception as exc: + wrapper.info("Error during response handler:", exc) + await maybe_send_error_response(wrapper, exc) + + if wrapper.conn.our_state is h11.MUST_CLOSE: + wrapper.info("connection is not reusable, so shutting down") + await wrapper.shutdown_and_clean_up() + return + else: + try: + wrapper.info("trying to re-use connection") + wrapper.conn.prepare_to_reuse() + except h11.ProtocolError: + states = wrapper.conn.states + wrapper.info("unexpected state", states, "-- bailing out") + await maybe_send_error_response( + wrapper, + RuntimeError("unexpected state {}".format(states))) + await wrapper.shutdown_and_clean_up() + return -async def send_simple_response(transport, status_code, content_type, body): - transport.info("Sending", status_code, - "response with", len(body), "bytes") - headers = transport.basic_headers() +################################################################ +# Actual response handlers +################################################################ + +# Helper function +async def send_simple_response(wrapper, status_code, content_type, body): + wrapper.info("Sending", status_code, + "response with", len(body), "bytes") + headers = wrapper.basic_headers() headers.append(("Content-Type", content_type)) headers.append(("Content-Length", str(len(body)))) res = h11.Response(status_code=status_code, headers=headers) - await transport.send(res) - await transport.send(h11.Data(data=body)) - await transport.send(h11.EndOfMessage()) + await wrapper.send(res) + await wrapper.send(h11.Data(data=body)) + await wrapper.send(h11.EndOfMessage()) -async def maybe_send_error_response(transport, exc): +async def maybe_send_error_response(wrapper, exc): # If we can't send an error, oh well, nothing to be done - transport.info("trying to send error response...") - if transport.conn.our_state not in {h11.IDLE, h11.SEND_RESPONSE}: - transport.info("...but I can't, because our state is", - transport.conn.our_state) + wrapper.info("trying to send error response...") + if wrapper.conn.our_state not in {h11.IDLE, h11.SEND_RESPONSE}: + wrapper.info("...but I can't, because our state is", + wrapper.conn.our_state) return - if isinstance(exc, h11.RemoteProtocolError): - status_code = exc.error_status_hint - else: - status_code = 500 - body = str(exc).encode("utf-8") - await send_simple_response(transport, - status_code, - "text/plain; charset=utf-8", - body) - -async def http_serve(sock, addr): - transport = CurioServerTransport(sock) - while True: - assert transport.conn.states == { - h11.CLIENT: h11.IDLE, h11.SERVER: h11.IDLE} - transport.info("Entering server main loop; waiting for event") - event = await transport.receive_next_event() - transport.info("Server main loop got event:", event) - if type(event) is h11.Request: - try: - await send_echo_response(transport, event) - except Exception as exc: - transport.info("Oops:", exc) - await maybe_send_error_response(transport, exc) - if transport.conn.our_state is h11.MUST_CLOSE: - transport.info("connection is not reusable, so shutting down") - await transport.shutdown_and_clean_up() - return - if transport.conn.states == { - h11.CLIENT: h11.DONE, h11.SERVER: h11.DONE}: - # Ready to re-use; loop back around - transport.info("connection reusable and all is well, so looping") - transport.conn.prepare_to_reuse() + try: + if isinstance(exc, h11.RemoteProtocolError): + status_code = exc.error_status_hint else: - # something has gone wrong - transport.info("Response handler left us with unexpected state", - transport.conn.states, - "bailing out") - await maybe_send_error_response( - transport, - RuntimeError( - "unexpected states {}".format(transport.conn.states))) - await transport.shutdown_and_clean_up() - return + status_code = 500 + body = str(exc).encode("utf-8") + await send_simple_response(wrapper, + status_code, + "text/plain; charset=utf-8", + body) + except Exception as exc: + wrapper.info("error while sending error response:", exc) -async def send_echo_response(transport, request): - transport.info("Preparing echo response") +async def send_echo_response(wrapper, request): + wrapper.info("Preparing echo response") + if request.method not in {b"GET", b"POST"}: + # Laziness: we should send a proper 405 Method Not Allowed with the + # appropriate Accept: header, but we don't. + raise RuntimeError("unsupported method") response_json = { "method": request.method.decode("ascii"), "target": request.target.decode("ascii"), @@ -172,7 +271,7 @@ async def send_echo_response(transport, request): "body": "", } while True: - event = await transport.receive_next_event() + event = await wrapper.next_event() if type(event) is h11.EndOfMessage: break assert type(event) is h11.Data @@ -182,11 +281,15 @@ async def send_echo_response(transport, request): indent=4, separators=(",", ": ")) response_body_bytes = response_body_unicode.encode("utf-8") - await send_simple_response(transport, + await send_simple_response(wrapper, 200, "application/json; charset=utf-8", response_body_bytes) +################################################################ +# Run the server +################################################################ + if __name__ == "__main__": kernel = curio.Kernel() print("Listening on http://localhost:8080") diff --git a/examples/tiny-client-demo.py b/examples/tiny-client-demo.py index 1cbeecc..0857269 100644 --- a/examples/tiny-client-demo.py +++ b/examples/tiny-client-demo.py @@ -2,11 +2,19 @@ import ssl import h11 +################################################################ +# Setup +################################################################ + conn = h11.Connection(our_role=h11.CLIENT) ctx = ssl.create_default_context() sock = ctx.wrap_socket(socket.create_connection(("httpbin.org", 443)), server_hostname="httpbin.org") +################################################################ +# Sending a request +################################################################ + def send(event): print("Sending event:") print(event) @@ -22,16 +30,33 @@ def send(event): ("Connection", "close")])) send(h11.EndOfMessage()) -done = False -while not done: - # Fetch data from the socket - data = sock.recv(2048) - # Give it to h11 to convert back into events - for event in conn.receive_data(data): - print("Received event:") - print(event) - print() - if type(event) is h11.EndOfMessage: - done = True +################################################################ +# Receiving the response +################################################################ + +def next_event(): + while True: + # Check if an event is already available + event = conn.next_event() + if event is h11.NEED_DATA: + # Nope, so fetch some data from the socket... + data = sock.recv(2048) + # ...and give it to h11 to convert back into events... + conn.receive_data(data) + # ...and then loop around to try again. + continue + return event + +while True: + event = next_event() + print("Received event:") + print(event) + print() + if type(event) is h11.EndOfMessage: + break + +################################################################ +# Clean up +################################################################ sock.close() diff --git a/h11/_connection.py b/h11/_connection.py index 965a13d..113c2ce 100644 --- a/h11/_connection.py +++ b/h11/_connection.py @@ -6,7 +6,7 @@ # Import all state sentinels from ._state import * # Import the internal things we need -from ._util import LocalProtocolError, RemoteProtocolError +from ._util import LocalProtocolError, RemoteProtocolError, Sentinel from ._state import ConnectionState, _SWITCH_UPGRADE, _SWITCH_CONNECT from ._headers import ( get_comma_header, set_comma_header, has_expect_100_continue, @@ -16,7 +16,10 @@ from ._writers import WRITERS # Everything in __all__ gets re-exported as part of the h11 public API. -__all__ = ["Connection"] +__all__ = ["Connection", "NEED_DATA", "PAUSED"] + +NEED_DATA = Sentinel("NEED_DATA") +PAUSED = Sentinel("PAUSED") # If we ever have this much buffered without it making a complete parseable # event, we error out. The only time we really buffer is when reading the @@ -28,7 +31,7 @@ # - tomcat: 8 * 1024 # - IIS: 16 * 1024 # - Apache: <8 KiB per line> -HTTP_DEFAULT_MAX_BUFFER_SIZE = 16 * 1024 +DEFAULT_MAX_INCOMPLETE_EVENT_SIZE = 16 * 1024 # RFC 7230's rules for connection lifecycles: # - If either side says they want to close the connection, then the connection @@ -110,16 +113,18 @@ class Connection(object): our_role: If you're implementing a client, pass :data:`h11.CLIENT`. If you're implementing a server, pass :data:`h11.SERVER`. - max_buffer_size (int): - The maximum number of bytes of received but unprocessed data we're - willing to buffer. In practice this mostly sets a limit on the + max_incomplete_event_size (int): + The maximum number of bytes we're willing to buffer of an + incomplete event. In practice this mostly sets a limit on the maximum size of the request/response line + headers. If this is - exceeded, then :meth:`receive_data` will raise + exceeded, then :meth:`next_event` will raise :exc:`RemoteProtocolError`. """ - def __init__(self, our_role, max_buffer_size=HTTP_DEFAULT_MAX_BUFFER_SIZE): - self._max_buffer_size = max_buffer_size + def __init__(self, + our_role, + max_incomplete_event_size=DEFAULT_MAX_INCOMPLETE_EVENT_SIZE): + self._max_incomplete_event_size = max_incomplete_event_size # State and role tracking if our_role not in (CLIENT, SERVER): raise ValueError( @@ -300,25 +305,16 @@ def trailing_data(self): return (bytes(self._receive_buffer), self._receive_buffer_closed) def receive_data(self, data): - """Convert bytes received from the remote peer into high-level events, - while updating our internal state machine. + """Add data to our internal recieve buffer. Args: - data (:term:`bytes-like object`, or None): - The new data that was just recieved. - - Normally, *data* is a :term:`bytes-like object` containing new - data received from the peer. We append this to our internal - receive buffer, and then check whether any new events can be - parsed from it. We always parse and return as many events as - possible. + data (:term:`bytes-like object`): + The new data that was just received. - There are two important special cases: - - **Special case 1:** If *data* is an empty byte-string like - ``b""``, then this indicates that the remote side has closed - the connection (end of file). Normally this is convenient, - because standard Python APIs like :meth:`file.read` or + Special case: If *data* is an empty byte-string like ``b""``, + then this indicates that the remote side has closed the + connection (end of file). Normally this is convenient, because + standard Python APIs like :meth:`file.read` or :meth:`socket.recv` use ``b""`` to indicate end-of-file, while other failures to read are indicated using other mechanisms like raising :exc:`TimeoutError`. When using such an API you @@ -330,27 +326,87 @@ def receive_data(self, data): make sure to check for such strings and avoid passing them to :meth:`receive_data`. - **Special case 2:** If *data* is ``None``, then we don't add - any data to the internal receive buffer, but we attempt to - parse it again to see if we can pull any new events out. + Returns: + Nothing, but after calling this you should call :meth:`next_event` + to parse the newly received data. + + Raises: + RuntimeError: + Raised if you pass an empty *data*, indicating EOF, and then + pass a non-empty *data*, indicating more data that somehow + arrived after the EOF. + + (Calling ``receive_data(b"")`` multiple times is fine, + and equivalent to calling it once.) + + """ + if data: + if self._receive_buffer_closed: + raise RuntimeError( + "received close, then received more data?") + self._receive_buffer += data + else: + self._receive_buffer_closed = True + + def _extract_next_receive_event(self): + state = self.their_state + # We don't pause immediately when they enter DONE, because even in + # DONE state we can still process a ConnectionClosed() event. But + # if we have data in our buffer, then we definitely aren't getting + # a ConnectionClosed() immediately and we need to pause. + if state is DONE and self._receive_buffer: + return PAUSED + if state is MIGHT_SWITCH_PROTOCOL or state is SWITCHED_PROTOCOL: + return PAUSED + assert self._reader is not None + event = self._reader(self._receive_buffer) + if event is None: + if not self._receive_buffer and self._receive_buffer_closed: + # In some unusual cases (basically just HTTP/1.0 bodies), EOF + # triggers an actual protocol event; in that case, we want to + # return that event, and then the state will change and we'll + # get called again to generate the actual ConnectionClosed(). + if hasattr(self._reader, "read_eof"): + event = self._reader.read_eof() + else: + event = ConnectionClosed() + if event is None: + event = NEED_DATA + return event - :meth:`receive_data` normally pulls out all possible events - immediately, so this is only useful after calling - :meth:`prepare_to_reuse` -- see - :ref:`keepalive-and-pipelining` for details. + def next_event(self): + """Parse the next event out of our receive buffer, update our internal + state, and return it. + + This is a mutating operation -- think of it like calling :func:`next` + on an iterator. Returns: - A list of :ref:`event ` objects. + One of three things: + + 1) An event object -- see :ref:`events`. + + 2) The special constant :data:`NEED_DATA`, which indicates that + you need to read more data from your socket and pass it to + :meth:`receive_data` before this method will be able to return + any more events. + + 3) The special constant :data:`PAUSED`, which indicates that we + are not in a state where we can process incoming data (usually + because the peer has finished their part of the current + request/response cycle, and you have not yet called + :meth:`prepare_to_reuse`). See :ref:`flow-control` for details. Raises: RemoteProtocolError: The peer has misbehaved. You should close the connection - (possibly after sending some kind of 400 response). + (possibly after sending some kind of 4xx response). + + Once this method returns :class:`ConnectionClosed` once, then all + subsequent calls will also return :class:`ConnectionClosed`. - For robustness you might want to be prepared to catch other exceptions - as well, but if this happens then please do file a bug report -- the - intention is that :exc:`RemoteProtocolError` is the *only* exception - that this method should be able to raise. + If this method raises any exception besides :exc:`RemoteProtocolError` + then that's a bug -- if it happens please file a bug report! If this method raises any exception then it also sets :attr:`Connection.their_state` to :data:`ERROR` -- see @@ -362,55 +418,22 @@ def receive_data(self, data): raise RemoteProtocolError( "Can't receive data when peer state is ERROR") try: - # Update self._receive_buffer with new data - if data is not None: - if data: - if self._receive_buffer_closed: - raise RuntimeError( - "received close, then received more data?") - self._receive_buffer += data - else: - self._receive_buffer_closed = True - - # Read out all the events we can - events = [] - while True: - event = self._extract_next_receive_event() - if event is None: - break - events.append(event) + event = self._extract_next_receive_event() + if type(event) is not Sentinel: self._process_event(self.their_role, event) - if type(event) is ConnectionClosed: - break - - # Buffer maintainence - self._receive_buffer.compress() - if len(self._receive_buffer) > self._max_buffer_size: - # We don't enforce buffer size limits when paused, because - # ever-growing buffers here would indicate a problem with - # the user code, not with the remote client -- and it's - # entirely possible that a single receive_data call all by - # itself could put us over this limit, with no real way to - # avoid it. - if not self._paused(): + self._receive_buffer.compress() + if event is NEED_DATA: + if len(self._receive_buffer) > self._max_incomplete_event_size: # 431 is "Request header fields too large" which is pretty # much the only situation where we can get here raise RemoteProtocolError("Receive buffer too long", error_status_hint=431) - - # We've greedily processed all possible events, so if there's no - # more data coming, we better either be paused or else have - # delivered that ConnectionClosed -- we don't want to hang forever - # waiting for data that never arrives. - if self._receive_buffer_closed: - if not (self._paused() - or (events - and type(events[-1]) is ConnectionClosed)): + if self._receive_buffer_closed: + # We're still trying to complete some event, but that's + # never going to happen because no more data is coming raise RemoteProtocolError( "peer unexpectedly closed connection") - - # Return them - return events + return event except BaseException as exc: self._process_error(self.their_role) if isinstance(exc, LocalProtocolError): @@ -418,35 +441,6 @@ def receive_data(self, data): else: raise - def _paused(self): - state = self.their_state - # We don't pause immediately when they enter DONE, because even in - # DONE state we can still process a ConnectionClosed() event. But - # if we have data in our buffer, then we definitely aren't getting - # a ConnectionClosed() immediately and we need to pause. - if state is DONE and self._receive_buffer: - return True - if state in {MIGHT_SWITCH_PROTOCOL, SWITCHED_PROTOCOL}: - return True - return False - - def _extract_next_receive_event(self): - if self._paused(): - return None - assert self._reader is not None - event = self._reader(self._receive_buffer) - if event is None: - if not self._receive_buffer and self._receive_buffer_closed: - # In some unusual cases (basically just HTTP/1.0 bodies), EOF - # triggers an actual protocol event; in that case, we want to - # return that event, and then the state will change and we'll - # get called again to generate the actual ConnectionClosed(). - if hasattr(self._reader, "read_eof"): - event = self._reader.read_eof() - else: - event = ConnectionClosed() - return event - def send(self, event): """Convert a high-level event into bytes that can be sent to the peer, while updating our internal state machine. diff --git a/h11/_receivebuffer.py b/h11/_receivebuffer.py index e677f49..62f9a7a 100644 --- a/h11/_receivebuffer.py +++ b/h11/_receivebuffer.py @@ -39,7 +39,9 @@ def __len__(self): return len(self._data) - self._start def compress(self): - if self._start > 0: + # Heuristic: only compress if it lets us reduce size by a factor + # of 2 + if self._start > len(self._data) // 2: self._data = self._data[self._start:] self._looked_at -= self._start self._start -= self._start diff --git a/h11/tests/helpers.py b/h11/tests/helpers.py index 23f8992..6158995 100644 --- a/h11/tests/helpers.py +++ b/h11/tests/helpers.py @@ -1,6 +1,21 @@ from .._events import * from .._state import * -from .._connection import Connection +from .._connection import * + +def get_all_events(conn): + got_events = [] + while True: + event = conn.next_event() + if event in (NEED_DATA, PAUSED): + break + got_events.append(event) + if type(event) is ConnectionClosed: + break + return got_events + +def receive_and_get(conn, data): + conn.receive_data(data) + return get_all_events(conn) # Merges adjacent Data events, and converts payloads to bytestrings def normalize_data_events(in_events): @@ -27,7 +42,7 @@ def __init__(self): def conns(self): return self.conn.values() - # expect=None to disable checking, expect=[...] to say what expected + # expect="match" if expect=send_events; expect=[...] to say what expected def send(self, role, send_events, expect="match"): if not isinstance(send_events, list): send_events = [send_events] @@ -42,11 +57,11 @@ def send(self, role, send_events, expect="match"): # send uses b"" to mean b"", and None to mean closed # receive uses b"" to mean closed, and None to mean "try again" # so we have to translate between the two conventions - got_events = [] if data: - got_events += self.conn[self.other[role]].receive_data(data) + self.conn[self.other[role]].receive_data(data) if closed: - got_events += self.conn[self.other[role]].receive_data(b"") + self.conn[self.other[role]].receive_data(b"") + got_events = get_all_events(self.conn[self.other[role]]) if expect == "match": expect = send_events if not isinstance(expect, list): diff --git a/h11/tests/test_against_stdlib_http.py b/h11/tests/test_against_stdlib_http.py index 6bd4625..5e70cac 100644 --- a/h11/tests/test_against_stdlib_http.py +++ b/h11/tests/test_against_stdlib_http.py @@ -51,18 +51,20 @@ def test_h11_as_client(): s.sendall(c.send(h11.EndOfMessage())) data = bytearray() - done = False - while not done: - # Use a small read buffer to make things more challenging and - # exercise more paths :-) - for event in c.receive_data(s.recv(10)): - print(event) - if type(event) is h11.Response: - assert event.status_code == 200 - if type(event) is h11.Data: - data += event.data - if type(event) is h11.EndOfMessage: - done = True + while True: + event = c.next_event() + print(event) + if event is h11.NEED_DATA: + # Use a small read buffer to make things more challenging + # and exercise more paths :-) + c.receive_data(s.recv(10)) + continue + if type(event) is h11.Response: + assert event.status_code == 200 + if type(event) is h11.Data: + data += event.data + if type(event) is h11.EndOfMessage: + break assert bytes(data) == test_file_data class H11RequestHandler(socketserver.BaseRequestHandler): @@ -70,17 +72,17 @@ def handle(self): with closing(self.request) as s: c = h11.Connection(h11.SERVER) request = None - done = False - while not done: - # Use a small read buffer to make things more challenging and - # exercise more paths :-) - for event in c.receive_data(s.recv(10)): - print(event) - if type(event) is h11.Request: - request = event - if type(event) is h11.EndOfMessage: - done = True - break + while True: + event = c.next_event() + if event is h11.NEED_DATA: + # Use a small read buffer to make things more challenging + # and exercise more paths :-) + c.receive_data(s.recv(10)) + continue + if type(event) is h11.Request: + request = event + if type(event) is h11.EndOfMessage: + break info = json.dumps({ "method": request.method.decode("ascii"), "target": request.target.decode("ascii"), diff --git a/h11/tests/test_connection.py b/h11/tests/test_connection.py index 3e2db09..76fea93 100644 --- a/h11/tests/test_connection.py +++ b/h11/tests/test_connection.py @@ -5,10 +5,10 @@ from .._state import * from .._connection import ( _keep_alive, _body_framing, - Connection, + Connection, NEED_DATA, PAUSED, ) -from .helpers import ConnectionPair +from .helpers import ConnectionPair, get_all_events, receive_and_get def test__keep_alive(): assert _keep_alive( @@ -169,23 +169,24 @@ def test_client_talking_to_http10_server(): c.send(EndOfMessage()) assert c.our_state is DONE # No content-length, so Http10 framing for body - assert (c.receive_data(b"HTTP/1.0 200 OK\r\n\r\n") + assert (receive_and_get(c, b"HTTP/1.0 200 OK\r\n\r\n") == [Response(status_code=200, headers=[], http_version="1.0")]) assert c.our_state is MUST_CLOSE - assert (c.receive_data(b"12345") == [Data(data=b"12345")]) - assert (c.receive_data(b"67890") == [Data(data=b"67890")]) - assert (c.receive_data(b"") == [EndOfMessage(), ConnectionClosed()]) + assert (receive_and_get(c, b"12345") == [Data(data=b"12345")]) + assert (receive_and_get(c, b"67890") == [Data(data=b"67890")]) + assert (receive_and_get(c, b"") == [EndOfMessage(), ConnectionClosed()]) assert c.their_state is CLOSED def test_server_talking_to_http10_client(): c = Connection(SERVER) # No content-length, so no body # NB: no host header - assert (c.receive_data(b"GET / HTTP/1.0\r\n\r\n") - == [Request(method="GET", target="/", - headers=[], - http_version="1.0"), - EndOfMessage()]) + assert receive_and_get(c, b"GET / HTTP/1.0\r\n\r\n") == [ + Request(method="GET", target="/", + headers=[], + http_version="1.0"), + EndOfMessage(), + ] assert c.their_state is MUST_CLOSE # We automatically Connection: close back at them @@ -199,15 +200,16 @@ def test_server_talking_to_http10_client(): # Check that it works if they do send Content-Length c = Connection(SERVER) # NB: no host header - assert (c.receive_data(b"POST / HTTP/1.0\r\nContent-Length: 10\r\n\r\n1") + assert (receive_and_get(c, + b"POST / HTTP/1.0\r\nContent-Length: 10\r\n\r\n1") == [Request(method="POST", target="/", headers=[("Content-Length", "10")], http_version="1.0"), Data(data=b"1")]) - assert (c.receive_data(b"234567890") + assert (receive_and_get(c, b"234567890") == [Data(data=b"234567890"), EndOfMessage()]) assert c.their_state is MUST_CLOSE - assert c.receive_data(b"") == [ConnectionClosed()] + assert receive_and_get(c, b"") == [ConnectionClosed()] def test_automatic_transfer_encoding_in_response(): # Check that in responses, the user can specify either Transfer-Encoding: @@ -237,7 +239,7 @@ def test_automatic_transfer_encoding_in_response(): # When speaking to HTTP/1.0 client, all of the above cases get # normalized to no-framing-headers c = Connection(SERVER) - c.receive_data(b"GET / HTTP/1.0\r\n\r\n") + receive_and_get(c, b"GET / HTTP/1.0\r\n\r\n") assert (c.send(Response(status_code=200, headers=user_headers)) == b"HTTP/1.1 200 \r\nconnection: close\r\n\r\n") assert c.send(Data(data=b"12345")) == b"12345" @@ -304,59 +306,66 @@ def setup(): assert not conn.client_is_waiting_for_100_continue assert not conn.they_are_waiting_for_100_continue -def test_max_buffer_size_countermeasure(): +def test_max_incomplete_event_size_countermeasure(): # Infinitely long headers are definitely not okay c = Connection(SERVER) c.receive_data(b"GET / HTTP/1.0\r\nEndless: ") + assert c.next_event() is NEED_DATA with pytest.raises(RemoteProtocolError): while True: c.receive_data(b"a" * 1024) + c.next_event() # Checking that the same header is accepted / rejected depending on the - # max_buffer_size setting: - c = Connection(SERVER, max_buffer_size=5000) + # max_incomplete_event_size setting: + c = Connection(SERVER, max_incomplete_event_size=5000) c.receive_data(b"GET / HTTP/1.0\r\nBig: ") c.receive_data(b"a" * 4000) - assert c.receive_data(b"\r\n\r\n") == [ + c.receive_data(b"\r\n\r\n") + assert get_all_events(c) == [ Request(method="GET", target="/", http_version="1.0", headers=[("big", "a" * 4000)]), EndOfMessage(), ] - c = Connection(SERVER, max_buffer_size=4000) + c = Connection(SERVER, max_incomplete_event_size=4000) c.receive_data(b"GET / HTTP/1.0\r\nBig: ") + c.receive_data(b"a" * 4000) with pytest.raises(RemoteProtocolError): - c.receive_data(b"a" * 4000) + c.next_event() - # Temporarily exceeding the max buffer size is fine; it's just maintaining - # large buffers over multiple calls that's a problem: - c = Connection(SERVER, max_buffer_size=5000) + # Temporarily exceeding the size limit is fine, as long as its done with + # complete events: + c = Connection(SERVER, max_incomplete_event_size=5000) c.receive_data(b"GET / HTTP/1.0\r\nContent-Length: 10000") - assert c.receive_data(b"\r\n\r\n" + b"a" * 10000) == [ + c.receive_data(b"\r\n\r\n" + b"a" * 10000) + assert get_all_events(c) == [ Request(method="GET", target="/", http_version="1.0", headers=[("Content-Length", "10000")]), Data(data=b"a" * 10000), EndOfMessage(), ] - # Exceeding the max buffer size is fine if we are paused - c = Connection(SERVER, max_buffer_size=100) + c = Connection(SERVER, max_incomplete_event_size=100) # Two pipelined requests to create a way-too-big receive buffer... but # it's fine because we're not checking - assert (c.receive_data(b"GET /1 HTTP/1.1\r\nHost: a\r\n\r\n" - b"GET /2 HTTP/1.1\r\nHost: b\r\n\r\n" - + b"X" * 1000) - == [Request(method="GET", target="/1", headers=[("host", "a")]), - EndOfMessage()]) + c.receive_data(b"GET /1 HTTP/1.1\r\nHost: a\r\n\r\n" + b"GET /2 HTTP/1.1\r\nHost: b\r\n\r\n" + + b"X" * 1000) + assert get_all_events(c) == [ + Request(method="GET", target="/1", headers=[("host", "a")]), + EndOfMessage(), + ] # Even more data comes in, still no problem c.receive_data(b"X" * 1000) # We can respond and reuse to get the second pipelined request c.send(Response(status_code=200, headers=[])) c.send(EndOfMessage()) c.prepare_to_reuse() - assert (c.receive_data(None) - == [Request(method="GET", target="/2", headers=[("host", "b")]), - EndOfMessage()]) + assert get_all_events(c) == [ + Request(method="GET", target="/2", headers=[("host", "b")]), + EndOfMessage(), + ] # But once we unpause and try to read the next message, and find that it's # incomplete and the buffer is *still* way too large, then *that's* a # problem: @@ -364,7 +373,7 @@ def test_max_buffer_size_countermeasure(): c.send(EndOfMessage()) c.prepare_to_reuse() with pytest.raises(RemoteProtocolError): - c.receive_data(None) + c.next_event() def test_reuse_simple(): p = ConnectionPair() @@ -388,15 +397,15 @@ def test_reuse_simple(): def test_pipelining(): # Client doesn't support pipelining, so we have to do this by hand c = Connection(SERVER) - assert c.receive_data(None) == [] + assert c.next_event() is NEED_DATA # 3 requests all bunched up - events = c.receive_data( + c.receive_data( b"GET /1 HTTP/1.1\r\nHost: a.com\r\nContent-Length: 5\r\n\r\n" b"12345" b"GET /2 HTTP/1.1\r\nHost: a.com\r\nContent-Length: 5\r\n\r\n" b"67890" b"GET /3 HTTP/1.1\r\nHost: a.com\r\n\r\n") - assert events == [ + assert get_all_events(c) == [ Request(method="GET", target="/1", headers=[("Host", "a.com"), ("Content-Length", "5")]), Data(data=b"12345"), @@ -405,7 +414,7 @@ def test_pipelining(): assert c.their_state is DONE assert c.our_state is SEND_RESPONSE - assert c.receive_data(None) == [] + assert c.next_event() is PAUSED c.send(Response(status_code=200, headers=[])) c.send(EndOfMessage()) @@ -414,42 +423,41 @@ def test_pipelining(): c.prepare_to_reuse() - events = c.receive_data(None) - assert events == [ + assert get_all_events(c) == [ Request(method="GET", target="/2", headers=[("Host", "a.com"), ("Content-Length", "5")]), Data(data=b"67890"), EndOfMessage(), ] + assert c.next_event() is PAUSED c.send(Response(status_code=200, headers=[])) c.send(EndOfMessage()) c.prepare_to_reuse() - events = c.receive_data(None) - assert events == [ + assert get_all_events(c) == [ Request(method="GET", target="/3", headers=[("Host", "a.com")]), EndOfMessage(), - # Doesn't pause this time, no trailing data ] + # Doesn't pause this time, no trailing data + assert c.next_event() is NEED_DATA c.send(Response(status_code=200, headers=[])) c.send(EndOfMessage()) # Arrival of more data triggers pause - assert c.receive_data(None) == [] - assert c.receive_data(b"SADF") == [] + assert c.next_event() is NEED_DATA + c.receive_data(b"SADF") + assert c.next_event() is PAUSED assert c.trailing_data == (b"SADF", False) - assert c.receive_data(b"") == [] + # If EOF arrives while paused, we don't see that either: + c.receive_data(b"") assert c.trailing_data == (b"SADF", True) - assert c.receive_data(None) == [] - assert c.receive_data(b"") == [] + assert c.next_event() is PAUSED + c.receive_data(b"") + assert c.next_event() is PAUSED # Can't call receive_data with non-empty buf after closing it with pytest.raises(RuntimeError): c.receive_data(b"FDSA") - # Can't re-use after an error like that - with pytest.raises(LocalProtocolError): - c.prepare_to_reuse() - def test_protocol_switch(): for (req, deny, accept) in [ @@ -494,7 +502,7 @@ def setup(): p.send(CLIENT, [Data(data=b"1"), EndOfMessage()]) for conn in p.conns: assert conn.states[CLIENT] is MIGHT_SWITCH_PROTOCOL - assert p.conn[SERVER].receive_data(None) == [] + assert p.conn[SERVER].next_event() is PAUSED return p # Test deny case @@ -513,8 +521,10 @@ def setup(): for conn in p.conns: assert conn.states == {CLIENT: SWITCHED_PROTOCOL, SERVER: SWITCHED_PROTOCOL} - assert conn.receive_data(b"123") == [] - assert conn.receive_data(b"456") == [] + conn.receive_data(b"123") + assert conn.next_event() is PAUSED + conn.receive_data(b"456") + assert conn.next_event() is PAUSED assert conn.trailing_data == (b"123456", False) # Pausing in might-switch, then recovery @@ -523,14 +533,14 @@ def setup(): # logic) p = setup() sc = p.conn[SERVER] - assert sc.receive_data(b"GET / HTTP/1.0\r\n\r\n") == [] - assert sc.receive_data(None) == [] + sc.receive_data(b"GET / HTTP/1.0\r\n\r\n") + assert sc.next_event() is PAUSED assert sc.trailing_data == (b"GET / HTTP/1.0\r\n\r\n", False) sc.send(deny) - assert sc.receive_data(None) == [] + assert sc.next_event() is PAUSED sc.send(EndOfMessage()) sc.prepare_to_reuse() - assert sc.receive_data(None) == [ + assert get_all_events(sc) == [ Request(method="GET", target="/", headers=[], http_version="1.0"), EndOfMessage(), ] @@ -540,17 +550,18 @@ def setup(): # switched, we don't. p = setup() sc = p.conn[SERVER] - assert sc.receive_data(b"") == [] - assert sc.receive_data(None) == [] + sc.receive_data(b"") + assert sc.next_event() is PAUSED assert sc.trailing_data == (b"", True) p.send(SERVER, accept) - assert sc.receive_data(None) == [] + assert sc.next_event() is PAUSED p = setup() sc = p.conn[SERVER] - assert sc.receive_data(b"") == [] + sc.receive_data(b"") == [] + assert sc.next_event() is PAUSED sc.send(deny) - assert sc.receive_data(None) == [ConnectionClosed()] + assert sc.next_event() == ConnectionClosed() # You can't send after switching protocols, or while waiting for a # protocol switch @@ -583,12 +594,10 @@ def setup(): # You can keep putting b"" into a closed connection, and you keep # getting ConnectionClosed() out: p = setup() - assert p.conn[who_shot_second].receive_data(None) == [ - ConnectionClosed(), - ] - assert p.conn[who_shot_second].receive_data(b"") == [ - ConnectionClosed(), - ] + assert p.conn[who_shot_second].next_event() == ConnectionClosed() + assert p.conn[who_shot_second].next_event() == ConnectionClosed() + p.conn[who_shot_second].receive_data(b"") + assert p.conn[who_shot_second].next_event() == ConnectionClosed() # Second party can close... p = setup() p.send(who_shot_second, ConnectionClosed()) @@ -603,9 +612,9 @@ def setup(): p.conn[who_shot_second].receive_data(b"123") # And receiving new data on a MUST_CLOSE connection is a ProtocolError p = setup() + p.conn[who_shot_first].receive_data(b"GET") with pytest.raises(RemoteProtocolError): - p.conn[who_shot_first].receive_data(b"GET") - + p.conn[who_shot_first].next_event() def test_close_different_states(): req = [Request(method="GET", target="/foo", headers=[("Host", "a")]), @@ -630,8 +639,9 @@ def test_close_different_states(): p.send(CLIENT, req) with pytest.raises(LocalProtocolError): p.conn[SERVER].send(ConnectionClosed()) + p.conn[CLIENT].receive_data(b"") with pytest.raises(RemoteProtocolError): - p.conn[CLIENT].receive_data(b"") + p.conn[CLIENT].next_event() # Server after response p = ConnectionPair() @@ -657,8 +667,9 @@ def test_close_different_states(): headers=[("Host", "a"), ("Content-Length", "10")])) with pytest.raises(LocalProtocolError): p.conn[CLIENT].send(ConnectionClosed()) + p.conn[SERVER].receive_data(b"") with pytest.raises(RemoteProtocolError): - p.conn[SERVER].receive_data(b"") + p.conn[SERVER].next_event() # Receive several requests and then client shuts down their side of the # connection; we can respond to each @@ -671,12 +682,18 @@ def test_pipelined_close(): b"GET /2 HTTP/1.1\r\nHost: a.com\r\nContent-Length: 5\r\n\r\n" b"67890") c.receive_data(b"") + assert get_all_events(c) == [ + Request(method="GET", target="/1", + headers=[("host", "a.com"), ("content-length", "5")]), + Data(data=b"12345"), + EndOfMessage(), + ] assert c.states[CLIENT] is DONE c.send(Response(status_code=200, headers=[])) c.send(EndOfMessage()) assert c.states[SERVER] is DONE c.prepare_to_reuse() - assert c.receive_data(None) == [ + assert get_all_events(c) == [ Request(method="GET", target="/2", headers=[("host", "a.com"), ("content-length", "5")]), Data(data=b"67890"), @@ -698,9 +715,10 @@ def __len__(self): def setup(header, http_version): c = Connection(SERVER) - c.receive_data("GET / HTTP/{}\r\nHost: a\r\n\r\n" - .format(http_version) - .encode("ascii")) + receive_and_get(c, + "GET / HTTP/{}\r\nHost: a\r\n\r\n" + .format(http_version) + .encode("ascii")) headers = [] if header: headers.append(header) @@ -726,14 +744,15 @@ def test_errors(): # After a receive error, you can't receive for role in [CLIENT, SERVER]: c = Connection(our_role=role) + c.receive_data(b"gibberish\r\n\r\n") with pytest.raises(RemoteProtocolError): - c.receive_data(b"gibberish\r\n\r\n") + c.next_event() # Now any attempt to receive continues to raise assert c.their_state is ERROR assert c.our_state is not ERROR print(c._cstate.states) with pytest.raises(RemoteProtocolError): - c.receive_data(None) + c.next_event() # But we can still yell at the client for sending us gibberish if role is SERVER: assert (c.send(Response(status_code=400, headers=[])) @@ -746,7 +765,7 @@ def conn(role): c = Connection(our_role=role) if role is SERVER: # Put it into the state where it *could* send a response... - c.receive_data(b"GET / HTTP/1.0\r\n\r\n") + receive_and_get(c, b"GET / HTTP/1.0\r\n\r\n") assert c.our_state is SEND_RESPONSE return c @@ -780,13 +799,15 @@ def test_idle_receive_nothing(): # At one point this incorrectly raised an error for role in [CLIENT, SERVER]: c = Connection(role) - assert c.receive_data(None) == [] + assert c.next_event() is NEED_DATA def test_connection_drop(): c = Connection(SERVER) - assert c.receive_data(b"GET /") == [] + c.receive_data(b"GET /") + assert c.next_event() is NEED_DATA + c.receive_data(b"") with pytest.raises(RemoteProtocolError): - c.receive_data(b"") + c.next_event() def test_408_request_timeout(): # Should be able to send this spontaneously as a server without seeing @@ -797,12 +818,14 @@ def test_408_request_timeout(): # This used to raise IndexError def test_empty_request(): c = Connection(SERVER) + c.receive_data(b"\r\n") with pytest.raises(RemoteProtocolError): - c.receive_data(b"\r\n") + c.next_event() # This used to raise IndexError def test_empty_response(): c = Connection(CLIENT) c.send(Request(method="GET", target="/", headers=[("Host", "a")])) + c.receive_data(b"\r\n") with pytest.raises(RemoteProtocolError): - c.receive_data(b"\r\n") + c.next_event() From 235430ef759068f5f3e82ad547e37f68e4af217e Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Wed, 18 May 2016 01:14:49 -0700 Subject: [PATCH 8/9] Update fuzz harness for new receive API --- fuzz/afl-server.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/fuzz/afl-server.py b/fuzz/afl-server.py index adbf955..972d2dd 100644 --- a/fuzz/afl-server.py +++ b/fuzz/afl-server.py @@ -14,6 +14,14 @@ else: in_file = sys.stdin +def process_all(c): + while True: + event = c.next_event() + if event is h11.NEED_DATA or event is h11.PAUSED: + break + if type(event) is h11.ConnectionClosed: + break + afl.init() data = in_file.read() @@ -22,19 +30,20 @@ server1 = h11.Connection(h11.SERVER) try: server1.receive_data(data) + process_all(server1) server1.receive_data(b"") + process_all(server1) except h11.RemoteProtocolError: pass # byte at a time server2 = h11.Connection(h11.SERVER) -for i in range(len(data)): - try: - server2.receive_data(data[i:i + 1]) - except h11.RemoteProtocolError: - pass try: + for i in range(len(data)): + server2.receive_data(data[i:i + 1]) + process_all(server2) server2.receive_data(b"") + process_all(server2) except h11.RemoteProtocolError: pass From ca1a5b59b44aa272a7b4384bb2616428e3ecdc34 Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Wed, 18 May 2016 12:08:39 -0700 Subject: [PATCH 9/9] Small doc tweak --- h11/_connection.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/h11/_connection.py b/h11/_connection.py index 113c2ce..b0f57a7 100644 --- a/h11/_connection.py +++ b/h11/_connection.py @@ -307,6 +307,9 @@ def trailing_data(self): def receive_data(self, data): """Add data to our internal recieve buffer. + This does not actually do any processing on the data, just stores + it. To trigger processing, you have to call :meth:`next_event`. + Args: data (:term:`bytes-like object`): The new data that was just received.