classification
Title: httplib persistent connections violate MUST in RFC2616 sec 8.1.4.
Type: Stage:
Components: Library (Lib) Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: orsenthil Nosy List: Yuri.Bochkarev, agriffis, alanjds, amak, cananian, demian.brecht, icordasc, jcea, jhylton, mhammond, orsenthil, r.david.murray, rbcollins, vadmium
Priority: normal Keywords: patch

Created on 2008-08-16 15:41 by cananian, last changed 2015-01-31 12:38 by vadmium.

Files
File name Uploaded Description Edit
persistent-closing.py vadmium, 2015-01-17 06:26 Demo closing after normal and long request
persistent-closing.py vadmium, 2015-01-19 23:51 Avoid EPIPE race; demo ECONNRESET
zero_response_poc.py demian.brecht, 2015-01-20 18:10
ConnectionClosed.patch vadmium, 2015-01-24 08:42 review
ConnectionClosed.v2.patch vadmium, 2015-01-25 00:52 review
ConnectionClosed.v3.patch vadmium, 2015-01-31 12:37 review
Messages (29)
msg71221 - (view) Author: C. Scott Ananian (cananian) Date: 2008-08-16 15:41
Although HTTP/1.1 says that servers SHOULD send a Connection: close
header if they intend to close a persistent connection (sec 8.1.2.1),
clients (like httplib) "MUST be able to recover from asynchronous close
events. Client software SHOULD reopen the transport connection and
retransmit the aborted sequence of requests without user interaction so
long as the request sequence is idempotent" (sec 8.1.4) since servers
MAY close a persistent connection after a request due to a timeout or
other reason.  Further, "Clients and servers SHOULD both constantly
watch for the other side of the transport close, and respond to it as
appropriate." (sec 8.1.4).

httplib currently does not detect when the server closes its side of the
connection, until the following bit of HTTPResponse in httplib.py
(python2.5.2):

    def _read_status(self):
        # Initialize with Simple-Response defaults
        line = self.fp.readline()
        ...
        if not line:
            # Presumably, the server closed the connection before
            # sending a valid response.
            raise BadStatusLine(line)
        ...

So we end up raising a BadStatusLine exception for a completely
permissible case: the server closed a persistent connection.  This
causes persistent connections to fail for users of httplib in mysterious
ways, especially if they are behind proxies: Squid, for example, seems
to limit persistent connections to a maximum of 3 requests, and then
closes the connection, causing future requests to raise the BadStatusLine.

There appears to be code attempting to fix this problem in
HTTPConnection.request(), but it doesn't always work.  RFC793 says, "If
an unsolicited FIN arrives from the network, the receiving TCP can ACK
it and tell the user that the connection is closing.  The user will
respond with a CLOSE, upon which the TCP can send a FIN to the other TCP
after sending any remaining data." (sec 3.5 case 2)  Key phrase here is
"after sending any remaining data": python is usually allowed to put the
request on the network without raising a socket.error; the close is not
signaled to python until HTTPResponse.begin() invokes
HTTPResponse._read_status.

