From: Serhiy Storchaka <storchaka@gmail.com>
Date: Wed, 6 Feb 2013 08:35:40 +0000 (+0200)
Subject: Issue #16723: httplib.HTTPResponse no longer marked closed when the connection
X-Git-Tag: v3.3.1rc1~219
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b6c86fd87f93f73c8d5f759d3f5136f1a1cc67e9;p=python

Issue #16723: httplib.HTTPResponse no longer marked closed when the connection
is automatically closed.
---

b6c86fd87f93f73c8d5f759d3f5136f1a1cc67e9
diff --cc Lib/http/client.py
index 62d9cff889,5466d0618d..4663d439e3
--- a/Lib/http/client.py
+++ b/Lib/http/client.py
@@@ -490,157 -486,103 +494,157 @@@ class HTTPResponse(io.RawIOBase)
              return b""
  
          if self._method == "HEAD":
-             self.close()
+             self._close_conn()
              return b""
  
 -        if self.chunked:
 -            return self._read_chunked(amt)
 +        if amt is not None:
 +            # Amount is given, so call base class version
 +            # (which is implemented in terms of self.readinto)
 +            return super(HTTPResponse, self).read(amt)
 +        else:
 +            # Amount is not given (unbounded read) so we must check self.length
 +            # and self.chunked
 +
 +            if self.chunked:
 +                return self._readall_chunked()
  
 -        if amt is None:
 -            # unbounded read
              if self.length is None:
                  s = self.fp.read()
              else:
                  try:
                      s = self._safe_read(self.length)
                  except IncompleteRead:
-                     self.close()
+                     self._close_conn()
                      raise
                  self.length = 0
-             self.close()        # we read everything
+             self._close_conn()        # we read everything
              return s
  
 +    def readinto(self, b):
 +        if self.fp is None:
 +            return 0
 +
 +        if self._method == "HEAD":
-             self.close()
++            self._close_conn()
 +            return 0
 +
 +        if self.chunked:
 +            return self._readinto_chunked(b)
 +
          if self.length is not None:
 -            if amt > self.length:
 +            if len(b) > self.length:
                  # clip the read to the "end of response"
 -                amt = self.length
 +                b = memoryview(b)[0:self.length]
  
          # we do not use _safe_read() here because this may be a .will_close
          # connection, and the user is reading more bytes than will be provided
          # (for example, reading in 1k chunks)
 -        s = self.fp.read(amt)
 -        if not s:
 +        n = self.fp.readinto(b)
 +        if not n:
              # Ideally, we would raise IncompleteRead if the content-length
              # wasn't satisfied, but it might break compatibility.
-             self.close()
+             self._close_conn()
          elif self.length is not None:
 -            self.length -= len(s)
 +            self.length -= n
              if not self.length:
-                 self.close()
+                 self._close_conn()
 +        return n
  
 -        return s
 +    def _read_next_chunk_size(self):
 +        # Read the next chunk size from the file
 +        line = self.fp.readline(_MAXLINE + 1)
 +        if len(line) > _MAXLINE:
 +            raise LineTooLong("chunk size")
 +        i = line.find(b";")
 +        if i >= 0:
 +            line = line[:i] # strip chunk-extensions
 +        try:
 +            return int(line, 16)
 +        except ValueError:
 +            # close the connection as protocol synchronisation is
 +            # probably lost
