wsgi: Fix handling partial writes on Python 3partial-write-fix-2

Closes https://github.com/eventlet/eventlet/issues/295 (in the wsgi
module we use a custom writelines implementation now).

Those write() calls might write only part of the data (and even if they
don't - it's more readable to make sure all data is written
explicitly).

I changed the test code so that the write() implementation returns the
number of characters logged and it cooperates nicely with writeall()
now.

4 files changed, 88 insertions, 8 deletions

diff --git a/eventlet/support/__init__.py b/eventlet/support/__init__.py
index 4c2b75d..d78ac15 100644
--- a/eventlet/support/__init__.py
+++ b/eventlet/support/__init__.py
@@ -53,3 +53,20 @@ def capture_stderr():
     finally:
         sys.stderr = original
         stream.seek(0)
+
+
+def safe_writelines(fd, to_write):
+    # Standard Python 3 writelines() is not reliable because it doesn't care if it
+    # loses data. See CPython bug report: http://bugs.python.org/issue26292
+    for item in to_write:
+        writeall(fd, item)
+
+
+if six.PY2:
+    def writeall(fd, buf):
+        fd.write(buf)
+else:
+    def writeall(fd, buf):
+        written = 0
+        while written < len(buf):
+            written += fd.write(buf[written:])
diff --git a/eventlet/wsgi.py b/eventlet/wsgi.py
index 88e7752..e72fec7 100644
--- a/eventlet/wsgi.py
+++ b/eventlet/wsgi.py
@@ -1,4 +1,5 @@
 import errno
+import functools
 import os
 import sys
 import time
@@ -11,7 +12,7 @@ from eventlet.green import socket
 from eventlet import greenio
 from eventlet import greenpool
 from eventlet import support
-from eventlet.support import six
+from eventlet.support import safe_writelines, six, writeall
 
 from eventlet.support.six.moves import urllib
 
@@ -112,7 +113,7 @@ class Input(object):
         # Blank line
         towrite.append(b'\r\n')
 
-        self.wfile.writelines(towrite)
+        safe_writelines(self.wfile, towrite)
 
         # Reinitialize chunk_length (expect more data)
         self.chunk_length = -1
@@ -260,7 +261,7 @@ class LoggerFileWrapper(object):
         msg = msg + '\n'
         if args:
             msg = msg % args
-        self.log.write(msg)
+        writeall(self.log, msg)
 
 
 class FileObjectForHeaders(object):
@@ -314,7 +315,8 @@ class HttpProtocol(BaseHTTPServer.BaseHTTPRequestHandler):
         try:
             self.raw_requestline = self.rfile.readline(self.server.url_length_limit)
             if len(self.raw_requestline) == self.server.url_length_limit:
-                self.wfile.write(
+                writeall(
+                    self.wfile,
                     b"HTTP/1.0 414 Request URI Too Long\r\n"
                     b"Connection: close\r\nContent-length: 0\r\n\r\n")
                 self.close_connection = 1
@@ -336,13 +338,15 @@ class HttpProtocol(BaseHTTPServer.BaseHTTPRequestHandler):
             if not self.parse_request():
                 return
         except HeaderLineTooLong:
-            self.wfile.write(
+            writeall(
+                self.wfile,
                 b"HTTP/1.0 400 Header Line Too Long\r\n"
                 b"Connection: close\r\nContent-length: 0\r\n\r\n")
             self.close_connection = 1
             return
         except HeadersTooLarge:
-            self.wfile.write(
+            writeall(
+                self.wfile,
                 b"HTTP/1.0 400 Headers Too Large\r\n"
                 b"Connection: close\r\nContent-length: 0\r\n\r\n")
             self.close_connection = 1
@@ -355,7 +359,8 @@ class HttpProtocol(BaseHTTPServer.BaseHTTPRequestHandler):
             try:
                 int(content_length)
             except ValueError:
-                self.wfile.write(
+                writeall(
+                    self.wfile,
                     b"HTTP/1.0 400 Bad Request\r\n"
                     b"Connection: close\r\nContent-length: 0\r\n\r\n")
                 self.close_connection = 1
@@ -385,7 +390,7 @@ class HttpProtocol(BaseHTTPServer.BaseHTTPRequestHandler):
         length = [0]
         status_code = [200]
 
-        def write(data, _writelines=wfile.writelines):
+        def write(data, _writelines=functools.partial(safe_writelines, wfile)):
             towrite = []
             if not headers_set:
                 raise AssertionError("write() before start_response()")
diff --git a/tests/isolated/wsgi_connection_timeout.py b/tests/isolated/wsgi_connection_timeout.py
index 6a8c623..c2c7d3c 100644
--- a/tests/isolated/wsgi_connection_timeout.py
+++ b/tests/isolated/wsgi_connection_timeout.py
@@ -42,6 +42,7 @@ class BufferLog(object):
     @staticmethod
     def write(s):
         output_buffer.append(s.rstrip())
+        return len(s)
 
 
 # This test might make you wince
diff --git a/tests/wsgi_test.py b/tests/wsgi_test.py
index 50091eb..ef55cd9 100644
--- a/tests/wsgi_test.py
+++ b/tests/wsgi_test.py
@@ -10,6 +10,8 @@ import tempfile
 import traceback
 import unittest
 
+from nose.tools import eq_
+
 import eventlet
 from eventlet import debug
 from eventlet import event
@@ -444,6 +446,61 @@ class TestHttpd(_TestBase):
         # Require a CRLF to close the message body
         self.assertEqual(response, b'\r\n')
 
+    def test_partial_writes_are_handled(self):
+        # The bug was caused by the default writelines() implementaiton
+        # (used by the wsgi module) which doesn't check if write()
+        # successfully completed sending *all* data therefore data could be
+        # lost and the client could be left hanging forever.
+        #
+        # This test additionally ensures that plain write() calls in the wsgi
+        # are also correct now (replaced with writeare also correct now (replaced with writeall()).
+        #
+        # Eventlet issue: "Python 3: wsgi doesn't handle correctly partial
+        # write of socket send() when using writelines()",
+        # https://github.com/eventlet/eventlet/issues/295
+        #
+        # Related CPython issue: "Raw I/O writelines() broken",
+        # http://bugs.python.org/issue26292
+
+        # Custom accept() and send() in order to simulate a connection that
+        # only sends one byte at a time so that any code that doesn't handle
+        # partial writes correctly has to fail.
+        listen_socket = eventlet.listen(('localhost', 0))
+        original_accept = listen_socket.accept
+
+        def accept():
+            connection, address = original_accept()
+            original_send = connection.send
+
+            def send(b, *args):
+                if b:
+                    b = b[0:1]
+                return original_send(b, *args)
+
+            connection.send = send
+            return connection, address
+
+        listen_socket.accept = accept
+
+        def application(env, start_response):
+            # Sending content-length is important here so that the client knows
+            # exactly how many bytes does it need to wait for.
+            start_response('200 OK', [('Content-length', 3)])
+            yield 'asd'
+
+        self.spawn_server(sock=listen_socket)
+        self.site.application = application
+
+        sock = eventlet.connect(('localhost', self.port))
+
+        sock.sendall(b'GET / HTTP/1.1\r\nHost: localhost\r\n\r\n')
+
+        # This would previously hang forever
+        result = read_http(sock)
+
+        # Just to be sure we actually read what we wanted
+        eq_(result.body, b'asd')
+
     @tests.skip_if_no_ssl
     def test_012_ssl_server(self):
         def wsgi_app(environ, start_response):