It would be best to extend the retry logic in request() to cover this
case as well (store away the previous parameters to request() and if
_read_status() fails invoke HTTPConnection.close(),
HTTPConnection.connect(), HTTPConnection.request(stored_params), and
retry the HTTPConnection.getresponse().  But at the very least python
should document and raise a EAGAIN exception of some kind so that the
caller can distinguish this case from an actual bad status line and
retry the request.
msg71806 - (view) Author: C. Scott Ananian (cananian) Date: 2008-08-23 13:44
FWIW, the following code works around the issue for me:

class HTTPConnection(httplib.HTTPConnection):
    def request(self, method, url, body=None, headers={}):
        # patch upstream to recv() after we send, so that a closed
        # connection is discovered here rather than later in
        # HTTPResponse._read_status()
        try:
            self._send_request(method, url, body, headers)
            b = self.sock.recv(1, socket.MSG_PEEK)
            if b == '':
                self.close()
                raise socket.error(32) # sender closed connection.
        except socket.error, v:
            # trap 'Broken pipe' if we're allowed to automatically reconnect
            if v[0] != 32 or not self.auto_open:
                raise
            # try one more time
            self._send_request(method, url, body, headers)

In general, attempting to read a byte of the response just to trigger
the broken pipe error is suboptimal; it doesn't accurately distinguish
the "closed before received request" case from the "closed just after
received request" case.  For idempotent methods like GET and HEAD, this
doesn't matter, but it does for POST, etc.

There ought to be a better way to force the underlying OS to give you
the current 'closed' status on a socket which doesn't require your
trying to send/receive bytes.  The fundamental issue here is that the
TCP spec, and Linux, allow the request send to succeed even if TCP FIN
has been received from the other side -- the receipt of FIN after the
response is received is always going to be a race to some degree.

All of which is why the HTTP pipelining specs encourage you *not* to use
pipelining for non-idempotent requests.  The above patch could be
tweaked to only perform the MSG_PEEK and retry if this is the second or
later pipelined request.  Then, following the HTTP recommendations and
always calling HttpConnection.close()/HttpConnection.connect() before
invoking "unsafe" requests like POST, would make the whole thing safe.
msg91650 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2009-08-16 22:24
btw, when using async io (poll, select, etc) I -think- your socket will
see a read event when the server closes the connection (sends you a FIN
or even a RST) at which point your sock.recv() when you've been told
data was ready will return 0 bytes indicating that the server has closed
the connection.

I like your MSG_PEEK hack here.  I'll figure out if this or something
else should go into (the giant mess that is) httplib.
msg91681 - (view) Author: Jeremy Hylton (jhylton) Date: 2009-08-18 11:32
Not that we've removed the try one more time branch of the code,
because it was causing other problems.

Jeremy

On Sun, Aug 16, 2009 at 6:24 PM, Gregory P. Smith<report@bugs.python.org> wrote:
>
> Gregory P. Smith <greg@krypto.org> added the comment:
>
> btw, when using async io (poll, select, etc) I -think- your socket will
> see a read event when the server closes the connection (sends you a FIN
> or even a RST) at which point your sock.recv() when you've been told
> data was ready will return 0 bytes indicating that the server has closed
> the connection.
>
> I like your MSG_PEEK hack here.  I'll figure out if this or something
> else should go into (the giant mess that is) httplib.
>
> ----------
> priority:  -> normal
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue3566>
> _______________________________________
> _______________________________________________
> Python-bugs-list mailing list
> Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/jeremy%40alum.mit.edu
>
>
msg124366 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2010-12-19 22:05
unassigning, i don't had time to look at this one.
msg206661 - (view) Author: Alan Justino (alanjds) Date: 2013-12-20 02:38
Seems to affect 2.7 too.
msg231413 - (view) Author: Martin Panter (vadmium) * Date: 2014-11-20 04:37
I don’t think the peek hack needs to go into to the standard library. It is just a workaround for code using the current library, to differentiate an empty status line due to EOF from any other kind of “bad” status line. This is what Issue 8450, “False BadStatusLine” is about.
msg234107 - (view) Author: Demian Brecht (demian.brecht) * Date: 2015-01-16 02:00
Trying to reproduce this on my own in 3.5, 2.7 and 2.5 yields a ConnectionResetError (ECONNRESET), which seems to be correct. That said, this could be due to varying TCP implementations on the server so might still be valid. It could also be due to an older kernel which has been fixed since this was originally reported. Is this still reproducible? If so, can an example be provided?

If the error as written is reproducible, I think that the error message should be fixed, but I'm not so sure that any more than that should be done.


As far as the RFC goes, I think the MUST clause pointed out can be left to the interpretation of the reader. You /could/ consider http.client as the client, but you could also consider the application that a user is interacting with as the client.

Offhand, I think that the library as it is does the right thing in allowing the application code to handle the exceptions as it sees fit. Because http.client in its current state doesn't allow for request pipelining, retries from calling code should be trivial, if that is what the caller intends to do.
msg234108 - (view) Author: Martin Panter (vadmium) * Date: 2015-01-16 02:20
I believe the BadStatusLine can still happen, depending on the circumstances. When I get a chance I will see if I can make a demonstration. In the meantime these comments from my persistent connection handler <https://github.com/vadmium/python-iview/blob/587a198/iview/utils.py#L169> might be useful:

# If the server closed the connection,
# by calling close() or shutdown(SHUT_WR),
# before receiving a short request (<= 1 MB),
# the "http.client" module raises a BadStatusLine exception.
#
# To produce EPIPE:
# 1. server: close() or shutdown(SHUT_RDWR)
# 2. client: send(large request >> 1 MB)
#
# ENOTCONN probably not possible with current Python,
# but could be generated on Linux by:
# 1. server: close() or shutdown(SHUT_RDWR)
# 2. client: send(finite data)
# 3. client: shutdown()
# ENOTCONN not covered by ConnectionError even in Python 3.3.
#
# To produce ECONNRESET:
# 1. client: send(finite data)
# 2. server: close() without reading all data
# 3. client: send()

I think these behaviours were from experimenting on Linux with Python 3 sockets, and reading the man pages.

I think there should be a new documented exception, a subclass of BadStatusLine for backwards compatibility. Then user code could catch the new exception, and true bad status lines that do not conform to the specification or whatever won’t be caught. I agree that the library shouldn’t be doing any special retrying of its own, but should make it easy for the caller to do so.
msg234163 - (view) Author: Martin Panter (vadmium) * Date: 2015-01-17 06:26
Okay here is a demonstration script, which does two tests: a short basic GET request, and a 2 MB POST request. Output for me is usually:

Platform: Linux-3.15.5-2-ARCH-x86_64-with-arch
Normal request: getresponse() raised BadStatusLine("''",)
2 MB request: request() raised BrokenPipeError(32, 'Broken pipe'); errno=EPIPE

Sometimes I get a BadStatusLine even for the second request:

Platform: Linux-3.15.5-2-ARCH-x86_64-with-arch
Normal request: getresponse() raised BadStatusLine("''",)
2 MB request: getresponse() raised BadStatusLine("''",)
msg234294 - (view) Author: Martin Panter (vadmium) * Date: 2015-01-18 23:55
Spotted code in Python’s own library that maintains a persistent connection and works around this issue:

Lib/xmlrpc/client.py:1142
msg234315 - (view) Author: Demian Brecht (demian.brecht) * Date: 2015-01-19 16:34
Hi Martin,

Thanks for the example code. I'm not sure whether or not this was your intention, but your example demonstrates a misbehaving client, one that seems to expect a persistent connection from a non-persistent server. TCPServer will only serve a single request and will shutdown and close the socket once the response has been written.

The reason that you're seeing different responses sometimes (varying between BadStatusLine and BrokenPipeError) is because of an understandable race condition between the client sending the requests and the server fully shutting down the socket and the client receiving FIN.

After digging into this, I'm not sure that there is a better way of handling this case. This exception can occur whether the client has issued a request prior to cleaning up and is expecting a response, or the server is simply misbehaving and sends an invalid status line (i.e. change your response code to an empty string to see what I mean).

I'll do some further digging, but I don't believe that there's really a good way to determine whether the BadStatusLine is due to a misbehaving server (sending a non-conforming response) or a closed socket. Considering that the client potentially has no way of knowing whether or not a server socket has been closed (in the case of TCPServer, it does a SHUT_WR), I think that BadStatusLine may be the appropriate exception to use here and the resulting action would have to be left up to the client implementation, such as in xmlrpc.client.
msg234316 - (view) Author: Demian Brecht (demian.brecht) * Date: 2015-01-19 16:37
> I'm not sure whether or not this was your intention, but your example demonstrates a misbehaving client, one that seems to expect a persistent connection from a non-persistent server. TCPServer will only serve a single request and will shutdown and close the socket once the response has been written.

Sorry, I mis-worded that. I'm /assuming/ that the misbehaving client is what you were intending on demonstrating as it shows the server closing the connection before the client expects it to do so.
msg234330 - (view) Author: Martin Panter (vadmium) * Date: 2015-01-19 23:51
Hi Demian, my intention is to demonstrate normal usage of Python’s HTTP client, whether or not its implementation misbehaves. I am trying to demonstrate a valid persistent server that happens to decide to close the connection after the first request but before reading a second request. Quoting the original post: “Servers may close a persistent connection after a request due to a timeout or other reason.” I am attaching a second demo script which includes short sleep() calls to emulate a period of time elapsing and the server timing out the connection, which is common for real-world servers.

The new script also avoids the EPIPE race by waiting until the server has definitely shut down the socket, and also demonstrates ECONNRESET. However this synchronization is artificial: in the real world the particular failure mode (BadStatusLine/EPIPE/ECONNRESET) may be uncertain.

If you are worried about detecting a misbehaving server that closes the connection before even responding to the first request, perhaps the HTTPConnection class could maintain a flag and handle the closed connection differently if it has not already seen a complete response.

If you are worried about detecting a misbehaving server that sends an empty status line without closing the connection, there will still be a newline code received. This is already handled separately by existing code: Lib/http/client.py:210 versus Lib/http/client.py:223.

I think there should be a separate exception, say called ConnectionClosed, for when the “status line” is an empty string (""), which is caused by reading the end of the stream. This is valid HTTP behaviour for the second and subsequent requests, so the HTTP library should understand it. BadStatusLine is documented for “status codes we don’t understand”. The new ConnectionClosed exception should probably be a subclass of BadStatusLine for backwards compatibility.

A further enhancement could be to wrap any ConnectionError during the request() stage, or first part of the getresponse() stage, in the same ConnectionClosed exception. Alternatively, the new ConnectionClosed exception could subclass both BadStatusLine and ConnectionError. Either way, code like the XML-RPC client could be simplified to:

try:
    return self.single_request(...)
except http.client.ConnectionClosed:
#except ConnectionError:  # Alternative
    #retry request once if cached connection has gone cold
    return self.single_request(...)
msg234331 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-01-19 23:55
FWIW, I agree with the analysis here, its standard HTTP behaviour in the real world, and we should indeed handle it.
msg234335 - (view) Author: Demian Brecht (demian.brecht) * Date: 2015-01-20 01:55
Sorry Martin, I should really not dig into issues like this first thing in the morning ;)