-             self.close()
++            self._close_conn()
 +            raise
  
 -    def _read_chunked(self, amt):
 +    def _read_and_discard_trailer(self):
 +        # read and discard trailer up to the CRLF terminator
 +        ### note: we shouldn't have any trailers!
 +        while True:
 +            line = self.fp.readline(_MAXLINE + 1)
 +            if len(line) > _MAXLINE:
 +                raise LineTooLong("trailer line")
 +            if not line:
 +                # a vanishingly small number of sites EOF without
 +                # sending the trailer
 +                break
 +            if line in (b'\r\n', b'\n', b''):
 +                break
 +
 +    def _readall_chunked(self):
          assert self.chunked != _UNKNOWN
          chunk_left = self.chunk_left
          value = []
          while True:
              if chunk_left is None:
 -                line = self.fp.readline(_MAXLINE + 1)
 -                if len(line) > _MAXLINE:
 -                    raise LineTooLong("chunk size")
 -                i = line.find(b";")
 -                if i >= 0:
 -                    line = line[:i] # strip chunk-extensions
                  try:
 -                    chunk_left = int(line, 16)
 +                    chunk_left = self._read_next_chunk_size()
 +                    if chunk_left == 0:
 +                        break
                  except ValueError:
 -                    # close the connection as protocol synchronisation is
 -                    # probably lost
 -                    self._close_conn()
                      raise IncompleteRead(b''.join(value))
 -                if chunk_left == 0:
 -                    break
 -            if amt is None:
 -                value.append(self._safe_read(chunk_left))
 -            elif amt < chunk_left:
 -                value.append(self._safe_read(amt))
 -                self.chunk_left = chunk_left - amt
 -                return b''.join(value)
 -            elif amt == chunk_left:
 -                value.append(self._safe_read(amt))
 +            value.append(self._safe_read(chunk_left))
 +
 +            # we read the whole chunk, get another
 +            self._safe_read(2)      # toss the CRLF at the end of the chunk
 +            chunk_left = None
 +
 +        self._read_and_discard_trailer()
 +
 +        # we read everything; close the "file"
-         self.close()
++        self._close_conn()
 +
 +        return b''.join(value)
 +
 +    def _readinto_chunked(self, b):
 +        assert self.chunked != _UNKNOWN
 +        chunk_left = self.chunk_left
 +
 +        total_bytes = 0
 +        mvb = memoryview(b)
 +        while True:
 +            if chunk_left is None:
 +                try:
 +                    chunk_left = self._read_next_chunk_size()
 +                    if chunk_left == 0:
 +                        break
 +                except ValueError:
 +                    raise IncompleteRead(bytes(b[0:total_bytes]))
 +
 +            if len(mvb) < chunk_left:
 +                n = self._safe_readinto(mvb)
 +                self.chunk_left = chunk_left - n
 +                return total_bytes + n
 +            elif len(mvb) == chunk_left:
 +                n = self._safe_readinto(mvb)
                  self._safe_read(2)  # toss the CRLF at the end of the chunk
                  self.chunk_left = None
 -                return b''.join(value)
 +                return total_bytes + n
              else:
 -                value.append(self._safe_read(chunk_left))
 -                amt -= chunk_left
 +                temp_mvb = mvb[0:chunk_left]
 +                n = self._safe_readinto(temp_mvb)
 +                mvb = mvb[n:]
 +                total_bytes += n
  
              # we read the whole chunk, get another
              self._safe_read(2)      # toss the CRLF at the end of the chunk
              chunk_left = None
  
 -        # read and discard trailer up to the CRLF terminator
 -        ### note: we shouldn't have any trailers!
 -        while True:
 -            line = self.fp.readline(_MAXLINE + 1)
 -            if len(line) > _MAXLINE:
 -                raise LineTooLong("trailer line")
 -            if not line:
 -                # a vanishingly small number of sites EOF without
 -                # sending the trailer
 -                break
 -            if line in (b'\r\n', b'\n', b''):
 -                break
 +        self._read_and_discard_trailer()
  
          # we read everything; close the "file"
