From 1897119a24d5e4cd1c84c0ee56cc97a6a2d2ecd6 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Tue, 19 May 2015 16:31:26 +0530 Subject: [PATCH] Do not send message body when method is HEAD in simple_response Also improve logging and add first working server test --- src/calibre/srv/http.py | 13 +++++++++---- src/calibre/srv/tests/base.py | 12 +++++++++--- src/calibre/srv/tests/http.py | 12 +++++++++++- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/calibre/srv/http.py b/src/calibre/srv/http.py index 999cd14e96..aa13222fd3 100644 --- a/src/calibre/srv/http.py +++ b/src/calibre/srv/http.py @@ -176,8 +176,12 @@ def http_communicate(conn): pair.simple_response(httplib.REQUEST_TIMEOUT) except NonHTTPConnRequest: 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: - 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: pair.simple_response(httplib.INTERNAL_SERVER_ERROR) @@ -272,6 +276,7 @@ class HTTPPair(object): self.request_line = None self.path = () self.qs = MultiDict() + self.method = None """When True, the request has been parsed and is ready to begin generating the response. When False, signals the calling Connection that the response @@ -316,7 +321,7 @@ class HTTPPair(object): self.ready = True 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 # from here on out. @@ -338,7 +343,6 @@ class HTTPPair(object): httplib.BAD_REQUEST, "HTTP requires CRLF terminators") return False - self.request_line = request_line try: method, uri, req_protocol = request_line.strip().split(b' ', 2) rp = int(req_protocol[5]), int(req_protocol[7]) @@ -456,7 +460,8 @@ class HTTPPair(object): buf.append("Connection: close") buf.append('') buf = [(x + '\r\n').encode('ascii') for x in buf] - buf.append(msg) + if self.method != 'HEAD': + buf.append(msg) if read_remaining_input: self.input_reader.read() self.flushed_write(b''.join(buf)) diff --git a/src/calibre/srv/tests/base.py b/src/calibre/srv/tests/base.py index d3d52480bd..4b3dd6e3b3 100644 --- a/src/calibre/srv/tests/base.py +++ b/src/calibre/srv/tests/base.py @@ -7,9 +7,10 @@ __license__ = 'GPL v3' __copyright__ = '2011, Kovid Goyal ' __docformat__ = 'restructuredtext en' -import unittest, shutil, time, httplib +import unittest, shutil, time, httplib, traceback from functools import partial from threading import Thread +from calibre.utils.logging import ThreadSafeLog rmtree = partial(shutil.rmtree, ignore_errors=True) @@ -20,6 +21,12 @@ class BaseTest(unittest.TestCase): 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): @@ -30,11 +37,10 @@ class TestServer(Thread): from calibre.srv.opts import Options from calibre.srv.loop import ServerLoop from calibre.srv.http import create_http_handler - from calibre.utils.logging import ThreadSafeLog self.loop = ServerLoop( opts=Options(shutdown_timeout=0.1), bind_address=('localhost', 0), http_handler=create_http_handler(handler), - log=ThreadSafeLog(level=ThreadSafeLog.WARN), + log=TestLog(level=ThreadSafeLog.WARN), ) def run(self): diff --git a/src/calibre/srv/tests/http.py b/src/calibre/srv/tests/http.py index 22ce0fdaa5..25e962df98 100644 --- a/src/calibre/srv/tests/http.py +++ b/src/calibre/srv/tests/http.py @@ -61,11 +61,21 @@ class TestHTTP(BaseTest): def test_http_basic(self): # {{{ 'Test basic HTTP protocol conformance' from calibre.srv.errors import HTTP404 + body = 'Requested resource not found' def handler(conn): - raise HTTP404('Requested resource not found') + raise HTTP404(body) with TestServer(handler) as server: conn = server.connect() conn.request('HEAD', '/moose') r = conn.getresponse() 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') # }}}