My concern about the proposed change isn't whether or not it isn't valid HTTP behaviour, it is. My concern (albeit a small one) is that the change implies an assumption that may not necessarily be true. No matter how valid based on the HTTP spec, it's still an assumption that /can/ potentially lead to confusion. I do agree that a change /should/ be made, I just want to make sure that all potential cases are considered before implementing one.

For example, applying the following patch to the first attachment:

52,57c52,53
<         self.wfile.write(
<             b"HTTP/1.1 200 Dropping connection\r\n"
<             b"Content-Length: 0\r\n"
<             b"\r\n"
<         )
<
---
>         self.wfile.write(b'')
>

Should the proposed change be made, the above would error out with a ConnectionClosed exception, which is invalid because the connection has not actually been closed and BadStatusLine is actually closer to being correct. Admittedly, this is a little contrived, doesn't adhere to the HTTP spec and is much less likely to happen in the real world than a connection unexpectedly closed by the server, but I don't think it's an unreasonable assumption for lesser used servers or proxies or those in development. In those cases, the proposed change would result in just as much confusion as the current behaviour with connections that are intended to be persistent.

In my mind, the one constant through both of these cases is that the response that the client has read is unexpected. In light of that, rather than a ConnectionClosed error, what about UnexpectedResponse, inheriting from BadStatusLine for backwards compatibility and documented as possibly being raised as a result of either case? I think this would cover both cases where a socket has been closed as well as an altogether invalid response.
msg234337 - (view) Author: Martin Panter (vadmium) * Date: 2015-01-20 02:28
Calling self.wfile.write(b"") should be equivalent to not calling write() at all, as far as I understand. Using strace, it does not seem to invoke send() at all. So the result will depend on what is written next. In the case of my code, nothing is written next; the connection is shut down instead. So I don’t believe this case is any different from “a connection unexpectedly closed by the server”. To be clear, I think the situation we are talking about is:

