Merge pull request #2162 from blackrock/master
Fix for uploading large files crashing the browser (issue #96)
commit
a42fa3f453
@ -0,0 +1,70 @@
|
||||
from notebook.services.contents.filemanager import FileContentsManager
|
||||
from contextlib import contextmanager
|
||||
from tornado import web
|
||||
import nbformat
|
||||
import base64
|
||||
import os, io
|
||||
|
||||
class LargeFileManager(FileContentsManager):
|
||||
"""Handle large file upload."""
|
||||
|
||||
def save(self, model, path=''):
|
||||
"""Save the file model and return the model with no content."""
|
||||
chunk = model.get('chunk', None)
|
||||
if chunk is not None:
|
||||
path = path.strip('/')
|
||||
|
||||
if 'type' not in model:
|
||||
raise web.HTTPError(400, u'No file type provided')
|
||||
if model['type'] != 'file':
|
||||
raise web.HTTPError(400, u'File type "{}" is not supported for large file transfer'.format(model['type']))
|
||||
if 'content' not in model and model['type'] != 'directory':
|
||||
raise web.HTTPError(400, u'No file content provided')
|
||||
|
||||
os_path = self._get_os_path(path)
|
||||
|
||||
try:
|
||||
if chunk == 1:
|
||||
self.log.debug("Saving %s", os_path)
|
||||
self.run_pre_save_hook(model=model, path=path)
|
||||
super(LargeFileManager, self)._save_file(os_path, model['content'], model.get('format'))
|
||||
else:
|
||||
self._save_large_file(os_path, model['content'], model.get('format'))
|
||||
except web.HTTPError:
|
||||
raise
|
||||
except Exception as e:
|
||||
self.log.error(u'Error while saving file: %s %s', path, e, exc_info=True)
|
||||
raise web.HTTPError(500, u'Unexpected error while saving file: %s %s' % (path, e))
|
||||
|
||||
model = self.get(path, content=False)
|
||||
|
||||
# Last chunk
|
||||
if chunk == -1:
|
||||
self.run_post_save_hook(model=model, os_path=os_path)
|
||||
return model
|
||||
else:
|
||||
return super(LargeFileManager, self).save(model, path)
|
||||
|
||||
def _save_large_file(self, os_path, content, format):
|
||||
"""Save content of a generic file."""
|
||||
if format not in {'text', 'base64'}:
|
||||
raise web.HTTPError(
|
||||
400,
|
||||
"Must specify format of file contents as 'text' or 'base64'",
|
||||
)
|
||||
try:
|
||||
if format == 'text':
|
||||
bcontent = content.encode('utf8')
|
||||
else:
|
||||
b64_bytes = content.encode('ascii')
|
||||
bcontent = base64.decodestring(b64_bytes)
|
||||
except Exception as e:
|
||||
raise web.HTTPError(
|
||||
400, u'Encoding error saving %s: %s' % (os_path, e)
|
||||
)
|
||||
|
||||
with self.perm_to_403(os_path):
|
||||
if os.path.islink(os_path):
|
||||
os_path = os.path.join(os.path.dirname(os_path), os.readlink(os_path))
|
||||
with io.open(os_path, 'ab') as f:
|
||||
f.write(bcontent)
|
||||
@ -0,0 +1,113 @@
|
||||
from unittest import TestCase
|
||||
from ipython_genutils.tempdir import TemporaryDirectory
|
||||
from ..largefilemanager import LargeFileManager
|
||||
import os
|
||||
from tornado import web
|
||||
|
||||
|
||||
def _make_dir(contents_manager, api_path):
|
||||
"""
|
||||
Make a directory.
|
||||
"""
|
||||
os_path = contents_manager._get_os_path(api_path)
|
||||
try:
|
||||
os.makedirs(os_path)
|
||||
except OSError:
|
||||
print("Directory already exists: %r" % os_path)
|
||||
|
||||
|
||||
class TestLargeFileManager(TestCase):
|
||||
|
||||
def setUp(self):
|
||||
self._temp_dir = TemporaryDirectory()
|
||||
self.td = self._temp_dir.name
|
||||
self.contents_manager = LargeFileManager(root_dir=self.td)
|
||||
|
||||
def make_dir(self, api_path):
|
||||
"""make a subdirectory at api_path
|
||||
|
||||
override in subclasses if contents are not on the filesystem.
|
||||
"""
|
||||
_make_dir(self.contents_manager, api_path)
|
||||
|
||||
def test_save(self):
|
||||
|
||||
cm = self.contents_manager
|
||||
# Create a notebook
|
||||
model = cm.new_untitled(type='notebook')
|
||||
name = model['name']
|
||||
path = model['path']
|
||||
|
||||
# Get the model with 'content'
|
||||
full_model = cm.get(path)
|
||||
# Save the notebook
|
||||
model = cm.save(full_model, path)
|
||||
assert isinstance(model, dict)
|
||||
self.assertIn('name', model)
|
||||
self.assertIn('path', model)
|
||||
self.assertEqual(model['name'], name)
|
||||
self.assertEqual(model['path'], path)
|
||||
|
||||
try:
|
||||
model = {'name': 'test', 'path': 'test', 'chunk': 1}
|
||||
cm.save(model, model['path'])
|
||||
except web.HTTPError as e:
|
||||
self.assertEqual('HTTP 400: Bad Request (No file type provided)', str(e))
|
||||
|
||||
try:
|
||||
model = {'name': 'test', 'path': 'test', 'chunk': 1, 'type': 'notebook'}
|
||||
cm.save(model, model['path'])
|
||||
except web.HTTPError as e:
|
||||
self.assertEqual('HTTP 400: Bad Request (File type "notebook" is not supported for large file transfer)', str(e))
|
||||
|
||||
try:
|
||||
model = {'name': 'test', 'path': 'test', 'chunk': 1, 'type': 'file'}
|
||||
cm.save(model, model['path'])
|
||||
except web.HTTPError as e:
|
||||
self.assertEqual('HTTP 400: Bad Request (No file content provided)', str(e))
|
||||
|
||||
try:
|
||||
model = {'name': 'test', 'path': 'test', 'chunk': 2, 'type': 'file',
|
||||
'content': u'test', 'format': 'json'}
|
||||
cm.save(model, model['path'])
|
||||
except web.HTTPError as e:
|
||||
self.assertEqual("HTTP 400: Bad Request (Must specify format of file contents as 'text' or 'base64')",
|
||||
str(e))
|
||||
|
||||
# Save model for different chunks
|
||||
model = {'name': 'test', 'path': 'test', 'type': 'file',
|
||||
'content': u'test==', 'format': 'text'}
|
||||
name = model['name']
|
||||
path = model['path']
|
||||
cm.save(model, path)
|
||||
|
||||
for chunk in (1, 2, -1):
|
||||
for fm in ('text', 'base64'):
|
||||
full_model = cm.get(path)
|
||||
full_model['chunk'] = chunk
|
||||
full_model['format'] = fm
|
||||
model_res = cm.save(full_model, path)
|
||||
assert isinstance(model_res, dict)
|
||||
|
||||
self.assertIn('name', model_res)
|
||||
self.assertIn('path', model_res)
|
||||
self.assertNotIn('chunk', model_res)
|
||||
self.assertEqual(model_res['name'], name)
|
||||
self.assertEqual(model_res['path'], path)
|
||||
|
||||
# Test in sub-directory
|
||||
# Create a directory and notebook in that directory
|
||||
sub_dir = '/foo/'
|
||||
self.make_dir('foo')
|
||||
model = cm.new_untitled(path=sub_dir, type='notebook')
|
||||
name = model['name']
|
||||
path = model['path']
|
||||
model = cm.get(path)
|
||||
|
||||
# Change the name in the model for rename
|
||||
model = cm.save(model, path)
|
||||
assert isinstance(model, dict)
|
||||
self.assertIn('name', model)
|
||||
self.assertIn('path', model)
|
||||
self.assertEqual(model['name'], 'Untitled.ipynb')
|
||||
self.assertEqual(model['path'], 'foo/Untitled.ipynb')
|
||||
Loading…
Reference in new issue