wsgi: use buffered wfile
Fix the root cause of makefile().writelines() data loss. https://github.com/eventlet/eventlet/issues/295 Also wsgi.log.write can break file-object API and not return number of bytes again. https://github.com/eventlet/eventlet/issues/296
This commit is contained in:
Sergey Shepelev
@@ -53,20 +53,3 @@ 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:])
|
||||
|
||||
@@ -10,9 +10,9 @@ import warnings
|
||||
from eventlet import greenio
|
||||
from eventlet import greenpool
|
||||
from eventlet import support
|
||||
from eventlet.support import safe_writelines, six, writeall
|
||||
from eventlet.green import BaseHTTPServer
|
||||
from eventlet.green import socket
|
||||
from eventlet.support import six
|
||||
from eventlet.support.six.moves import urllib
|
||||
|
||||
|
||||
@@ -112,7 +112,8 @@ class Input(object):
|
||||
# Blank line
|
||||
towrite.append(b'\r\n')
|
||||
|
||||
safe_writelines(self.wfile, towrite)
|
||||
self.wfile.writelines(towrite)
|
||||
self.wfile.flush()
|
||||
|
||||
# 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
|
||||
writeall(self.log, msg)
|
||||
self.log.write(msg)
|
||||
|
||||
|
||||
class FileObjectForHeaders(object):
|
||||
@@ -292,6 +293,11 @@ class HttpProtocol(BaseHTTPServer.BaseHTTPRequestHandler):
|
||||
# Contrary to stdlib, it's enabled by default.
|
||||
disable_nagle_algorithm = True
|
||||
|
||||
# https://github.com/eventlet/eventlet/issues/295
|
||||
# Stdlib default is 0 (unbuffered), but then `wfile.writelines()` looses data
|
||||
# so before going back to unbuffered, remove any usage of `writelines`.
|
||||
wbufsize = 16 << 10
|
||||
|
||||
def setup(self):
|
||||
# overriding SocketServer.setup to correctly handle SSL.Connection objects
|
||||
conn = self.connection = self.request
|
||||
@@ -323,8 +329,7 @@ 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:
|
||||
writeall(
|
||||
self.wfile,
|
||||
self.wfile.write(
|
||||
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
|
||||
@@ -346,15 +351,13 @@ class HttpProtocol(BaseHTTPServer.BaseHTTPRequestHandler):
|
||||
if not self.parse_request():
|
||||
return
|
||||
except HeaderLineTooLong:
|
||||
writeall(
|
||||
self.wfile,
|
||||
self.wfile.write(
|
||||
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:
|
||||
writeall(
|
||||
self.wfile,
|
||||
self.wfile.write(
|
||||
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
|
||||
@@ -367,8 +370,7 @@ class HttpProtocol(BaseHTTPServer.BaseHTTPRequestHandler):
|
||||
try:
|
||||
int(content_length)
|
||||
except ValueError:
|
||||
writeall(
|
||||
self.wfile,
|
||||
self.wfile.write(
|
||||
b"HTTP/1.0 400 Bad Request\r\n"
|
||||
b"Connection: close\r\nContent-length: 0\r\n\r\n")
|
||||
self.close_connection = 1
|
||||
@@ -398,7 +400,7 @@ class HttpProtocol(BaseHTTPServer.BaseHTTPRequestHandler):
|
||||
length = [0]
|
||||
status_code = [200]
|
||||
|
||||
def write(data, _writelines=functools.partial(safe_writelines, wfile)):
|
||||
def write(data):
|
||||
towrite = []
|
||||
if not headers_set:
|
||||
raise AssertionError("write() before start_response()")
|
||||
@@ -447,7 +449,8 @@ class HttpProtocol(BaseHTTPServer.BaseHTTPRequestHandler):
|
||||
towrite.append(six.b("%x" % (len(data),)) + b"\r\n" + data + b"\r\n")
|
||||
else:
|
||||
towrite.append(data)
|
||||
_writelines(towrite)
|
||||
wfile.writelines(towrite)
|
||||
wfile.flush()
|
||||
length[0] = length[0] + sum(map(len, towrite))
|
||||
|
||||
def start_response(status, response_headers, exc_info=None):
|
||||
|
||||
@@ -880,6 +880,41 @@ def test_double_close_219():
|
||||
tests.run_isolated('greenio_double_close_219.py')
|
||||
|
||||
|
||||
def test_partial_write_295():
|
||||
# https://github.com/eventlet/eventlet/issues/295
|
||||
# `socket.makefile('w').writelines()` must send all
|
||||
# despite partial writes by underlying socket
|
||||
listen_socket = eventlet.listen(('localhost', 0))
|
||||
original_accept = listen_socket.accept
|
||||
|
||||
def talk(conn):
|
||||
f = conn.makefile('wb')
|
||||
line = b'*' * 2140
|
||||
f.writelines([line] * 10000)
|
||||
conn.close()
|
||||
|
||||
def accept():
|
||||
connection, address = original_accept()
|
||||
original_send = connection.send
|
||||
|
||||
def slow_send(b, *args):
|
||||
b = b[:1031]
|
||||
return original_send(b, *args)
|
||||
|
||||
connection.send = slow_send
|
||||
eventlet.spawn(talk, connection)
|
||||
return connection, address
|
||||
|
||||
listen_socket.accept = accept
|
||||
|
||||
eventlet.spawn(listen_socket.accept)
|
||||
sock = eventlet.connect(listen_socket.getsockname())
|
||||
with eventlet.Timeout(10):
|
||||
bs = sock.makefile('rb').read()
|
||||
assert len(bs) == 21400000
|
||||
assert bs == (b'*' * 21400000)
|
||||
|
||||
|
||||
def test_socket_file_read_non_int():
|
||||
listen_socket = eventlet.listen(('localhost', 0))
|
||||
|
||||
|
||||
@@ -389,17 +389,16 @@ class TestHttpd(_TestBase):
|
||||
self.assertEqual(response, b'\r\n')
|
||||
|
||||
def test_partial_writes_are_handled(self):
|
||||
# https://github.com/eventlet/eventlet/issues/295
|
||||
# Eventlet issue: "Python 3: wsgi doesn't handle correctly partial
|
||||
# write of socket send() when using writelines()".
|
||||
#
|
||||
# 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
|
||||
# Switching wsgi wfile to buffered mode fixes the issue.
|
||||
#
|
||||
# Related CPython issue: "Raw I/O writelines() broken",
|
||||
# http://bugs.python.org/issue26292
|
||||
|
||||
Reference in New Issue
Block a user