simplehttpserver: Simplify and cleanup options

Remove use-hash-url option, and add root-path option. If given, it
must be a subpath of base-path and the server will only grant access
to paths that are subpaths of root-path. This allow to reproduce the
functionality of use-hash-url by setting root-path as a hash subpath
of base-path.

slapos/recipe/simplehttpserver/__init__.py

@@ -29,6 +29,7 @@ import string, random
 import os
 from six.moves import range
 
+from zc.buildout import UserError
 from zc.buildout.buildout import bool_option
 
 
@@ -40,48 +41,29 @@ def issubpathof(subpath, path):
 
 
 class Recipe(GenericBaseRecipe):
-
   def __init__(self, buildout, name, options):
-
     base_path = options['base-path']
-    if bool_option(options, 'use-hash-url', 'false'):
-      pool = string.ascii_letters + string.digits
-      hash_string = ''.join(random.choice(pool) for i in range(64))
-      path = os.path.join(base_path, hash_string)
-
-      if os.path.exists(base_path):
-        path_list = os.listdir(base_path)
-        if len(path_list) == 1:
-          hash_string = path_list[0]
-          path = os.path.join(base_path, hash_string)
-        elif len(path_list) > 1:
-          raise ValueError("Folder %s should contain 0 or 1 element." % base_path)
-
-      options['root-dir'] = path
-      options['path'] = hash_string
+    root_path = options.get('root-path')
+    if root_path:
+      if not issubpathof(root_path, base_path):
+        raise UserError("root-path must be a subpath of base-path")
     else:
-      options['root-dir'] = base_path
-      options['path'] = ''
+      root_path = base_path
+    self.server_parameters = {
+      'host': options['host'],
+      'port': int(options['port']),
+      'cwd': base_path,
+      'log-file': options['log-file'],
+      'cert-file': options.get('cert-file', ''),
+      'key-file': options.get('key-file', ''),
+      'root-path': root_path,
+      'allow-write': bool_option(options, 'allow-write', 'false')
+    }
     return GenericBaseRecipe.__init__(self, buildout, name, options)
 
-
   def install(self):
-
-    if not os.path.exists(self.options['root-dir']):
-      os.mkdir( self.options['root-dir'] )
-    parameters = {
-      'host': self.options['host'],
-      'port': int(self.options['port']),
-      'cwd': self.options['base-path'],
-      'log-file': self.options['log-file'],
-      'cert-file': self.options.get('cert-file', ''),
-      'key-file': self.options.get('key-file', ''),
-      'root-dir': self.options['root-dir'],
-      'allow-write': bool_option(self.options, 'allow-write', 'false')
-    }
-
     return self.createPythonScript(
         self.options['wrapper'].strip(),
         __name__ + '.simplehttpserver.run',
-        (parameters,)
+        (self.server_parameters,)
       )

slapos/recipe/simplehttpserver/simplehttpserver.py

@@ -14,10 +14,10 @@ from . import issubpathof
 
 
 class ServerHandler(SimpleHTTPRequestHandler):
-
-  document_path = ''
-  restrict_root_folder = True
-  restrict_write = True
+  base_path = None # set by run
+  root_path = None # set by run
+  restrict_path = False # set by run
+  restrict_write = True # set by run
 
   def respond(self, code=200, type='text/html'):
     self.send_response(code)
@@ -25,11 +25,14 @@ class ServerHandler(SimpleHTTPRequestHandler):
     self.end_headers()
 
   def restrictedAccess(self):
-    if self.restrict_root_folder and self.path and self.path == '/':
-      # no access to root path
-      self.respond(403)
-      self.wfile.write(b"Forbidden")
-      return True
+    if self.restrict_path:
+      path = os.path.join(self.base_path, os.path.relpath(self.path, '/'))
+      if not issubpathof(path, self.root_path):
+        logging.info('TOTO %s %s', path, self.root_path)
+        # no access outside root path
+        self.respond(403)
+        self.wfile.write(b"Forbidden")
+        return True
     if self.restrict_write and self.command not in ('GET', 'HEAD'):
       # no write access
       self.respond(403)
@@ -77,9 +80,9 @@ class ServerHandler(SimpleHTTPRequestHandler):
     self.writeFile(file_path, file_content, file_open_mode)
 
   def writeFile(self, filename, content, method='ab'):
-    file_path = os.path.abspath(os.path.join(self.document_path, filename))
+    file_path = os.path.abspath(os.path.join(self.root_path, filename))
     # Check writing there is allowed
-    if not issubpathof(file_path, self.document_path):
+    if not issubpathof(file_path, self.root_path):
       self.respond(403, 'text/plain')
       self.wfile.write(b"Forbidden")
       return
@@ -115,11 +118,15 @@ def run(args):
 
   port = args['port']
   host = args['host']
-  os.chdir(args['cwd'])
+  cwd = args['cwd']
+  root_path = args['root-path']
+
+  os.chdir(cwd)
 
   Handler = ServerHandler
-  Handler.document_path = args['root-dir']
-  Handler.restrict_root_folder = (args['root-dir'] != args['cwd'])
+  Handler.base_path = cwd
+  Handler.root_path = root_path
+  Handler.restrict_path = (root_path != cwd)
   Handler.restrict_write = not args['allow-write']
 
   if valid_ipv6(host):

slapos/test/recipe/test_simplehttpserver.py

@@ -48,13 +48,15 @@ class SimpleHTTPServerTest(unittest.TestCase):
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
     )
-    server_base_url = urlparse.urljoin(
-        self.server_url,
-        self.recipe.options['path'],
-    )
+    root_path = self.recipe.options.get('root-path')
+    if root_path:
+      root_relpath = os.path.relpath(root_path, self.base_path)
+      server_ok_url = urlparse.urljoin(self.server_url, root_relpath)
+    else:
+      server_ok_url = self.server_url
     for i in range(16):
       try:
