http.server.BaseHTTPRequestHandler send_response_only doesn't check the type and value of the code.

[karlcow]

BPO	17319
Nosy	@orsenthil, @karlcow, @berkerpeksag, @vadmium, @1st1
Files	issue17319.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2013-02-28.04:12:05.347>
labels = ['type-feature', 'library']
title = "http.server.BaseHTTPRequestHandler send_response_only doesn't check the type and value of the code."
updated_at = <Date 2015-09-19.06:28:46.527>
user = 'https://github.com/karlcow'

bugs.python.org fields:

activity = <Date 2015-09-19.06:28:46.527>
actor = 'martin.panter'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2013-02-28.04:12:05.347>
creator = 'karlcow'
dependencies = []
files = ['30100']
hgrepos = []
issue_num = 17319
keywords = ['patch']
message_count = 4.0
messages = ['183201', '188242', '227506', '251054']
nosy_count = 6.0
nosy_names = ['orsenthil', 'karlcow', 'berker.peksag', 'martin.panter', 'yselivanov', 'dmi.baranov']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue17319'
versions = ['Python 3.5']

[karlcow]

def send_response_only(self, code, message=None):
http://hg.python.org/cpython/file/3.3/Lib/http/server.py#l448

There is no type checking on code or if the code is appropriate. Let's take

==============================================

#!/usr/bin/env python3.3
import http.server


class HTTPHandler(http.server.BaseHTTPRequestHandler):
    "A very simple server"
    def do_GET(self):
        self.send_response(200)
        self.send_header('Content-type', 'text/plain')
        self.end_headers()
        self.wfile.write(bytes('Response body\n\n', 'latin1'))

if __name__ == '__main__':
    addr = ('', 9000)
    http.server.HTTPServer(addr, HTTPHandler).serve_forever()

=====================================================

A request is working well.

=========================================
→ http GET localhost:9000
HTTP/1.0 200 OK
Server: BaseHTTP/0.6 Python/3.3.0
Date: Thu, 28 Feb 2013 04:00:44 GMT
Content-type: text/plain

Response body

=========================================

And the server log is

127.0.0.1 - - [27/Feb/2013 23:00:44] "GET / HTTP/1.1" 200 -

Then let's try

=========================================

#!/usr/bin/env python3.3
import http.server


class HTTPHandler(http.server.BaseHTTPRequestHandler):
    "A very simple server"
    def do_GET(self):
        self.send_response(999)
        self.send_header('Content-type', 'text/plain')
        self.end_headers()
        self.wfile.write(bytes('Response body\n\n', 'latin1'))

if __name__ == '__main__':
    addr = ('', 9000)
    http.server.HTTPServer(addr, HTTPHandler).serve_forever()

=========================================

The response is

=========================================
→ http GET localhost:9000
HTTP/1.0 999
Server: BaseHTTP/0.6 Python/3.3.0
Date: Thu, 28 Feb 2013 03:55:54 GMT
Content-type: text/plain

Response body

=========================================

and the log server is

127.0.0.1 - - [27/Feb/2013 22:55:12] "GET / HTTP/1.1" 999 -

And finally

=========================================

#!/usr/bin/env python3.3
import http.server


class HTTPHandler(http.server.BaseHTTPRequestHandler):
    "A very simple server"
    def do_GET(self):
        self.send_response('foobar')
        self.send_header('Content-type', 'text/plain')
        self.end_headers()
        self.wfile.write(bytes('Response body\n\n', 'latin1'))

if __name__ == '__main__':
    addr = ('', 9000)
    http.server.HTTPServer(addr, HTTPHandler).serve_forever()

=========================================

The response is then

=========================================
→ http GET localhost:9000
HTTPConnectionPool(host='localhost', port=9000): Max retries exceeded with url: /
=========================================

and the server log is

127.0.0.1 - - [27/Feb/2013 22:56:39] "GET / HTTP/1.1" foobar -
----------------------------------------

Exception happened during processing of request from ('127.0.0.1', 53917)
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.3/lib/python3.3/socketserver.py", line 306, in _handle_request_noblock
    self.process_request(request, client_address)
  File "/Library/Frameworks/Python.framework/Versions/3.3/lib/python3.3/socketserver.py", line 332, in process_request
    self.finish_request(request, client_address)
  File "/Library/Frameworks/Python.framework/Versions/3.3/lib/python3.3/socketserver.py", line 345, in finish_request
    self.RequestHandlerClass(request, client_address, self)
  File "/Library/Frameworks/Python.framework/Versions/3.3/lib/python3.3/socketserver.py", line 666, in __init__
    self.handle()
  File "/Library/Frameworks/Python.framework/Versions/3.3/lib/python3.3/http/server.py", line 400, in handle
    self.handle_one_request()
  File "/Library/Frameworks/Python.framework/Versions/3.3/lib/python3.3/http/server.py", line 388, in handle_one_request
    method()
  File "../25/server.py", line 8, in do_GET
    self.send_response('foobar')
  File "/Library/Frameworks/Python.framework/Versions/3.3/lib/python3.3/http/server.py", line 444, in send_response
    self.send_response_only(code, message)
  File "/Library/Frameworks/Python.framework/Versions/3.3/lib/python3.3/http/server.py", line 459, in send_response_only
    (self.protocol_version, code, message)).encode(
TypeError: %d format: a number is required, not str

---

Both error messages and server logs are not very helpful.
Shall we fix it?
What others think?

I guess there should be test cases too?
I'm happy to make unit test cases for it though I might need a bit of guidance as I'm not comfortable with unit test cases mocking connections.

[dmibaranov]

Attached patch for checking status code based at RFC 2616 [1].
Covered by tests.

[1] http://tools.ietf.org/html/rfc2616#section-6.1.1

[karlcow]

Where this is defined in the new RFC.

http://tools.ietf.org/html/rfc7230#section-3.1.2

 status-line = HTTP-version SP status-code SP reason-phrase CRLF

Things to enforce

 status-code    = 3DIGIT

Response status code are now defined in
http://tools.ietf.org/html/rfc7231#section-6
with something important.

HTTP status codes are extensible. HTTP clients are not required to
understand the meaning of all registered status codes, though such
understanding is obviously desirable. However, a client MUST
understand the class of any status code, as indicated by the first
digit, and treat an unrecognized status code as being equivalent to
the x00 status code of that class, with the exception that a
recipient MUST NOT cache a response with an unrecognized status code.

For example, if an unrecognized status code of 471 is received by a
client, the client can assume that there was something wrong with its
request and treat the response as if it had received a 400 (Bad
Request) status code. The response message will usually contain a
representation that explains the status.

That should help.

The full registry of status code is defined here
http://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml

@dmi.baranov

In the patch
+def _is_valid_status_code(code):
+ return isinstance(code, int) and 0 <= code <= 999

Maybe there is a missing check where the len(str(code)) == 3

[vadmium]

The proposed patch changes the code to 500 if the code is invalid (rather than raising an exception as I initially assumed).

I would be inclined to leave send_response() without any extra error checking or handling, unless this is a common problem and there is a real need for it. Although I reckon it might be nice to have a generic (higher-level) exception handler for the HTTP server that responds with “500 Internal server error” if possible.