GitLab

Bug fix candidate.

Kevin Jacobs found that the code simplification did not
exactly match the semantics of the original.  Regression
test cases were requested.

* Replaced "while 1" with "while True"
* Rewrote read() and readline() for clarity and speed.
* Replaced variable 'list' with 'hlist'
* Used augmented assignment in two places.

The buggy yahoo server was fixed, and the opalgroup test files are
gone.

Also, remove unused local variable noted by pychecker.

along with some small changes (e.g. use of +=).

If the transfer-encoding is unknown, ignore it.  Suggested by Tom
Emerson.

The implementation now stores all the lines of the request in a buffer
and makes a single send() call when the request is finished,
specifically when endheaders() is called.

This appears to improve performance.  The old code called send() for
each line.  The sends are all short, so they caused bad interactions
with the Nagle algorithm and delayed acknowledgements.  In simple
tests, the second packet was delayed by 100s of ms.  The second send was
delayed by the Nagle algorithm, waiting for the ack.  The delayed ack
strategy delays the ack in hopes of piggybacking it on a data packet,
but the server won't send any data until it receives the complete
request.

This change minimizes the problem that Nagle + delayed ack will cause
a problem, although a request large enough to be broken into two
packets will still suffer some delay.  Luckily the MSS is large enough
to accomodate most single packets.

XXX Bug fix candidate?

Client code could create responses explicitly.

The recent SSL changes resulted in important, but subtle changes to
close() semantics.  Since builtin socket makefile() is not called for
SSL connections, we don't get separately closeable fds for connection
and response.  Comments in the code explain how to restore makefile
semantics.

Bug fix candidate.

If multiple header fields with the same name occur, they are combined
according to the rules in RFC 2616 sec 4.2:

Appending each subsequent field-value to the first, each separated by
a comma. The order in which header fields with the same field-name are
received is significant to the interpretation of the combined field
value.

Subclasses of Exception that define an __init__ must call
Exception.__init__ or define self.args.  Otherwise, str() will fail.

Bug fix candidate.

Section 19.6 of RFC 2616 (HTTP/1.1):

   It is beyond the scope of a protocol specification to mandate
   compliance with previous versions. HTTP/1.1 was deliberately
   designed, however, to make supporting previous versions easy....

   And we would expect HTTP/1.1 clients to:

      - recognize the format of the Status-Line for HTTP/1.0 and 1.1
        responses;

      - understand any valid response in the format of HTTP/0.9, 1.0, or
        1.1.

The changes to the code do handle response in the format of HTTP/0.9.
Some users may consider this a bug because all responses with a
sufficiently corrupted status line will look like an HTTP/0.9
response.  These users can pass strict=1 to the HTTP constructors to
get a BadStatusLine exception instead.

While this is a new feature of sorts, it enhances the robustness of
the code (be tolerant in what you accept).  Thus, I consider it a bug
fix candidate.

XXX strict needs to be documented.

See discussion in SF bug 458463.

The HTTPResponse class now handles 100 continue responses, instead of
choking on them.  It detects them internally in the _begin() method
and ignores them.  Based on a patch by Bob Kline.

This closes SF bugs 498149 and 551273.

The FakeSocket class (for SSL) is now usable with HTTP/1.1
connections.  The old version of the code could not work with
persistent connections, because the makefile() implementation read
until EOF before returning.  If the connection is persistent, the
server sends a response and leaves the connection open.  A client that
reads until EOF will block until the server gives up on the connection
-- more than a minute in my test case.

The problem was fixed by implementing a reasonable makefile().  It
reads data only when it is needed by the layers above it.  It's
implementation uses an internal buffer with a default size of 8192.

Also, rename begin() method of HTTPResponse to _begin() because it
should only be called by the HTTPConnection.

Fixes #494762.

FakeSocket class.  Without it, the sendall() call got the method on
the underlying socket object, and that messed up SSL.

Does httplib use other methods of sockets that FakeSocket doesn't support?
Someone should take a look...  (I'll try to give it a once-over.)

2.2.1 bugfix candidate.

Don't automatically add a Host: header if the headers passed to
request() already has a Host key.

In August, Greg said this looked good, so I'm going ahead with it.

The fix is different from the one in the bug report.  Instead of using
a regular expression to extract the host from the url, I use
urlparse.urlsplit.

Martin commented that the patch doesn't address URLs that have basic
authentication username and password in the header.  I don't see any
code anywhere in httplib that supports this feature, so I'm not going
to address it for this fix.

Bug fix candidate.

Replaces calls to socket.send() (which isn't guaranteed to send all data)
with the new socket.sendall() method.

SyntaxError.  Fix it.

Try to be systematic about dealing with socket and ssl exceptions in
FakeSocket.makefile().  The previous version of the code caught all
ssl errors and treated them as EOF, even though most of the errors
don't mean EOF.

An SSL error can mean on of three things:

    1. The SSL/TLS connection was closed.
    2. The operation should be retried.
    3. An error occurred.

Also, if a socket error occurred and the error was EINTR, retry the
call.  Otherwise, it was a legitimate error and the caller should
receive the exception.

For the HTTPS class (when available), ensure that the x509 certificate data
gets passed through to the HTTPSConnection class. Create a new
HTTPS.__init__ to do this, and refactor the HTTP.__init__ into a new _setup
method for both init's to call.

Note: this is solved differently from the patch, which advocated a new
**x509 parameter on the base HTTPConnection class. But that would open
HTTPConnection to arbitrary (ignored) parameters, so was not as desirable.