Initial CL for Downloads resumption.

net/tools/testserver/testserver.py

Index: net/tools/testserver/testserver.py
diff --git a/net/tools/testserver/testserver.py b/net/tools/testserver/testserver.py
index 91fe6e1917197331f31d18cc066a76c6e50ffe8b..37582005cd02232ab9f90c97cd93bc47709c3ddd 100755
--- a/net/tools/testserver/testserver.py
+++ b/net/tools/testserver/testserver.py
@@ -28,6 +28,7 @@ import re
 import select
 import socket
 import SocketServer
+import struct
 import sys
 import threading
 import time
@@ -473,6 +474,7 @@ class TestPageHandler(BasePageHandler):
       self.MultipartSlowHandler,
       self.GetSSLSessionCacheHandler,
       self.CloseSocketHandler,
+      self.RangeResetHandler,
       self.DefaultResponseHandler]
     post_handlers = [
       self.EchoTitleHandler,
@@ -1621,6 +1623,95 @@ class TestPageHandler(BasePageHandler):
     self.wfile.close()
     return True
 
+  def RangeResetHandler(self):
+    """Send data broken up by connection resets every N (default 10K) bytes.
+    Support range requests.  If the data requested doesn't straddle a reset
+    boundary, it will all be sent.  Used for testing resuming downloads."""
+
+    if not self._ShouldHandleRequest('/rangereset'):
+      return False
+
+    _, _, url_path, _, query, _ = urlparse.urlparse(self.path)
+
+    # Defaults
+    size = 15000
+    # Note that the rst is sent just before sending the rst_boundary byte.
+    rst_boundary = 10000
+    respond_to_range = True
+    hold_for_signal = False
+
+    # Parse the query
+    if query:
+      for ele in query.split('&'):

[Paweł Hajdan Jr., 2013/01/04 22:56:58]

Are you doing query string parsing here? Please use python's included batteries,
see e.g.
http://stackoverflow.com/questions/10113090/best-way-to-parse-a-url-query-string

[Randy Smith (Not in Mondays), 2013/01/07 20:54:10]

Whoops.  Very good point.  Done.

+        namevalue = ele.split('=')
+        name = namevalue[0]
+        value = namevalue[1] if len(namevalue) == 2 else None
+        if name == 'size':
+          size = int(val)
+        elif name == 'rst_boundary':
+          rst_boundary = int(val)
+        elif name == 'bounce_range':
+          respond_to_range = False
+        elif name == 'hold':
+          hold_for_signal = True
+        else:
+          return False
+
+    first_byte = 0
+    last_byte = size - 1
+
+    # Does that define what we want to return, or do we need to apply
+    # a range?
+    range_response = False
+    range_header = self.headers.getheader('range')
+    if range_header and respond_to_range:
+      mo = re.match("bytes=([0-9]*)-([0-9]*)", range_header)

[Paweł Hajdan Jr., 2013/01/04 22:56:58]

nit: Why not \d for the digits?

[Randy Smith (Not in Mondays), 2013/01/07 20:54:10]

Too much time using emacs regexps + hammering in a hack and not cleaning up
afterwards :-{.  Done.

+      if mo.group(1):
+        first_byte = int(mo.group(1))
+      if mo.group(2):
+        last_byte = int(mo.group(2))
+      if last_byte > size - 1:
+        last_byte = size - 1
+      range_response = True
+      if last_byte < first_byte:
+        return False
+
+    if not range_response:

[Paweł Hajdan Jr., 2013/01/04 22:56:58]

nit: Reverse the order - put if range_response branch first, to avoid
double-negation on else.

[Randy Smith (Not in Mondays), 2013/01/07 20:54:10]

Done.

+      self.send_response(200)
+    else:
+      self.send_response(206)
+      self.send_header('Content-Range',
+                       'bytes %d-%d/%d' % (first_byte, last_byte, size))
+    self.send_header('Content-Type', 'application/octet-stream')
+    self.send_header('Content-Length', last_byte - first_byte + 1)
+    self.end_headers()
+
+    if hold_for_signal:
+      ## Needed to allow server to handle new requests (weird).

[Paweł Hajdan Jr., 2013/01/04 22:56:58]

nit: No double # please, and what is really going on here?

[Randy Smith (Not in Mondays), 2013/01/07 20:54:10]

I've updated the comment; let me know if you want more than what I've written.

+      self.wfile.write('X')
+      first_byte = first_byte + 1
+      # handle requests until one of them clears this flag.
+      self.server.waitForDownload = True
+      while self.server.waitForDownload:
+        self.server.handle_request()
+
+    possible_rst = ((first_byte / rst_boundary) + 1) * rst_boundary
+    if possible_rst < last_byte:
+      ## We're resetting the conneciton part way in.

[Paweł Hajdan Jr., 2013/01/04 22:56:58]

nit: No double #.

[benjhayden, 2013/01/06 15:46:04]

connecTIon

[Randy Smith (Not in Mondays), 2013/01/07 20:54:10]

Done.

[Randy Smith (Not in Mondays), 2013/01/07 20:54:10]

Done.

+      self.wfile.write('X' * (possible_rst - first_byte))
+      s = self.connection  # This is the request socket.

[Paweł Hajdan Jr., 2013/01/04 22:56:58]

nit: Make the comment more meaningful. What is this doing with the socket? Why
not just write self.connection below? It's 2-3 occurrences.

[Randy Smith (Not in Mondays), 2013/01/07 20:54:10]

Done.

+      l_onoff = 1  # Linger is active.
+      l_linger = 0  # Seconds to linger for.
+      s.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER,
+                   struct.pack('ii', l_onoff, l_linger))
+      s.close()
+      time.sleep(0.05)  # Give time to the system.

[Paweł Hajdan Jr., 2013/01/04 22:56:58]

Huh? That makes me extremely suspicious. Did you intend to *wait* for something?
:)

[Randy Smith (Not in Mondays), 2013/01/07 20:54:10]

Interesting.  I copied that code, assuming that we needed the equivalent of an
OS level thread yield, but we get the RST even when it's not there.  Removed.  

[Randy Smith (Not in Mondays), 2013/01/08 19:31:12]

Pawel: I was wrong, that sleep was needed for the test to pass on Windows.  The
failure mode was not all the data being sent.  After playing around and musing a
bit, I concluded that the most likely explanation was dead in the middle of your
worst fears :-}: That the sleep was giving time for an async thread socket
implementation to shove the data out before being forcibly closed down the by
linger option on socket close.  Which isn't good code to commit.  

What I did instead was drop the amount of data we're attempting to send to under
the apparent internal buffer for initial send on Windows, and put a big honking
warning comment in the code.  Let me know what you think.  I'm sure you'll be
happy, I just want to know *how* happy :-z.

On 2013/01/07 20:54:10, rdsmith wrote:

+    else:
+      ## A more normal response

[Paweł Hajdan Jr., 2013/01/04 22:56:58]

nit: No double #, and make it more meaningful please. "more normal" is very
vague.

Actually it'd be cool to put this simple case first and return True early, to
avoid indenting the block above.

[Randy Smith (Not in Mondays), 2013/01/07 20:54:10]

Done, although I wince a little, as that makes the more "normal" case the
indented, and hence implied exceptional, one.

+      self.wfile.write('X' * (last_byte - first_byte + 1))
+
+    return True
+
   def DefaultResponseHandler(self):
     """This is the catch-all response handler for requests that aren't handled
     by one of the special handlers above.