Skip to content

Commit

Permalink
hub: return error 404 for non-existent logs
Browse files Browse the repository at this point in the history
Otherwise, the response would yield an empty file and 200 status code.

Resolves: #49
  • Loading branch information
lzaoral committed Jul 31, 2023
1 parent 890d46e commit dfb639d
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 8 deletions.
16 changes: 8 additions & 8 deletions kobo/hub/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from django.urls import reverse
else:
from django.core.urlresolvers import reverse
from django.http import HttpResponse, StreamingHttpResponse, HttpResponseForbidden
from django.http import HttpResponse, HttpResponseNotFound, StreamingHttpResponse, HttpResponseForbidden
from django.shortcuts import render, get_object_or_404
from django.template import RequestContext
from django.views.generic import RedirectView
Expand Down Expand Up @@ -111,13 +111,8 @@ def get_context_data(self, **kwargs):
return context


def _stream_file(file_path, offset=0):
def _stream_file(f, offset=0):
"""Generator that returns 1M file chunks."""
try:
f = open(file_path, "rb")
except IOError:
return

f.seek(offset)
while 1:
data = f.read(1024 ** 2)
Expand Down Expand Up @@ -149,8 +144,13 @@ def _streamed_log_response(task, log_name, offset, as_attachment):
except OSError:
content_len = 0

try:
f = open(file_path, "rb")
except OSError:
return HttpResponseNotFound('Cannot find file ' + log_name)

# use _stream_file() instead of passing file object in order to improve performance
response = StreamingHttpResponse(_stream_file(file_path, offset), content_type=mimetype)
response = StreamingHttpResponse(_stream_file(f, offset), content_type=mimetype)
response["Content-Length"] = content_len

if as_attachment:
Expand Down
14 changes: 14 additions & 0 deletions tests/test_view_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,20 @@ def setUp(self):
# for more accurate memory_profiler tests
gc.collect()

def test_view_log_404(self):
"""Fetching of non-existent logs should yield error 404"""
response = self.get_log('missing.htm')
self.assertEqual(response.content, b'Cannot find file missing.htm')
self.assertEqual(response.status_code, 404)

response = self.get_log('missing.html')
self.assertEqual(response.content, b'Cannot find file missing.html')
self.assertEqual(response.status_code, 404)

response = self.get_log('missing.tar.gz', data={'format': 'raw'})
self.assertEqual(response.content, b'Cannot find file missing.tar.gz')
self.assertEqual(response.status_code, 404)

def test_view_zipped_small_raw(self):
"""Fetching a small compressed log with raw format should yield
the gzip-compressed content."""
Expand Down

0 comments on commit dfb639d

Please sign in to comment.