Issue17319
Created on 2013-02-28 04:12 by karlcow, last changed 2015-09-19 06:28 by martin.panter.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
issue17319.patch | dmi.baranov, 2013-05-01 20:53 | review |
Messages (4) | |||
---|---|---|---|
msg183201 - (view) | Author: karl (karlcow) * | Date: 2013-02-28 04:12 | |
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. |
|||
msg188242 - (view) | Author: Dmi Baranov (dmi.baranov) * | Date: 2013-05-01 20:53 | |
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 |
|||
msg227506 - (view) | Author: karl (karlcow) * | Date: 2014-09-25 05:55 | |
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 |
|||
msg251054 - (view) | Author: Martin Panter (martin.panter) * ![]() |
Date: 2015-09-19 06:28 | |
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. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2015-09-19 06:28:46 | martin.panter | set | nosy:
+ martin.panter messages: + msg251054 |
2015-09-19 06:23:43 | martin.panter | set | messages: - msg227647 |
2015-09-19 06:23:26 | martin.panter | set | messages: - msg227645 |
2014-09-26 20:32:04 | yselivanov | set | messages: + msg227647 |
2014-09-26 20:30:23 | yselivanov | set | nosy:
+ yselivanov messages: + msg227645 versions: + Python 3.5, - Python 3.4 |
2014-09-25 05:55:44 | karlcow | set | messages: + msg227506 |
2013-05-02 05:05:34 | berker.peksag | set | nosy:
+ berker.peksag stage: patch review versions: + Python 3.4, - Python 3.3 |
2013-05-01 20:53:14 | dmi.baranov | set | files:
+ issue17319.patch nosy: + dmi.baranov messages: + msg188242 keywords: + patch |
2013-02-28 04:12:05 | karlcow | create |