-        resp = requests.get(server_base_url)
+        resp = requests.get(server_ok_url)
         break
       except requests.exceptions.ConnectionError:
         time.sleep(i * .1)
@@ -62,7 +64,7 @@ class SimpleHTTPServerTest(unittest.TestCase):
       self.fail(
           'server did not start.\nout: %s error: %s' % self.process.communicate())
     self.assertIn('Directory listing for /', resp.text)
-    return server_base_url
+    return self.server_url
 
   def tearDown(self):
     if self.process:
@@ -71,35 +73,17 @@ class SimpleHTTPServerTest(unittest.TestCase):
       self.process.communicate() # close pipes
       self.process = None
 
-  def test_options_use_hash(self):
-    self.setUpRecipe({'use-hash-url': 'true'})
-    self.assertNotEqual(self.recipe.options['path'], '')
-    self.assertEqual(
-        self.recipe.options['root-dir'],
-        os.path.join(
-            self.base_path,
-            self.recipe.options['path'],
-        ))
-
-  def test_options_no_hash(self):
-    self.setUpRecipe()
-    self.assertEqual(self.recipe.options['path'], '')
-    self.assertEqual(
-        self.recipe.options['root-dir'],
-        self.base_path
+  def test_options_valid_root_path(self):
+    self.assertRaises(
+      simplehttpserver.UserError,
+      self.setUpRecipe,
+      {'root-path': '/'}
     )
 
-  def test_write_outside_root_dir_should_fail(self):
-    self.setUpRecipe({'allow-write': 'true'})
-    server_base_url = self.startServer()
-
-    # A file outside the server's root directory
-    hack_path = os.path.join(self.install_dir, 'forbidden', 'hack.txt')
-    hack_content = "You should not be able to write to hack.txt"
-
+  def write_should_fail(self, url, hack_path, hack_content):
     # post with multipart/form-data encoding
     resp = requests.post(
-        server_base_url,
+        url,
         files={
             'path': hack_path,
             'content': hack_content,
@@ -115,11 +99,54 @@ class SimpleHTTPServerTest(unittest.TestCase):
     except IOError as e:
       if e.errno != errno.ENOENT:
         raise
-    self.assertFalse(os.path.exists(os.path.dirname(hack_path)))
     # Now check for proper response
     self.assertEqual(resp.status_code, requests.codes.forbidden)
     self.assertEqual(resp.text, 'Forbidden')
 
+  def test_write_outside_base_path_should_fail(self):
+    self.setUpRecipe({'allow-write': 'true'})
+    server_base_url = self.startServer()
+
+    # A file outside the server's root directory
+    hack_path = os.path.join(self.install_dir, 'forbidden', 'hack.txt')
+    hack_content = "You should not be able to write to hack.txt"
+
+    self.write_should_fail(server_base_url, hack_path, hack_content)
+    self.assertFalse(os.path.exists(os.path.dirname(hack_path)))
+
+  def test_write_outside_root_path_should_fail(self):
+    root_path = os.path.join(self.base_path, 'forbidden', 'allowed')
+    os.makedirs(root_path)
+    self.setUpRecipe({'allow-write': 'true', 'root-path': root_path})
+    server_base_url = self.startServer()
+
+    # A file outside the server's root directory
+    hack_path = os.path.join(self.base_path, 'forbidden', 'hack.txt')
+    hack_content = "You should not be able to write to hack.txt"
+
+    self.write_should_fail(server_base_url, hack_path, hack_content)
+
+  def test_access_outside_root_path_should_fail(self):
+    root_path = os.path.join(self.base_path, 'forbidden', 'allowed')
+    os.makedirs(root_path)
+    self.setUpRecipe({'root-path': root_path})
+
+    server_base_url = self.startServer()
+    forbiddenurl = os.path.join(server_base_url, 'forbidden')
+
+    resp = requests.get(forbiddenurl)
+    self.assertEqual(resp.status_code, requests.codes.forbidden)
+    self.assertEqual(resp.text, 'Forbidden')
+
+    resp = requests.post(
+        forbiddenurl,
+        files={
+            'path': 'index.txt',
+            'content': 'Not forbidden after all',
+        },
+    )
+    self.assertEqual(resp.status_code, requests.codes.forbidden)
+
   def test_write(self):
     self.setUpRecipe({'allow-write': 'true'})
     server_base_url = self.startServer()
@@ -135,8 +162,7 @@ class SimpleHTTPServerTest(unittest.TestCase):
     self.assertEqual(resp.status_code, requests.codes.ok)
     self.assertEqual(resp.text, 'Content written to hello-form-data.txt')
     with open(
-        os.path.join(self.base_path, self.recipe.options['path'],
-                     'hello-form-data.txt')) as f:
+        os.path.join(self.base_path, 'hello-form-data.txt')) as f:
       self.assertEqual(f.read(), 'hello-form-data')
 
     self.assertIn('hello-form-data.txt', requests.get(server_base_url).text)
@@ -153,8 +179,7 @@ class SimpleHTTPServerTest(unittest.TestCase):
     )
     self.assertEqual(resp.status_code, requests.codes.ok)
     with open(
-        os.path.join(self.base_path, self.recipe.options['path'],
-                     'hello-form-urlencoded.txt')) as f:
+        os.path.join(self.base_path, 'hello-form-urlencoded.txt')) as f:
       self.assertEqual(f.read(), 'hello-form-urlencoded')
 
     self.assertIn('hello-form-urlencoded.txt', requests.get(server_base_url).text)