1. Client connects to server and sends short request; server accepts connection and possibly reads request
2. Server does not write any response, or just calls write(b""), which is equivalent
3. Server shuts down connection
4. Client reads end of stream (b"") instead of proper status line

But to address your concern in any case, see the third paragram in <https://bugs.python.org/issue3566#msg234330>. I propose some internal flag like HTTPConnection._initial_response, that gets set to False after the first proper response is received. Then the code could be changed to something like:

if not line:
    # Presumably, the server closed the connection before
    # sending a valid response.
    if self._initial_response:
        raise BadStatusLine(line)
    else:
        raise ConnectionClosed("Stream ended before receiving response")
msg234350 - (view) Author: Demian Brecht (demian.brecht) * Date: 2015-01-20 05:42
> Calling self.wfile.write(b"") should be equivalent to not calling write() at all, as far as I understand.

Right (or at least, as I understand it as well).

Really, this boils down to a philosophical debate: Should the standard library account for unexpected conditions where possible (within reason of course), or should it only account for conditions as described by specifications?

> 1. Client connects to server and sends short request; server accepts connection and possibly reads request
> [snip]

This flow makes sense and is well accounted for with your proposed change. However, misbehaving cases such as:

1. Client connects to server or proxy (possibly through a tunnel) and sends request; server/proxy accepts connection and possibly reads request
2. Server/proxy intends to send a response, but doesn't for any number of reasons (bug, still in development, etc)
3. The connection is not closed and subsequent requests may succeed

