classification
Title: BaseHTTPServer incorrectly implements response code 100
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.1, Python 3.2, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: orsenthil Nosy List: Neil Muller, eric.araujo, gvanrossum, hodgestar, jcea, jerith, orsenthil, samwyse, vila
Priority: low Keywords: easy, patch

Created on 2007-11-23 12:52 by samwyse, last changed 2013-09-30 23:46 by jcea. This issue is now closed.

Files
File name Uploaded Description Edit
BaseHTTPServer_continue_3.patch Neil Muller, 2010-03-25 15:54 Update patch against current trunk
BaseHTTPServer_continue_3_py3k.patch Neil Muller, 2010-03-25 16:22 py3k port of the patch
Messages (12)
msg57783 - (view) Author: Samwyse (samwyse) Date: 2007-11-23 12:52
RFC 2616 sec 8.2.3 states, "An origin server that sends a 100 (Continue)
response MUST ultimately send a final status code, once the request body
is received and processed, unless it terminates the transport connection
prematurely."  The obvious way to do this is to invoke the
'send_response' method twice, once with a code of 100, then with the
final code.  However, BaseHTTPServer will always send two headers
('Server' and 'Date') when it send a response.  One possible fix is to
add an option to the method to suppress sending headers; another is to
add the following code to the 'send_response' method, immediately prior
to the calls to 'self.send_header':

        if code == 100:
            return

The more paranoid among us may wish to use this code instead:

        if code == 100:
            expectation = self.headers.get('Expect', "")
            if expectation.lower() == '100-continue':
                return
msg57859 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-11-26 22:14
I'm not sure I understand why anyone would ever want to send a 100
response anyway.

If I were to add support for this, I'd probably refactor send_response()
so that there's a lower-level function send_response_only() that *only*
sends the response header and change send_response() to call that
followed by sending the headers. I'm not sure where the request logging
code should go but I suspect it should be in send_response(), not in
send_response_only().

Speaking of send_response(), I wonder if it is correct to send the
Server: and Date: headers when the request version is HTTP/0.9?

I don't think we should add the paranoid version to the code; in general
this code is not sufficiently aware of all the quirks of HTTP to prevent
nonsensical things from happening.
msg58265 - (view) Author: Samwyse (samwyse) Date: 2007-12-07 06:33
Refactoring sounds like a good idea.  Someone would need to check how
other web servers log this, if at all.  You're probably right about the
HTTP/0.9 as well.

The main reason to send a 100 response is because the client sent an
"Expect: 100-continue" header, and won't send data until either it
receives the header or a timeout expires.  The server is expected to
validate the received headers before sending the response, although it
is allowed to send it in any event.
msg59344 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-01-06 00:05
Something for the bug day on Jan 19.
msg66571 - (view) Author: Jeremy Thurgood (jerith) Date: 2008-05-10 19:34
Added handling for "Expect: 100-continue" header to
BaseHTTPRequestHandler. By default, any request that has this header
gets a 100 Continue response (with no other headers) before
do_WHATEVER() is called. By overriding handle_expect_100(), you can
reject incoming requests instead of sending a 100 Continue if you so desire.

Refactoring as per comments above was also performed.

Note: This patch changes the default behaviour in the case where both
the client and the server claim to support HTTP/1.1 from doing nothing
in the case of an "Expect: 100-continue" header on the request to
sending a 100 Continue response and then completing the request as normal.
msg66572 - (view) Author: Jeremy Thurgood (jerith) Date: 2008-05-10 19:36
The above patch adds a set of tests for BaseHTTPServer, although the
only tests actually written were those around the areas touched by the
work done for this issue.
msg66595 - (view) Author: Samwyse (samwyse) Date: 2008-05-11 02:01
Although any given implementation of an HTTP server is likely to serve
up its headers in a predicable, repeatable, order, I don't think that
we should specify a particular order in the test suite.  Section 4.2
of RFC 2616 specifically states, "The order in which header fields
with differing field names are received is not significant."  So, I
dislike these (and similar) statements in the patch:

+        self.assertTrue(result[1].startswith('Server: '))
+        self.assertTrue(result[2].startswith('Date: '))
+        self.assertTrue(result[3].startswith('Content-Type: '))

I think it would be better to say this:

+        self.assertEqual(sum(r.startswith('Server: ') for r in
result[1:-1]), 1)
+        self.assertEqual(sum(r.startswith('Date: ') for r in result[1:-1]), 1)
+        self.assertEqual(sum(r.startswith('Content-Type: ') for r in
result[1:-1]), 1)

or even this:

+        # Verify that certain message headers occur exactly once.
+        for fieldName in 'Server: ', 'Date: ', 'Content-Type: ':
+            self.assertEqual(sum(r.startswith(fieldName) for r in
result[1:-1]), 1)

