From be09169c67b355504d73a609f014c1b1e3c40804 Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <divius.inside@gmail.com>
Date: Tue, 28 Aug 2018 14:47:12 +0200
Subject: [PATCH] Prevent HTML from appearing in API error messages

Currently if the request does not have an Accept header, all pecan
errors end up being HTML, which then gets wrapped into our standard
error format. This change prevents it.

Change-Id: I1968ab20fdeced852744fac5e0e795e997ae6e34
Story: #1662228
Task: #24869
---
 ironic/api/middleware/parsable_error.py            |  5 +++++
 .../unit/api/controllers/v1/test_portgroup.py      |  3 +--
 ironic/tests/unit/api/test_root.py                 | 14 ++++++++++++++
 .../notes/html-errors-27579342e7e8183b.yaml        |  5 +++++
 4 files changed, 25 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/html-errors-27579342e7e8183b.yaml

diff --git a/ironic/api/middleware/parsable_error.py b/ironic/api/middleware/parsable_error.py
index 6e742c0692..37e7137a7e 100644
--- a/ironic/api/middleware/parsable_error.py
+++ b/ironic/api/middleware/parsable_error.py
@@ -64,6 +64,11 @@ class ParsableErrorMiddleware(object):
                 state['headers'] = headers
                 return start_response(status, headers, exc_info)
 
+        # The default for ironic is application/json. However, Pecan will try
+        # to output HTML errors if no Accept header is provided.
+        if 'HTTP_ACCEPT' not in environ or environ['HTTP_ACCEPT'] == '*/*':
+            environ['HTTP_ACCEPT'] = 'application/json'
+
         app_iter = self.app(environ, replacement_start_response)
         if (state['status_code'] // 100) not in (2, 3):
             req = webob.Request(environ)
diff --git a/ironic/tests/unit/api/controllers/v1/test_portgroup.py b/ironic/tests/unit/api/controllers/v1/test_portgroup.py
index 38cd447d55..cefe8d4812 100644
--- a/ironic/tests/unit/api/controllers/v1/test_portgroup.py
+++ b/ironic/tests/unit/api/controllers/v1/test_portgroup.py
@@ -400,8 +400,7 @@ class TestListPortgroups(test_api_base.BaseApiTest):
                                  expect_errors=True,
                                  headers={api_base.Version.string: '1.23'})
         self.assertEqual(http_client.NOT_FOUND, response.status_int)
-        self.assertIn('The resource could not be found.',
-                      response.json['error_message'])
+        self.assertIn('Not Found', response.json['error_message'])
 
     def test_ports_subresource_portgroup_not_found(self):
         non_existent_uuid = 'eeeeeeee-cccc-aaaa-bbbb-cccccccccccc'
diff --git a/ironic/tests/unit/api/test_root.py b/ironic/tests/unit/api/test_root.py
index 3ca47fae45..bb70a0c830 100644
--- a/ironic/tests/unit/api/test_root.py
+++ b/ironic/tests/unit/api/test_root.py
@@ -13,6 +13,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from six.moves import http_client
+
 from ironic.api.controllers.v1 import versions
 from ironic.tests.unit.api import base
 
@@ -35,6 +37,18 @@ class TestRoot(base.BaseApiTest):
                          version1['min_version'])
         self.assertEqual(versions.max_version_string(), version1['version'])
 
+    def test_no_html_errors(self):
+        response = self.get_json('/foo', expect_errors=True)
+        self.assertEqual(http_client.NOT_FOUND, response.status_int)
+        self.assertIn('Not Found', response.json['error_message'])
+        self.assertNotIn('<html', response.json['error_message'])
+
+    def test_no_html_errors2(self):
+        response = self.delete('/v1', expect_errors=True)
+        self.assertEqual(http_client.METHOD_NOT_ALLOWED, response.status_int)
+        self.assertIn('Not Allowed', response.json['error_message'])
+        self.assertNotIn('<html', response.json['error_message'])
+
 
 class TestV1Root(base.BaseApiTest):
 
diff --git a/releasenotes/notes/html-errors-27579342e7e8183b.yaml b/releasenotes/notes/html-errors-27579342e7e8183b.yaml
new file mode 100644
index 0000000000..42cd99ae9d
--- /dev/null
+++ b/releasenotes/notes/html-errors-27579342e7e8183b.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    The bare metal API no longer returns HTML as part of the ``error_message``
+    field in error responses when no ``Accept`` header is provided.