Granted, the result is unexpected and doesn't comply with HTTP RFCs. However, leading the user to believe that the connection has been closed when it actually hasn't is misleading. I've spent many an hour trying to hunt down root causes of issues like this and bashed my head against a keyboard in disbelief when I found out what the cause /really/ was. Because of those headaches, I still think that the introduction of an UnexpectedResponse, if well documented, covers both cases nicely, but won't heatedly argue it further :) If others (namely core devs) think that the introduction of ConnectionClosed exception is a better way to go, then I'll bow out. It would maybe be nice to have Senthil chime in on this.


> But to address your concern in any case, see the third paragram in <https://bugs.python.org/issue3566#msg234330>.

I don't think that should be added at all as the issue that I'm describing can occur at any point, not only the first request.


On another note, were you interested in submitting a patch for this?
msg234354 - (view) Author: Martin Panter (vadmium) * Date: 2015-01-20 06:22
Yeah I’m happy to put a patch together, once I have an idea of the details.

I’d also like to understand your scenario that would mislead the user to believe that the connection has been closed when it actually hasn’t. Can you give a concrete example or demonstration?

Given your misbehaving flow:

1. Client connects to server or proxy (possibly through a tunnel) and sends request; server/proxy accepts connection and possibly reads request
2. Server/proxy intends to send a response, but doesn't for any number of reasons (bug, still in development, etc)
3. The connection is not closed and subsequent requests may succeed

