Merge pull request #2917 from minrk/fix-auth-file-handler

register contents_manager.files_handler_class directly

Author: takluyver

notebook/files/handlers.py

@@ -12,12 +12,19 @@
     from base64 import decodestring as decodebytes
 
 
-from tornado import web, escape
+from tornado import web
 
 from notebook.base.handlers import IPythonHandler
 
+
 class FilesHandler(IPythonHandler):
-    """serve files via ContentsManager"""
+    """serve files via ContentsManager
+
+    Normally used when ContentsManager is not a FileContentsManager.
+
+    FileContentsManager subclasses use AuthenticatedFilesHandler by default,
+    a subclass of StaticFileHandler.
+    """
 
     @web.authenticated
     def head(self, path):
@@ -27,16 +34,10 @@ def head(self, path):
     def get(self, path, include_body=True):
         cm = self.contents_manager
 
-        if cm.files_handler_class:
-            return cm.files_handler_class(self.application, self.request, path=cm.root_dir)._execute(
-                [t(self.request) for t in self.application.transforms],
-                path
-            )
-
         if cm.is_hidden(path):
             self.log.info("Refusing to serve hidden file, via 404 Error")
             raise web.HTTPError(404)
-        
+
         path = path.strip('/')
         if '/' in path:
             _, name = path.rsplit('/', 1)
@@ -73,6 +74,5 @@ def get(self, path, include_body=True):
                 self.write(model['content'])
             self.flush()
 
-default_handlers = [
-    (r"/files/(.*)", FilesHandler),
-]
+
+default_handlers = []

notebook/notebookapp.py

@@ -277,7 +277,7 @@ def init_settings(self, jupyter_app, kernel_manager, contents_manager,
 
     def init_handlers(self, settings):
         """Load the (URL pattern, handler) tuples for each component."""
-        
+
         # Order matters. The first handler to match the URL will handle the request.
         handlers = []
         handlers.extend(load_handlers('tree.handlers'))
@@ -299,7 +299,8 @@ def init_handlers(self, settings):
         handlers.extend(load_handlers('services.kernelspecs.handlers'))
         handlers.extend(load_handlers('services.security.handlers'))
         handlers.extend(load_handlers('services.shutdown'))
-        
+        handlers.extend(settings['contents_manager'].get_extra_handlers())
+
         handlers.append(
             (r"/nbextensions/(.*)", FileFindHandler, {
                 'path': settings['nbextensions_path'],

notebook/services/contents/filemanager.py

@@ -153,6 +153,10 @@ def _checkpoints_class_default(self):
     def _files_handler_class_default(self):
         return AuthenticatedFileHandler
 
+    @default('files_handler_params')
+    def _files_handler_params_default(self):
+        return {'path': self.root_dir}
+
     def is_hidden(self, path):
         """Does the API style path correspond to a hidden directory or file?
 

notebook/services/contents/manager.py

@@ -10,8 +10,9 @@
 import os
 import re
 
-from tornado.web import HTTPError
+from tornado.web import HTTPError, RequestHandler
 
+from ...files.handlers import FilesHandler
 from .checkpoints import Checkpoints
 from traitlets.config.configurable import LoggingConfigurable
 from nbformat import sign, validate as validate_nb, ValidationError
@@ -131,7 +132,40 @@ def _default_checkpoints_kwargs(self):
             log=self.log,
         )
 
-    files_handler_class = Type(IPythonHandler, allow_none=True, config=True)
+    files_handler_class = Type(
+        FilesHandler, klass=RequestHandler, allow_none=True, config=True,
+        help="""handler class to use when serving raw file requests.
+
+        Default is a fallback that talks to the ContentsManager API,
+        which may be inefficient, especially for large files.
+
+        Local files-based ContentsManagers can use a StaticFileHandler subclass,
+        which will be much more efficient.
+
+        Access to these files should be Authenticated.
+        """
+    )
+
+    files_handler_params = Dict(
+        config=True,
+        help="""Extra parameters to pass to files_handler_class.
+
+        For example, StaticFileHandlers generally expect a `path` argument
+        specifying the root directory from which to serve files.
+        """
+    )
+
+    def get_extra_handlers(self):
+        """Return additional handlers
+
+        Default: self.files_handler_class on /files/.*
+        """
+        handlers = []
+        if self.files_handler_class:
+            handlers.append(
+                (r"/files/(.*)", self.files_handler_class, self.files_handler_params)
+            )
+        return handlers
 
     # ContentsManager API part 1: methods that must be
     # implemented in subclasses.