BaseHTTPServer incorrectly implements response code 100

[samwyse]

BPO	1491
Nosy	@gvanrossum, @jcea, @orsenthil, @merwok
Files	BaseHTTPServer_continue_3.patch: Update patch against current trunk BaseHTTPServer_continue_3_py3k.patch: py3k port of the 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 = 'https://github.com/orsenthil'
closed_at = <Date 2010-09-30.06:13:25.684>
created_at = <Date 2007-11-23.12:52:39.953>
labels = ['easy', 'type-bug', 'library']
title = 'BaseHTTPServer incorrectly implements response code 100'
updated_at = <Date 2013-09-30.23:46:02.735>
user = 'https://bugs.python.org/samwyse'

bugs.python.org fields:

activity = <Date 2013-09-30.23:46:02.735>
actor = 'jcea'
assignee = 'orsenthil'
closed = True
closed_date = <Date 2010-09-30.06:13:25.684>
closer = 'orsenthil'
components = ['Library (Lib)']
creation = <Date 2007-11-23.12:52:39.953>
creator = 'samwyse'
dependencies = []
files = ['16652', '16653']
hgrepos = []
issue_num = 1491
keywords = ['patch', 'easy']
message_count = 12.0
messages = ['57783', '57859', '58265', '59344', '66571', '66572', '66595', '66597', '68514', '101712', '101717', '117699']
nosy_count = 9.0
nosy_names = ['gvanrossum', 'jcea', 'orsenthil', 'vila', 'samwyse', 'hodgestar', 'Neil Muller', 'jerith', 'eric.araujo']
pr_nums = []
priority = 'low'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue1491'
versions = ['Python 3.1', 'Python 2.7', 'Python 3.2']

[samwyse]

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

[gvanrossum]

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.

[samwyse]

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.

[gvanrossum]

Something for the bug day on Jan 19.

[jerith]

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.

[jerith]

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.

[samwyse]

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\>

---

[samwyse]

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.

[NeilMuller]

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.

[NeilMuller]

Poking the issue with an updated patch for trunk.

[NeilMuller]

And a py3k version (although the conversion can be improved).