[issue21793] httplib client/server status refactor

Demian Brecht report at bugs.python.org
Sat Feb 21 17:03:16 CET 2015


Demian Brecht added the comment:

Well, I’m not entirely sure how I came to the conclusion that errors were a problem (other than not spending enough time walking through it) and of course you’re right about the coercion handling it just fine. I’ve removed the explicit conversion in the code but have left the tests as they’re not tested elsewhere. Hopefully that should now wrap this up.

----------
Added file: http://bugs.python.org/file38197/issue21793_logfix_4.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	Sat Feb 21 07:59:08 2015 -0800
@@ -517,6 +517,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	Sat Feb 21 07:59:08 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