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

[Martin Panter]

Martin Panter added the comment:

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.

--
nosy: +martin.panter

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Martin Panter]

Changes by Martin Panter :


--
Removed message: http://bugs.python.org/msg227647

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Martin Panter]

Changes by Martin Panter :


--
Removed message: http://bugs.python.org/msg227645

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Yury Selivanov]

Yury Selivanov added the comment:

I left some comments in the codereview.

I think that having some half-baked solution is great, but I really would like 
to see a proper fix, i.e. with remove_header and other methods fixed. Ideally, 
you should add a UserDict implementation for headers that implements proper 
__XXXitem__ methods.

--
nosy: +yselivanov
versions: +Python 3.5 -Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17319
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Yury Selivanov]

Yury Selivanov added the comment:

Oups, my previous comment is related to issue #5550, wrong window.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17319
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[karl]

karl added the comment:

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

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17319
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Dmi Baranov]

Dmi Baranov added the comment:

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

--
keywords: +patch
nosy: +dmi.baranov
Added file: http://bugs.python.org/file30100/issue17319.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17319
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Berker Peksag]

Changes by Berker Peksag berker.pek...@gmail.com:


--
nosy: +berker.peksag
stage:  - patch review
versions: +Python 3.4 -Python 3.3

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17319
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[karl]

New submission from karl:

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.

--
components: Library (Lib)
messages: 183201
nosy: karlcow, orsenthil
priority: normal
severity: normal
status: open
title: http.server.BaseHTTPRequestHandler send_response_only doesn't check the 
type and value of the code.
type: enhancement
versions: Python 3.3

___