I would expect the client would still be waiting to read the status line of the response that was never sent in step 2. Eventually the server _would_ probably drop the connection (so ConnectionClosed would not be misleading), or the client would time it out (a different error would be raised).
msg234355 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-01-20 06:43
I think that in other stdlib networking modules, a connection closed error is raised when an operation is attempted on a closed connection.  For example, in smtplib, the server may return an error code and then (contrary to the RFC) close the connection.  We fixed a bug in smtplib where this was mishandled (the error code was lost and SMTPServerDisconnected was raised instead).  Now we return the error code, and the *next* operation on the connection gets the connection closed error.

I think this is a good model, but I'm not sure if/how it can be applied here.  That is, if the connection has been closed, I think an operation attempted on the connection should raise an exception that makes it clear that the connection is closed (in the case of some stdlib modules, that may be a socket level exception for the operation on the closed socket).

I think the remote server writing a blank line to the socket is a very different thing from the remote server closing the connection without writing anything, so I may be misunderstanding something here.  Note that handling this is potentially more complicated with https, since in that case we have a wrapper around the socket communication that has some buffering involved.

But also note that if a new exception is introduced this becomes a feature and by our normal policies can only go into 3.5.
msg234379 - (view) Author: Demian Brecht (demian.brecht) * Date: 2015-01-20 17:10
TL;DR: Because HTTP is an application-level protocol, it's nearly impossible to gauge how a server will behave given a set of conditions. Because of that, any time that assumptions can be avoided, they should be.


@R. David Murray:

> That is, if the connection has been closed, I think an operation attempted on the connection should raise an exception that makes it clear that the connection is closed (in the case of some stdlib modules, that may be a socket level exception for the operation on the closed socket).

In the typical case, this is exactly what happens. This issue is around a race condition that can occur between the client issuing a request prior to terminating the connection with the server, but the server terminating it prior to processing the request. In these cases, the client is left in a state where as far as it's concerned, it's in a valid state waiting for a response which the server will not issue as it has closed the socket on its side. In this case, the client reading an empty string from the receive buffer implies a closed socket. Unfortunately, it's not entirely uncommon when using persistent connections, as Martin's examples demonstrate.

> I think the remote server writing a blank line to the socket is a very different thing from the remote server closing the connection without writing anything, so I may be misunderstanding something here.

