From a715c03eea200c3882ba2f60325cb154843db58b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Zaoral?= <lzaoral@redhat.com>
Date: Wed, 23 Aug 2023 09:59:06 +0200
Subject: [PATCH] hub: return error 404 for non-existent logs

Otherwise, the response would yield an empty file and 200 status code.

Resolves: https://github.com/release-engineering/kobo/issues/49
---
 kobo/hub/views.py      | 16 ++++++++--------
 tests/test_view_log.py | 14 ++++++++++++++
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/kobo/hub/views.py b/kobo/hub/views.py
index d109f00e..187f679e 100644
--- a/kobo/hub/views.py
+++ b/kobo/hub/views.py
@@ -13,7 +13,7 @@
 from django.conf import settings
 from django.contrib.auth import REDIRECT_FIELD_NAME, get_user_model
 from django.core.exceptions import ImproperlyConfigured
-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.urls import reverse
@@ -106,13 +106,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)
@@ -144,8 +139,13 @@ def _streamed_log_response(task, log_name, offset, as_attachment):
     except OSError:
         content_len = 0
 
+    try:
+        f = open(file_path, "rb")
+    except FileNotFoundError:
+        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:
diff --git a/tests/test_view_log.py b/tests/test_view_log.py
index 5f9ac5cb..9dd514c4 100644
--- a/tests/test_view_log.py
+++ b/tests/test_view_log.py
@@ -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."""