[issue21793] httplib client/server status refactor
Demian Brecht
report at bugs.python.org
Sat Feb 21 02:40:22 CET 2015
Demian Brecht added the comment:
Latest patch should address all comments. It also fixes the same issue in error logging which wasn’t previously accounted for. The test file has also been updated with using HTTPStatus where possible rather than hard coded ints. This is consistent with other areas in the http package.
----------
Added file: http://bugs.python.org/file38193/issue21793_logfix_3.patch
_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue21793>
_______________________________________
-------------- next part --------------
diff -r 46bfddb14cbe Lib/http/server.py
--- a/Lib/http/server.py Fri Feb 20 10:34:20 2015 -0500
+++ b/Lib/http/server.py Fri Feb 20 17:37:25 2015 -0800
@@ -442,6 +442,10 @@
message = shortmsg
if explain is None:
explain = longmsg
+
+ if isinstance(code, HTTPStatus):
+ code = code.value
+
self.log_error("code %d, message %s", code, message)
# using _quote_html to prevent Cross Site Scripting attacks (see bug #1100201)
content = (self.error_message_format %
@@ -517,6 +521,8 @@
This is called by send_response().
"""
+ if isinstance(code, HTTPStatus):
+ code = code.value
self.log_message('"%s" %s %s',
self.requestline, str(code), str(size))
diff -r 46bfddb14cbe Lib/test/test_httpservers.py
--- a/Lib/test/test_httpservers.py Fri Feb 20 10:34:20 2015 -0500
+++ b/Lib/test/test_httpservers.py Fri Feb 20 17:37:25 2015 -0800
@@ -6,7 +6,7 @@
from http.server import BaseHTTPRequestHandler, HTTPServer, \
SimpleHTTPRequestHandler, CGIHTTPRequestHandler
-from http import server
+from http import server, HTTPStatus
import os
import sys
@@ -79,13 +79,13 @@
default_request_version = 'HTTP/1.1'
def do_TEST(self):
- self.send_response(204)
+ self.send_response(HTTPStatus.NO_CONTENT)
self.send_header('Content-Type', 'text/html')
self.send_header('Connection', 'close')
self.end_headers()
def do_KEEP(self):
- self.send_response(204)
+ self.send_response(HTTPStatus.NO_CONTENT)
self.send_header('Content-Type', 'text/html')
self.send_header('Connection', 'keep-alive')
self.end_headers()
@@ -94,7 +94,7 @@
self.send_error(999)
def do_NOTFOUND(self):
- self.send_error(404)
+ self.send_error(HTTPStatus.NOT_FOUND)
def do_EXPLAINERROR(self):
self.send_error(999, "Short Message",
@@ -122,35 +122,35 @@
def test_command(self):
self.con.request('GET', '/')
res = self.con.getresponse()
- self.assertEqual(res.status, 501)
+ self.assertEqual(res.status, HTTPStatus.NOT_IMPLEMENTED)
def test_request_line_trimming(self):
self.con._http_vsn_str = 'HTTP/1.1\n'
self.con.putrequest('XYZBOGUS', '/')
self.con.endheaders()
res = self.con.getresponse()
- self.assertEqual(res.status, 501)
+ self.assertEqual(res.status, HTTPStatus.NOT_IMPLEMENTED)
def test_version_bogus(self):
self.con._http_vsn_str = 'FUBAR'
self.con.putrequest('GET', '/')
self.con.endheaders()
res = self.con.getresponse()
- self.assertEqual(res.status, 400)
+ self.assertEqual(res.status, HTTPStatus.BAD_REQUEST)
def test_version_digits(self):
self.con._http_vsn_str = 'HTTP/9.9.9'
self.con.putrequest('GET', '/')
self.con.endheaders()
res = self.con.getresponse()
- self.assertEqual(res.status, 400)
+ self.assertEqual(res.status, HTTPStatus.BAD_REQUEST)
def test_version_none_get(self):
self.con._http_vsn_str = ''
self.con.putrequest('GET', '/')
self.con.endheaders()
res = self.con.getresponse()
- self.assertEqual(res.status, 501)
+ self.assertEqual(res.status, HTTPStatus.NOT_IMPLEMENTED)
def test_version_none(self):
# Test that a valid method is rejected when not HTTP/1.x
@@ -158,7 +158,7 @@
self.con.putrequest('CUSTOM', '/')
self.con.endheaders()
res = self.con.getresponse()
- self.assertEqual(res.status, 400)
+ self.assertEqual(res.status, HTTPStatus.BAD_REQUEST)
def test_version_invalid(self):
self.con._http_vsn = 99
@@ -166,21 +166,21 @@
self.con.putrequest('GET', '/')
self.con.endheaders()
res = self.con.getresponse()
- self.assertEqual(res.status, 505)
+ self.assertEqual(res.status, HTTPStatus.HTTP_VERSION_NOT_SUPPORTED)
def test_send_blank(self):
self.con._http_vsn_str = ''
self.con.putrequest('', '')
self.con.endheaders()
res = self.con.getresponse()
- self.assertEqual(res.status, 400)
+ self.assertEqual(res.status, HTTPStatus.BAD_REQUEST)
def test_header_close(self):
self.con.putrequest('GET', '/')
self.con.putheader('Connection', 'close')
self.con.endheaders()
res = self.con.getresponse()
- self.assertEqual(res.status, 501)
+ self.assertEqual(res.status, HTTPStatus.NOT_IMPLEMENTED)
def test_head_keep_alive(self):
self.con._http_vsn_str = 'HTTP/1.1'
@@ -188,12 +188,12 @@
self.con.putheader('Connection', 'keep-alive')
self.con.endheaders()
res = self.con.getresponse()
- self.assertEqual(res.status, 501)
+ self.assertEqual(res.status, HTTPStatus.NOT_IMPLEMENTED)
def test_handler(self):
self.con.request('TEST', '/')
res = self.con.getresponse()
- self.assertEqual(res.status, 204)
+ self.assertEqual(res.status, HTTPStatus.NO_CONTENT)
def test_return_header_keep_alive(self):
self.con.request('KEEP', '/')
@@ -230,11 +230,48 @@
# Issue #16088: standard error responses should have a content-length
self.con.request('NOTFOUND', '/')
res = self.con.getresponse()
- self.assertEqual(res.status, 404)
+ self.assertEqual(res.status, HTTPStatus.NOT_FOUND)
+
data = res.read()
self.assertEqual(int(res.getheader('Content-Length')), len(data))
+class RequestHandlerLoggingTestCase(BaseTestCase):
+ class request_handler(BaseHTTPRequestHandler):
+ protocol_version = 'HTTP/1.1'
+ default_request_version = 'HTTP/1.1'
+
+ def do_GET(self):
+ self.send_response(HTTPStatus.OK)
+ self.end_headers()
+
+ def do_ERROR(self):
+ self.send_error(HTTPStatus.NOT_FOUND, 'File not found')
+
+ def test_get(self):
+ self.con = http.client.HTTPConnection(self.HOST, self.PORT)
+ self.con.connect()
+
+ with support.captured_stderr() as err:
+ self.con.request('GET', '/')
+ self.con.getresponse()
+
+ self.assertTrue(
+ err.getvalue().endswith('"GET / HTTP/1.1" 200 -\n'))
+
+ def test_err(self):
+ self.con = http.client.HTTPConnection(self.HOST, self.PORT)
+ self.con.connect()
+
+ with support.captured_stderr() as err:
+ self.con.request('ERROR', '/')
+ self.con.getresponse()
+
+ lines = err.getvalue().split('\n')
+ self.assertTrue(lines[0].endswith('code 404, message File not found'))
+ self.assertTrue(lines[1].endswith('"ERROR / HTTP/1.1" 404 -'))
+
+
class SimpleHTTPServerTestCase(BaseTestCase):
class request_handler(NoLogRequestHandler, SimpleHTTPRequestHandler):
pass
@@ -285,52 +322,52 @@
if name != 'test': # Ignore a filename created in setUp().
filename = name
break
- body = self.check_status_and_reason(response, 200)
+ body = self.check_status_and_reason(response, HTTPStatus.OK)
quotedname = urllib.parse.quote(filename, errors='surrogatepass')
self.assertIn(('href="%s"' % quotedname)
.encode(enc, 'surrogateescape'), body)
self.assertIn(('>%s<' % html.escape(filename))
.encode(enc, 'surrogateescape'), body)
response = self.request(self.tempdir_name + '/' + quotedname)
- self.check_status_and_reason(response, 200,
+ self.check_status_and_reason(response, HTTPStatus.OK,
data=support.TESTFN_UNDECODABLE)
def test_get(self):
#constructs the path relative to the root directory of the HTTPServer
response = self.request(self.tempdir_name + '/test')
- self.check_status_and_reason(response, 200, data=self.data)
+ self.check_status_and_reason(response, HTTPStatus.OK, data=self.data)
# check for trailing "/" which should return 404. See Issue17324
response = self.request(self.tempdir_name + '/test/')
- self.check_status_and_reason(response, 404)
+ self.check_status_and_reason(response, HTTPStatus.NOT_FOUND)
response = self.request(self.tempdir_name + '/')
- self.check_status_and_reason(response, 200)
+ self.check_status_and_reason(response, HTTPStatus.OK)
response = self.request(self.tempdir_name)
- self.check_status_and_reason(response, 301)
+ self.check_status_and_reason(response, HTTPStatus.MOVED_PERMANENTLY)
response = self.request(self.tempdir_name + '/?hi=2')
- self.check_status_and_reason(response, 200)
+ self.check_status_and_reason(response, HTTPStatus.OK)
response = self.request(self.tempdir_name + '?hi=1')
- self.check_status_and_reason(response, 301)
+ self.check_status_and_reason(response, HTTPStatus.MOVED_PERMANENTLY)
self.assertEqual(response.getheader("Location"),
self.tempdir_name + "/?hi=1")
response = self.request('/ThisDoesNotExist')
- self.check_status_and_reason(response, 404)
+ self.check_status_and_reason(response, HTTPStatus.NOT_FOUND)
response = self.request('/' + 'ThisDoesNotExist' + '/')
- self.check_status_and_reason(response, 404)
+ self.check_status_and_reason(response, HTTPStatus.NOT_FOUND)
with open(os.path.join(self.tempdir_name, 'index.html'), 'w') as f:
response = self.request('/' + self.tempdir_name + '/')
- self.check_status_and_reason(response, 200)
+ self.check_status_and_reason(response, HTTPStatus.OK)
# chmod() doesn't work as expected on Windows, and filesystem
# permissions are ignored by root on Unix.
if os.name == 'posix' and os.geteuid() != 0:
os.chmod(self.tempdir, 0)
response = self.request(self.tempdir_name + '/')
- self.check_status_and_reason(response, 404)
+ self.check_status_and_reason(response, HTTPStatus.NOT_FOUND)
os.chmod(self.tempdir, 0o755)
def test_head(self):
response = self.request(
self.tempdir_name + '/test', method='HEAD')
- self.check_status_and_reason(response, 200)
+ self.check_status_and_reason(response, HTTPStatus.OK)
self.assertEqual(response.getheader('content-length'),
str(len(self.data)))
self.assertEqual(response.getheader('content-type'),
@@ -338,12 +375,12 @@
def test_invalid_requests(self):
response = self.request('/', method='FOO')
- self.check_status_and_reason(response, 501)
+ self.check_status_and_reason(response, HTTPStatus.NOT_IMPLEMENTED)
# requests must be case sensitive,so this should fail too
response = self.request('/', method='custom')
- self.check_status_and_reason(response, 501)
+ self.check_status_and_reason(response, HTTPStatus.NOT_IMPLEMENTED)
response = self.request('/', method='GETs')
- self.check_status_and_reason(response, 501)
+ self.check_status_and_reason(response, HTTPStatus.NOT_IMPLEMENTED)
cgi_file1 = """\
@@ -490,12 +527,13 @@
def test_headers_and_content(self):
res = self.request('/cgi-bin/file1.py')
- self.assertEqual((b'Hello World' + self.linesep, 'text/html', 200),
+ self.assertEqual(
+ (b'Hello World' + self.linesep, 'text/html', HTTPStatus.OK),
(res.read(), res.getheader('Content-type'), res.status))
def test_issue19435(self):
res = self.request('///////////nocgi.py/../cgi-bin/nothere.sh')
- self.assertEqual(res.status, 404)
+ self.assertEqual(res.status, HTTPStatus.NOT_FOUND)
def test_post(self):
params = urllib.parse.urlencode(
@@ -508,38 +546,43 @@
def test_invaliduri(self):
res = self.request('/cgi-bin/invalid')
res.read()
- self.assertEqual(res.status, 404)
+ self.assertEqual(res.status, HTTPStatus.NOT_FOUND)
def test_authorization(self):
headers = {b'Authorization' : b'Basic ' +
base64.b64encode(b'username:pass')}
res = self.request('/cgi-bin/file1.py', 'GET', headers=headers)
- self.assertEqual((b'Hello World' + self.linesep, 'text/html', 200),
- (res.read(), res.getheader('Content-type'), res.status))
+ self.assertEqual(
+ (b'Hello World' + self.linesep, 'text/html', HTTPStatus.OK),
+ (res.read(), res.getheader('Content-type'), res.status))
def test_no_leading_slash(self):
# http://bugs.python.org/issue2254
res = self.request('cgi-bin/file1.py')
- self.assertEqual((b'Hello World' + self.linesep, 'text/html', 200),
- (res.read(), res.getheader('Content-type'), res.status))
+ self.assertEqual(
+ (b'Hello World' + self.linesep, 'text/html', HTTPStatus.OK),
+ (res.read(), res.getheader('Content-type'), res.status))
def test_os_environ_is_not_altered(self):
signature = "Test CGI Server"
os.environ['SERVER_SOFTWARE'] = signature
res = self.request('/cgi-bin/file1.py')
- self.assertEqual((b'Hello World' + self.linesep, 'text/html', 200),
- (res.read(), res.getheader('Content-type'), res.status))
+ self.assertEqual(
+ (b'Hello World' + self.linesep, 'text/html', HTTPStatus.OK),
+ (res.read(), res.getheader('Content-type'), res.status))
self.assertEqual(os.environ['SERVER_SOFTWARE'], signature)
def test_urlquote_decoding_in_cgi_check(self):
res = self.request('/cgi-bin%2ffile1.py')
- self.assertEqual((b'Hello World' + self.linesep, 'text/html', 200),
- (res.read(), res.getheader('Content-type'), res.status))
+ self.assertEqual(
+ (b'Hello World' + self.linesep, 'text/html', HTTPStatus.OK),
+ (res.read(), res.getheader('Content-type'), res.status))
def test_nested_cgi_path_issue21323(self):
res = self.request('/cgi-bin/child-dir/file3.py')
- self.assertEqual((b'Hello World' + self.linesep, 'text/html', 200),
- (res.read(), res.getheader('Content-type'), res.status))
+ self.assertEqual(
+ (b'Hello World' + self.linesep, 'text/html', HTTPStatus.OK),
+ (res.read(), res.getheader('Content-type'), res.status))
class SocketlessRequestHandler(SimpleHTTPRequestHandler):
@@ -549,7 +592,7 @@
def do_GET(self):
self.get_called = True
- self.send_response(200)
+ self.send_response(HTTPStatus.OK)
self.send_header('Content-Type', 'text/html')
self.end_headers()
self.wfile.write(b'<html><body>Data</body></html>\r\n')
@@ -559,7 +602,7 @@
class RejectingSocketlessRequestHandler(SocketlessRequestHandler):
def handle_expect_100(self):
- self.send_error(417)
+ self.send_error(HTTPStatus.EXPECTATION_FAILED)
return False
@@ -816,6 +859,7 @@
cwd = os.getcwd()
try:
support.run_unittest(
+ RequestHandlerLoggingTestCase,
BaseHTTPRequestHandlerTestCase,
BaseHTTPServerTestCase,
SimpleHTTPServerTestCase,
More information about the Python-bugs-list
mailing list