+1. What Martin is arguing here (Martin, please correct me if I'm wrong) is that a decently behaved server should not, at /any/ time write a blank line to (or effectively no-op) the socket, other than in the case where the socket connection has been closed. While I agree in the typical case, a blend of Postel and Murphy's laws leads me to believe it would be better to expect, accept and handle the unexpected.


@Martin:

Here's a concrete example of the unexpected behaviour. It's not specific to persistent connections and would be caught by the proposed "first request" solution, but ultimately, similar behaviour may be seen at any time from other servers/sources: http://stackoverflow.com/questions/22813645/options-http-methods-gives-an-empty-response-on-heroku.

Googling for "http empty response" and similar search strings should also provide a number of examples where unexpected behaviour is encountered and in which case raising an explicit "ConnectionClosed" error would add to the confusion.

Other examples are really hypotheticals and I don't think it's worth digging into them too deeply here. Unexpected behaviour (regardless of whether it's on the first or Nth request) should be captured well enough by now.

> Eventually the server _would_ probably drop the connection (so ConnectionClosed would not be misleading)

Sure, but you're raising an exception based on future /expected/ behaviour. That's my issue with the proposed solution in general. ConnectionClosed assumes specific behaviour, where literally /anything/ can happen server side.
msg234381 - (view) Author: Demian Brecht (demian.brecht) * Date: 2015-01-20 18:10
Now I think I'd like to take my foot out of my mouth.

Previous quick experiments that I had done were at the socket level, circumventing some of the logic in the HTTPResponse, mainly the calls to readline() rather than simple socket.recv(<N>).

I've confirmed that the /only/ way that the HTTPConnection object can possibly get a 0-length read is, in fact, if the remote host has closed the connection.

In light of that, I have no objection at all to the suggested addition of ConnectionClosed exception and my apologies for the added confusion and dragging this issue on much longer than it should have been.

I've also attached my proof of concept code for posterity.
msg234383 - (view) Author: Demian Brecht (demian.brecht) * Date: 2015-01-20 19:15
(Admittedly, I may also have been doing something entirely invalid in previous experiments as well)
msg234597 - (view) Author: Martin Panter (vadmium) * Date: 2015-01-24 08:42
Here is a patch, including tests and documentation. It ended up a bit more complicated than I anticipated, so I’m interested in hearing other ideas or options.

* Added http.client.ConnectionClosed exception
* HTTPConnection.close() is implicitly called for a persistent connection closure
* BadStatusLine or ConnectionError (rather than new exception) is still raised on first getresponse()
* request() raising a ConnectionError does not necessarily mean the server did not send a response, so ConnectionClosed is only raised by getresponse()
* ConnectionClosed wraps ECONNRESET from the first recv() of the status line, but not after part of the status line has already been received

With this I hope code for making idempotent requests on a persistent connection would look a bit like this:

def idempotent_request(connection)
    try:
        attempt_request(connection, ...)
        response = connection.get_response()
        if response.status == HTTPStatus.REQUEST_TIMEOUT:
            raise ConnectionClosed(response.reason)
    except ConnectionClosed:
        attempt_request(connection, ...)
        response = connection.get_response()
    return response

def attempt_request(connection):
    try:
        connection.request(...)
    except ConnectionError:
        pass  # Ignore and read server response
msg234643 - (view) Author: Martin Panter (vadmium) * Date: 2015-01-25 00:52
Updated v2 patch. This version avoids intruding into the HTTPConnection.connect() implementation, so that users, tests, etc may still set the undocumented “sock” attribute without calling the base connect() method. Also code style changes based on feedback to another of my patches.
msg234656 - (view) Author: Demian Brecht (demian.brecht) * Date: 2015-01-25 07:08
Thanks for the patch Martin (as well as catching a couple issues that I'd run into recently as well). I've left a couple comments up on Rietveld.
msg234710 - (view) Author: Martin Panter (vadmium) * Date: 2015-01-26 01:11
Thanks for the reviews.

I agree about the new HTTPResponse flag being a bit awkward; the HTTPResponse class should probably raise the ConnectionClosed exception in all cases. I was wondering if the the HTTPConnection class should wrap this in a PersistentConnectionClosed exception or something if it wasn’t for the first connection, now I’m thinking that should be up to the higher level user, in case they are doing something like HTTP preconnection.
msg234749 - (view) Author: Demian Brecht (demian.brecht) * Date: 2015-01-26 15:37
> now I’m thinking that should be up to the higher level user

+1. A closed connection is a closed connection, whether it's persistent or not. The higher level code should be responsible for the context, not the connection level.
msg235106 - (view) Author: Martin Panter (vadmium) * Date: 2015-01-31 12:37
I have changed my opinion of the “peek hack” from <https://bugs.python.org/issue3566#msg231413>. It would be useful when doing non-idempotent requests like POST, to avoid sending a request when we know it is going to fail. I looked into how to implement it so that it works for SSL (which does not support MSG_PEEK), and the neatest solution I could think of would require changing the non-blocking behaviour of BufferedReader.peek(), as described in Issue 13322. So I will leave that for later.

Adding ConnectionClosed.v3.patch; main changes:

* Removed the connection_reused flag to HTTPResponse
* ConnectionClosed raised even for the first request of a connection
* Added HTTPConnection.closed flag, which the user may check before a request to see if a fresh connection will be made, or an existing connection will be reused
* ConnectionClosed now subclasses both BadStatusLine and ConnectionError
* Fixed http.client.__all__ and added a somewhat automated test for it

BTW these patches kind of depend on Issue 5811 to confirm that BufferedReader.peek() will definitely return at least one byte unless at EOF.
History
Date User Action Args
2015-01-31 12:38:02vadmiumsetfiles: + ConnectionClosed.v3.patch

messages: + msg235106
2015-01-26 15:37:12demian.brechtsetmessages: + msg234749
2015-01-26 01:11:22vadmiumsetmessages: + msg234710
2015-01-25 07:08:43demian.brechtsetmessages: + msg234656
2015-01-25 00:52:01vadmiumsetfiles: + ConnectionClosed.v2.patch

messages: + msg234643
versions: - Python 2.7, Python 3.4
2015-01-24 08:42:50vadmiumsetfiles: + ConnectionClosed.patch
keywords: + patch
messages: + msg234597
2015-01-20 19:15:22demian.brechtsetmessages: + msg234383
2015-01-20 18:10:21demian.brechtsetfiles: + zero_response_poc.py

messages: + msg234381
2015-01-20 17:10:56demian.brechtsetmessages: + msg234379
2015-01-20 06:43:45r.david.murraysetmessages: + msg234355
versions: + Python 3.4, Python 3.5, - Python 3.3
2015-01-20 06:22:45vadmiumsetmessages: + msg234354
2015-01-20 05:43:57gregory.p.smithsetnosy: - gregory.p.smith
2015-01-20 05:42:24demian.brechtsetmessages: + msg234350
2015-01-20 02:28:30vadmiumsetmessages: + msg234337
2015-01-20 01:55:43demian.brechtsetmessages: + msg234335
2015-01-19 23:55:03rbcollinssetnosy: + rbcollins
messages: + msg234331
2015-01-19 23:51:48vadmiumsetfiles: + persistent-closing.py

messages: + msg234330
2015-01-19 16:37:13demian.brechtsetmessages: + msg234316
2015-01-19 16:34:37demian.brechtsetmessages: + msg234315
2015-01-18 23:55:10vadmiumsetmessages: + msg234294
2015-01-17 13:50:23r.david.murraysetnosy: + r.david.murray
2015-01-17 06:26:41vadmiumsetfiles: + persistent-closing.py

messages: + msg234163
2015-01-16 02:20:33vadmiumsetmessages: + msg234108
2015-01-16 02:00:54demian.brechtsetmessages: + msg234107
2014-11-20 04:37:42vadmiumsetmessages: + msg231413
2014-07-25 14:54:59icordascsetnosy: + icordasc
2014-07-24 00:20:22demian.brechtsetnosy: + demian.brecht
2013-12-20 02:38:03alanjdssetnosy: + alanjds

messages: + msg206661
versions: + Python 2.7
2013-11-01 03:00:48vadmiumsetnosy: + vadmium
2013-09-11 14:20:41Yuri.Bochkarevsetnosy: + Yuri.Bochkarev
2012-02-01 02:27:14jceasetnosy: + jcea

versions: + Python 3.3, - Python 3.1, Python 2.7, Python 3.2
2011-06-06 12:27:22agriffissetnosy: + agriffis
2011-05-09 15:40:05amaksetnosy: + amak
2011-03-19 22:35:08orsenthilsetassignee: orsenthil
2010-12-20 00:04:11orsenthilsetnosy: + orsenthil
2010-12-19 22:05:27gregory.p.smithsetassignee: gregory.p.smith -> (no value)
messages: + msg124366
2010-08-03 21:37:33terry.reedysetversions: + Python 3.1, Python 2.7, Python 3.2, - Python 2.5
2009-08-18 11:32:23jhyltonsetnosy: + jhylton
messages: + msg91681
2009-08-16 22:24:57gregory.p.smithsetpriority: normal

messages: + msg91650
2008-09-22 01:23:24gregory.p.smithsetassignee: gregory.p.smith
nosy: + gregory.p.smith
2008-08-23 13:44:57cananiansetmessages: + msg71806
2008-08-23 01:55:35mhammondsetnosy: + mhammond
2008-08-16 15:41:19cananiancreate