openstack/deb-python-eventlet
Code Issues Proposed changes

wsgi: Fix handling partial writes on Python 3

Browse Source 
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.
This commit is contained in:
Jakub Stasiak
2016-02-09 02:24:47 +01:00
parent 8a4a1212b2
commit b7380fdc70
4 changed files with 88 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
eventlet/support/__init__.py
View File

@@ -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:])

21
eventlet/wsgi.py
View File

@@ -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()")

1
tests/isolated/wsgi_connection_timeout.py
View File

@@ -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

57
tests/wsgi_test.py
View File

@@ -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):