Do not send message body when method is HEAD in simple_response

Also improve logging and add first working server test
This commit is contained in:
Kovid Goyal 2015-05-19 16:31:26 +05:30
parent 8952b8251e
commit 1897119a24
3 changed files with 29 additions and 8 deletions

View File

@ -176,8 +176,12 @@ def http_communicate(conn):
pair.simple_response(httplib.REQUEST_TIMEOUT) pair.simple_response(httplib.REQUEST_TIMEOUT)
except NonHTTPConnRequest: except NonHTTPConnRequest:
raise raise
except socket.error:
# This socket is broken. Log the error and close connection
conn.server_loop.log.exception(
'Communication failed while processing request:', pair.repr_for_log() if getattr(pair, 'started_request', False) else 'None')
except Exception: except Exception:
conn.server_loop.log.exception('Error serving request:', pair.repr_for_log() if pair else 'None') conn.server_loop.log.exception('Error serving request:', pair.repr_for_log() if getattr(pair, 'started_request', False) else 'None')
if pair and not pair.sent_headers: if pair and not pair.sent_headers:
pair.simple_response(httplib.INTERNAL_SERVER_ERROR) pair.simple_response(httplib.INTERNAL_SERVER_ERROR)
@ -272,6 +276,7 @@ class HTTPPair(object):
self.request_line = None self.request_line = None
self.path = () self.path = ()
self.qs = MultiDict() self.qs = MultiDict()
self.method = None
"""When True, the request has been parsed and is ready to begin generating """When True, the request has been parsed and is ready to begin generating
the response. When False, signals the calling Connection that the response the response. When False, signals the calling Connection that the response
@ -316,7 +321,7 @@ class HTTPPair(object):
self.ready = True self.ready = True
def read_request_line(self): def read_request_line(self):
request_line = self.conn.socket_file.readline(maxsize=self.max_header_line_size) self.request_line = request_line = self.conn.socket_file.readline(maxsize=self.max_header_line_size)
# Set started_request to True so http_communicate() knows to send 408 # Set started_request to True so http_communicate() knows to send 408
# from here on out. # from here on out.
@ -338,7 +343,6 @@ class HTTPPair(object):
httplib.BAD_REQUEST, "HTTP requires CRLF terminators") httplib.BAD_REQUEST, "HTTP requires CRLF terminators")
return False return False
self.request_line = request_line
try: try:
method, uri, req_protocol = request_line.strip().split(b' ', 2) method, uri, req_protocol = request_line.strip().split(b' ', 2)
rp = int(req_protocol[5]), int(req_protocol[7]) rp = int(req_protocol[5]), int(req_protocol[7])
@ -456,7 +460,8 @@ class HTTPPair(object):
buf.append("Connection: close") buf.append("Connection: close")
buf.append('') buf.append('')
buf = [(x + '\r\n').encode('ascii') for x in buf] buf = [(x + '\r\n').encode('ascii') for x in buf]
buf.append(msg) if self.method != 'HEAD':
buf.append(msg)
if read_remaining_input: if read_remaining_input:
self.input_reader.read() self.input_reader.read()
self.flushed_write(b''.join(buf)) self.flushed_write(b''.join(buf))

View File

@ -7,9 +7,10 @@ __license__ = 'GPL v3'
__copyright__ = '2011, Kovid Goyal <kovid@kovidgoyal.net>' __copyright__ = '2011, Kovid Goyal <kovid@kovidgoyal.net>'
__docformat__ = 'restructuredtext en' __docformat__ = 'restructuredtext en'
import unittest, shutil, time, httplib import unittest, shutil, time, httplib, traceback
from functools import partial from functools import partial
from threading import Thread from threading import Thread
from calibre.utils.logging import ThreadSafeLog
rmtree = partial(shutil.rmtree, ignore_errors=True) rmtree = partial(shutil.rmtree, ignore_errors=True)
@ -20,6 +21,12 @@ class BaseTest(unittest.TestCase):
ae = unittest.TestCase.assertEqual ae = unittest.TestCase.assertEqual
class TestLog(ThreadSafeLog):
def exception(self, *args, **kwargs):
limit = kwargs.pop('limit', None)
self.prints(self.ERROR, *args, **kwargs)
self.prints(self.WARN, traceback.format_exc(limit))
class TestServer(Thread): class TestServer(Thread):
@ -30,11 +37,10 @@ class TestServer(Thread):
from calibre.srv.opts import Options from calibre.srv.opts import Options
from calibre.srv.loop import ServerLoop from calibre.srv.loop import ServerLoop
from calibre.srv.http import create_http_handler from calibre.srv.http import create_http_handler
from calibre.utils.logging import ThreadSafeLog
self.loop = ServerLoop( self.loop = ServerLoop(
opts=Options(shutdown_timeout=0.1), opts=Options(shutdown_timeout=0.1),
bind_address=('localhost', 0), http_handler=create_http_handler(handler), bind_address=('localhost', 0), http_handler=create_http_handler(handler),
log=ThreadSafeLog(level=ThreadSafeLog.WARN), log=TestLog(level=ThreadSafeLog.WARN),
) )
def run(self): def run(self):

View File

@ -61,11 +61,21 @@ class TestHTTP(BaseTest):
def test_http_basic(self): # {{{ def test_http_basic(self): # {{{
'Test basic HTTP protocol conformance' 'Test basic HTTP protocol conformance'
from calibre.srv.errors import HTTP404 from calibre.srv.errors import HTTP404
body = 'Requested resource not found'
def handler(conn): def handler(conn):
raise HTTP404('Requested resource not found') raise HTTP404(body)
with TestServer(handler) as server: with TestServer(handler) as server:
conn = server.connect() conn = server.connect()
conn.request('HEAD', '/moose') conn.request('HEAD', '/moose')
r = conn.getresponse() r = conn.getresponse()
self.ae(r.status, httplib.NOT_FOUND) self.ae(r.status, httplib.NOT_FOUND)
self.assertIsNotNone(r.getheader('Date', None))
self.ae(r.getheader('Content-Length'), str(len(body)))
self.ae(r.getheader('Content-Type'), 'text/plain; charset=UTF-8')
self.ae(len(r.getheaders()), 3)
self.ae(r.read(), '')
conn.request('GET', '/moose')
r = conn.getresponse()
self.ae(r.status, httplib.NOT_FOUND)
self.ae(r.read(), 'Requested resource not found')
# }}} # }}}