Skip to content

Commit

Permalink
views: detect if input stream was already read
Browse files Browse the repository at this point in the history
* In the case of having an external library which has already
  read the `request.stream` files REST fails with a server error
  since it will not be able to read the uploaded file (closes #197).

  This has happened with Sentry's `raven-python` library which
  reads the stream when the mimetype is `application/json` causing
  JSON files uploads to not work and throwing a misleading error.
  More here
  https://github.com/getsentry/raven-python/blob/719be9afae528962e683d086f9c2ceeda07f881b/raven/contrib/flask.py#L198
  • Loading branch information
Diego Rodriguez committed May 20, 2019
1 parent b052522 commit 4ba1908
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 0 deletions.
12 changes: 12 additions & 0 deletions invenio_files_rest/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,16 @@ def inner(self, bucket, key, upload_id, *args, **kwargs):
return decorate


def ensure_input_stream_is_not_exhausted(f):
"""Make sure that the input stream has not been read already."""
@wraps(f)
def decorate(*args, **kwargs):
if request.content_length and request.stream.is_exhausted:
abort(500)
return f(*args, **kwargs)
return decorate


#
# Permission checking
#
Expand Down Expand Up @@ -797,6 +807,7 @@ def get(self, bucket=None, key=None, version_id=None, upload_id=None,
@use_kwargs(post_args)
@pass_bucket
@need_bucket_permission('bucket-update')
@ensure_input_stream_is_not_exhausted
def post(self, bucket=None, key=None, uploads=missing, upload_id=None):
"""Upload a new object or start/complete a multipart upload.
Expand All @@ -815,6 +826,7 @@ def post(self, bucket=None, key=None, uploads=missing, upload_id=None):
@use_kwargs(put_args)
@pass_bucket
@need_bucket_permission('bucket-update')
@ensure_input_stream_is_not_exhausted
def put(self, bucket=None, key=None, upload_id=None):
"""Update a new object or upload a part of a multipart upload.
Expand Down
30 changes: 30 additions & 0 deletions tests/test_views_multipart.py
Original file line number Diff line number Diff line change
Expand Up @@ -543,3 +543,33 @@ def test_get_listuploads(client, db, bucket, multipart, multipart_url,
bucket_id=str(bucket.id),
) + '?uploads')
assert res.status_code == expected


def test_already_exhausted_input_stream(app, client, db, bucket, admin_user):
"""Test server error when file stream is already read."""
key = 'test.json'
data = b'{"json": "file"}'
object_url = url_for(
'invenio_files_rest.object_api', bucket_id=bucket.id, key=key)
# Add a new before request hook which reads the incoming request payload.
# This simulates what happens when Sentry's raven-python library when it
# reads the JSON payloads, breaking the upload of JSON files
# (`application/json`).

def consume_request_input_stream(*args):
"""Reads input stream object."""
from flask import request
request.data

app.before_request(consume_request_input_stream)
login_user(client, admin_user)
resp = client.put(
object_url,
input_stream=BytesIO(data),
)
assert resp.status_code == 500
resp = client.post(
object_url,
input_stream=BytesIO(data),
)
assert resp.status_code == 500

0 comments on commit 4ba1908

Please sign in to comment.