(Note that in test_with_continue_1_1, you'd need to say result[2:-1].)

On Sat, May 10, 2008 at 2:34 PM, Jeremy Thurgood <report@bugs.python.org> wrote:
>
> Jeremy Thurgood <firxen@gmail.com> added the comment:
>
> Added handling for "Expect: 100-continue" header to
> BaseHTTPRequestHandler. By default, any request that has this header
> gets a 100 Continue response (with no other headers) before
> do_WHATEVER() is called. By overriding handle_expect_100(), you can
> reject incoming requests instead of sending a 100 Continue if you so desire.
>
> Refactoring as per comments above was also performed.
>
> Note: This patch changes the default behaviour in the case where both
> the client and the server claim to support HTTP/1.1 from doing nothing
> in the case of an "Expect: 100-continue" header on the request to
> sending a 100 Continue response and then completing the request as normal.
>
> ----------
> keywords: +patch
> nosy: +jerith
> Added file: http://bugs.python.org/file10269/BaseHTTPServer_continue.patch
>
> __________________________________
> Tracker <report@bugs.python.org>
> <http://bugs.python.org/issue1491>
> __________________________________
>
msg66597 - (view) Author: Samwyse (samwyse) Date: 2008-05-11 03:02
In the attached file, I've refactored the entire
BaseHTTPRequestHandlerTestCase class.  In doing so, I couldn't help but
notice that we're expecting HTTP/1.1 responses when sending HTTP/1.0
requests.  RFC 2616 is unclear about whether this is legitimate, but I'm
not going to tackle it tonight.
msg68514 - (view) Author: Neil Muller (Neil Muller) Date: 2008-06-21 16:45
The attached patch is against recent svn (r64442), and adds samwyse's
refactoring of the class. The test for server responses is made less
restrictive when the request isn't HTTP/1.1.
msg101712 - (view) Author: Neil Muller (Neil Muller) Date: 2010-03-25 15:53
Poking the issue with an updated patch for trunk.
msg101717 - (view) Author: Neil Muller (Neil Muller) Date: 2010-03-25 16:22
And a py3k version (although the conversion can be improved).
msg117699 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2010-09-30 06:13
Fixed and committed in revision 85125.

Actually, this changes the behavior of BaseHTTPServer a little and adds two new methods to the code, so I don't think, this should be back-ported to 2.7 and 3.1. If older code were to encounter the new 100 Continue response, then chances are that it may break.

I have added Misc/NEWS too.

BTW, there are tests for BaseHTTPServer in test_httpservers, thought it often gets overlooked. I merged the tests in the patch to that file.

I shall back-port some of the other tests which are useful.

Thanks!
History
Date User Action Args
2013-09-30 23:46:02jceasetnosy: + jcea
2010-09-30 06:13:25orsenthilsetstatus: open -> closed
resolution: fixed
messages: + msg117699

stage: patch review -> resolved
2010-09-22 06:01:45orsenthilsetassignee: orsenthil
2010-09-18 15:13:24BreamoreBoysetstage: patch review
versions: + Python 3.1, Python 2.7, Python 3.2, - Python 2.6
2010-09-18 15:11:11BreamoreBoysetfiles: - BaseHTTPServer_continue_2.patch
2010-09-18 15:10:59BreamoreBoysetfiles: - BaseHTTPRequestHandlerTestCase.py
2010-09-18 15:10:38BreamoreBoysetfiles: - BaseHTTPServer_continue.patch
2010-05-31 12:16:03eric.araujosetnosy: + eric.araujo
2010-03-25 16:23:40pitrousetnosy: + orsenthil
2010-03-25 16:22:26Neil Mullersetfiles: + BaseHTTPServer_continue_3_py3k.patch

messages: + msg101717
2010-03-25 15:54:01Neil Mullersetfiles: + BaseHTTPServer_continue_3.patch

messages: + msg101712
2008-06-21 16:46:01Neil Mullersetfiles: + BaseHTTPServer_continue_2.patch
nosy: + Neil Muller
messages: + msg68514
2008-05-11 08:51:59hodgestarsetnosy: + hodgestar
2008-05-11 03:02:58samwysesetfiles: + BaseHTTPRequestHandlerTestCase.py
messages: + msg66597
2008-05-11 02:01:37samwysesetmessages: + msg66595
2008-05-10 19:36:47jerithsetmessages: + msg66572
2008-05-10 19:34:06jerithsetfiles: + BaseHTTPServer_continue.patch
nosy: + jerith
messages: + msg66571
keywords: + patch
2008-01-12 14:00:07georg.brandlsetkeywords: + easy
2008-01-06 00:05:33gvanrossumsetmessages: + msg59344
2008-01-05 15:06:27vilasetnosy: + vila
2007-12-07 06:33:39samwysesetmessages: + msg58265
2007-11-26 22:14:39gvanrossumsetpriority: low
nosy: + gvanrossum
messages: + msg57859
versions: - Python 2.5, Python 2.4, Python 3.0
2007-11-23 12:52:40samwysecreate