From 194746bfa4df9c5e4658f013648e2db0af2259a1 Mon Sep 17 00:00:00 2001
From: Ashok Argent-Katwala <48444234+ashokdelphia@users.noreply.github.com>
Date: Mon, 27 May 2019 10:31:49 -0400
Subject: [PATCH] Improve accept header handling for unknown types (#222)

Add 406 response for if the requested content type is not supported.
Add more tests to verify correct content type negotiation.
---
 health_check/views.py |  9 ++++++---
 tests/test_views.py   | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/health_check/views.py b/health_check/views.py
index 1d8ec3cb..8c89d56a 100644
--- a/health_check/views.py
+++ b/health_check/views.py
@@ -2,7 +2,7 @@
 import re
 from concurrent.futures import ThreadPoolExecutor
 
-from django.http import JsonResponse
+from django.http import HttpResponse, JsonResponse
 from django.views.decorators.cache import never_cache
 from django.views.generic import TemplateView
 
@@ -126,8 +126,11 @@ def _run(plugin):
                 return self.render_to_response(context, status=status_code)
             elif media.mime_type in ('application/json', 'application/*'):
                 return self.render_to_response_json(plugins, status_code)
-            else:
-                return self.render_to_response(context, status=status_code)
+        return HttpResponse(
+            'Not Acceptable: Supported content types: text/html, application/json',
+            status=406,
+            content_type='text/plain'
+        )
 
     def render_to_response_json(self, plugins, status):
         return JsonResponse(
diff --git a/tests/test_views.py b/tests/test_views.py
index b1df69a9..b35e7806 100644
--- a/tests/test_views.py
+++ b/tests/test_views.py
@@ -138,6 +138,18 @@ def run_check(self):
         plugin_dir.register(JSONSuccessBackend)
         response = client.get(self.url, HTTP_ACCEPT='application/json')
         assert response['content-type'] == 'application/json'
+        assert response.status_code == 200
+
+    def test_success_prefer_json(self, client):
+        class JSONSuccessBackend(BaseHealthCheckBackend):
+            def run_check(self):
+                pass
+
+        plugin_dir.reset()
+        plugin_dir.register(JSONSuccessBackend)
+        response = client.get(self.url, HTTP_ACCEPT='application/json; q=0.8, text/html; q=0.5')
+        assert response['content-type'] == 'application/json'
+        assert response.status_code == 200
 
     def test_success_accept_xhtml(self, client):
         class SuccessBackend(BaseHealthCheckBackend):
@@ -148,6 +160,7 @@ def run_check(self):
         plugin_dir.register(SuccessBackend)
         response = client.get(self.url, HTTP_ACCEPT='application/xhtml+xml')
         assert response['content-type'] == 'text/html; charset=utf-8'
+        assert response.status_code == 200
 
     def test_success_unsupported_accept(self, client):
         class SuccessBackend(BaseHealthCheckBackend):
@@ -157,7 +170,20 @@ def run_check(self):
         plugin_dir.reset()
         plugin_dir.register(SuccessBackend)
         response = client.get(self.url, HTTP_ACCEPT='application/octet-stream')
-        assert response['content-type'] == 'text/html; charset=utf-8'
+        assert response['content-type'] == 'text/plain'
+        assert response.status_code == 406
+        assert response.content == b'Not Acceptable: Supported content types: text/html, application/json'
+
+    def test_success_unsupported_and_supported_accept(self, client):
+        class SuccessBackend(BaseHealthCheckBackend):
+            def run_check(self):
+                pass
+
+        plugin_dir.reset()
+        plugin_dir.register(SuccessBackend)
+        response = client.get(self.url, HTTP_ACCEPT='application/octet-stream, application/json; q=0.9')
+        assert response['content-type'] == 'application/json'
+        assert response.status_code == 200
 
     def test_success_accept_order(self, client):
         class JSONSuccessBackend(BaseHealthCheckBackend):
@@ -171,6 +197,7 @@ def run_check(self):
             HTTP_ACCEPT='text/html, application/xhtml+xml, application/json; q=0.9, */*; q=0.1'
         )
         assert response['content-type'] == 'text/html; charset=utf-8'
+        assert response.status_code == 200
 
     def test_success_accept_order__reverse(self, client):
         class JSONSuccessBackend(BaseHealthCheckBackend):
@@ -181,6 +208,7 @@ def run_check(self):
         plugin_dir.register(JSONSuccessBackend)
         response = client.get(self.url, HTTP_ACCEPT='text/html; q=0.1, application/xhtml+xml; q=0.1, application/json')
         assert response['content-type'] == 'application/json'
+        assert response.status_code == 200
 
     def test_format_override(self, client):
         class JSONSuccessBackend(BaseHealthCheckBackend):
@@ -191,6 +219,7 @@ def run_check(self):
         plugin_dir.register(JSONSuccessBackend)
         response = client.get(self.url + '?format=json', HTTP_ACCEPT='text/html')
         assert response['content-type'] == 'application/json'
+        assert response.status_code == 200
 
     def test_format_no_accept_header(self, client):
         class JSONSuccessBackend(BaseHealthCheckBackend):