-         self.close()
+         self._close_conn()
  
 -        return b''.join(value)
 +        return total_bytes
  
      def _safe_read(self, amt):
          """Read the number of bytes requested, compensating for partial reads.
diff --cc Lib/test/test_httplib.py
index a58a592476,420302c332..db123dcb56
--- a/Lib/test/test_httplib.py
+++ b/Lib/test/test_httplib.py
@@@ -185,24 -188,10 +188,30 @@@ class BasicTest(TestCase)
          self.assertFalse(resp.isclosed())
          self.assertEqual(resp.read(2), b'xt')
          self.assertTrue(resp.isclosed())
+         self.assertFalse(resp.closed)
+         resp.close()
+         self.assertTrue(resp.closed)
  
 +    def test_partial_readintos(self):
 +        # if we have a length, the system knows when to close itself
 +        # same behaviour than when we read the whole thing with read()
 +        body = "HTTP/1.1 200 Ok\r\nContent-Length: 4\r\n\r\nText"
 +        sock = FakeSocket(body)
 +        resp = client.HTTPResponse(sock)
 +        resp.begin()
 +        b = bytearray(2)
 +        n = resp.readinto(b)
 +        self.assertEqual(n, 2)
 +        self.assertEqual(bytes(b), b'Te')
 +        self.assertFalse(resp.isclosed())
 +        n = resp.readinto(b)
 +        self.assertEqual(n, 2)
 +        self.assertEqual(bytes(b), b'xt')
 +        self.assertTrue(resp.isclosed())
++        self.assertFalse(resp.closed)
++        resp.close()
++        self.assertTrue(resp.closed)
 +
      def test_partial_reads_no_content_length(self):
          # when no length is present, the socket should be gracefully closed when
          # all data was read
@@@ -215,26 -204,10 +224,29 @@@
          self.assertEqual(resp.read(2), b'xt')
          self.assertEqual(resp.read(1), b'')
          self.assertTrue(resp.isclosed())
+         self.assertFalse(resp.closed)
+         resp.close()
+         self.assertTrue(resp.closed)
  
 +    def test_partial_readintos_no_content_length(self):
 +        # when no length is present, the socket should be gracefully closed when
 +        # all data was read
 +        body = "HTTP/1.1 200 Ok\r\n\r\nText"
 +        sock = FakeSocket(body)
 +        resp = client.HTTPResponse(sock)
 +        resp.begin()
 +        b = bytearray(2)
 +        n = resp.readinto(b)
 +        self.assertEqual(n, 2)
 +        self.assertEqual(bytes(b), b'Te')
 +        self.assertFalse(resp.isclosed())
 +        n = resp.readinto(b)
 +        self.assertEqual(n, 2)
 +        self.assertEqual(bytes(b), b'xt')
 +        n = resp.readinto(b)
 +        self.assertEqual(n, 0)
 +        self.assertTrue(resp.isclosed())
 +
      def test_partial_reads_incomplete_body(self):
          # if the server shuts down the connection before the whole
          # content-length is delivered, the socket is gracefully closed
@@@ -247,25 -220,9 +259,28 @@@
          self.assertEqual(resp.read(2), b'xt')
          self.assertEqual(resp.read(1), b'')
          self.assertTrue(resp.isclosed())
 +
 +    def test_partial_readintos_incomplete_body(self):
 +        # if the server shuts down the connection before the whole
 +        # content-length is delivered, the socket is gracefully closed
 +        body = "HTTP/1.1 200 Ok\r\nContent-Length: 10\r\n\r\nText"
 +        sock = FakeSocket(body)
 +        resp = client.HTTPResponse(sock)
 +        resp.begin()
 +        b = bytearray(2)
 +        n = resp.readinto(b)
 +        self.assertEqual(n, 2)
 +        self.assertEqual(bytes(b), b'Te')
 +        self.assertFalse(resp.isclosed())
 +        n = resp.readinto(b)
 +        self.assertEqual(n, 2)
 +        self.assertEqual(bytes(b), b'xt')
 +        n = resp.readinto(b)
 +        self.assertEqual(n, 0)
 +        self.assertTrue(resp.isclosed())
+         self.assertFalse(resp.closed)
+         resp.close()
+         self.assertTrue(resp.closed)
  
      def test_host_port(self):
          # Check invalid host_port
@@@ -493,27 -367,10 +508,33 @@@
          self.assertEqual(resp.status, 200)
          self.assertEqual(resp.reason, 'OK')
          self.assertTrue(resp.isclosed())
+         self.assertFalse(resp.closed)
+         resp.close()
+         self.assertTrue(resp.closed)
  
 +    def test_readinto_chunked_head(self):
 +        chunked_start = (
 +            'HTTP/1.1 200 OK\r\n'
 +            'Transfer-Encoding: chunked\r\n\r\n'
 +            'a\r\n'
 +            'hello world\r\n'
 +            '1\r\n'
 +            'd\r\n'
 +        )
 +        sock = FakeSocket(chunked_start + '0\r\n')
 +        resp = client.HTTPResponse(sock, method="HEAD")
 +        resp.begin()
 +        b = bytearray(5)
 +        n = resp.readinto(b)
 +        self.assertEqual(n, 0)
 +        self.assertEqual(bytes(b), b'\x00'*5)
 +        self.assertEqual(resp.status, 200)
 +        self.assertEqual(resp.reason, 'OK')
 +        self.assertTrue(resp.isclosed())
++        self.assertFalse(resp.closed)
++        resp.close()
++        self.assertTrue(resp.closed)
 +
      def test_negative_content_length(self):
          sock = FakeSocket(
              'HTTP/1.1 200 OK\r\nContent-Length: -1\r\n\r\nHello\r\n')