From a668411978dec9bd2549ab8b576e37e949fa6930 Mon Sep 17 00:00:00 2001
From: Ian Wienand <iwienand@redhat.com>
Date: Thu, 7 Jun 2018 11:09:26 +1000
Subject: [PATCH] webapp: fix browser return

This small match had a surprising number of issues...

Even though we're testing returning 'text/plain' when 'text/html' is
specified, it turns out the accept headers real browsers send actually
specify "*/*;q=0.8" which means that unmatched types will be given the
same, lower weight.

This means in the extant code, the best_match() for a normal browser
request (without 'text/plain') will think that 'application/json' and
'text/plain' are the same preference, and will take the first in the
list, which happens to be 'application/json'.  The result is that
*real* webrowsers are getting back json, when we wanted them to get a
human readable result.

Also, in the mean time webob has a new accept handling model, and
best_match() is deprecated anyway.  Update the requirements to the
latest WebOb (new handling was added in 1.8.0, 1.8.1 is a bugfix)

So now we use the new API, and list 'text/html' as an acceptable
offer, which ensures it will be chosen if a browser is requesting
things.  It still returns a text/plain table.

A check for this is added to the test suite.  Also a bug with setting
the headers after the request is fixed, which went unnoticed because
of the prior default behaviour.

Change-Id: I84094ca67b16ce9246507aa0010646ffc3e9dbff
---
 nodepool/tests/test_webapp.py | 21 ++++++++++++++++++---
 nodepool/webapp.py            |  6 +++---
 requirements.txt              |  2 +-
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/nodepool/tests/test_webapp.py b/nodepool/tests/test_webapp.py
index 8cc24104c..7fd7249c8 100644
--- a/nodepool/tests/test_webapp.py
+++ b/nodepool/tests/test_webapp.py
@@ -25,6 +25,10 @@ from nodepool import zk
 class TestWebApp(tests.DBTestCase):
     log = logging.getLogger("nodepool.TestWebApp")
 
+    # A standard browser accept header
+    browser_accept = (
+        'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8')
+
     def test_image_list(self):
         configfile = self.setup_config('node.yaml')
         pool = self.useNodepool(configfile, watermark_sleep=1)
@@ -41,7 +45,18 @@ class TestWebApp(tests.DBTestCase):
             "http://localhost:%s/image-list" % port)
         # NOTE(ianw): we want pretty printed text/plain back, but
         # simulating a normal web-browser request.
-        req.add_header('Accept', 'text/html')
+        req.add_header('Accept', self.browser_accept)
+        f = request.urlopen(req)
+        self.assertEqual(f.info().get('Content-Type'),
+                         'text/plain; charset=UTF-8')
+        data = f.read()
+        self.assertTrue('fake-image' in data.decode('utf8'))
+
+        # also ensure that text/plain works (might be hand-set by a
+        # command-line curl script, etc)
+        req = request.Request(
+            "http://localhost:%s/image-list" % port)
+        req.add_header('Accept', 'text/plain')
         f = request.urlopen(req)
         self.assertEqual(f.info().get('Content-Type'),
                          'text/plain; charset=UTF-8')
@@ -62,7 +77,7 @@ class TestWebApp(tests.DBTestCase):
 
         req = request.Request(
             "http://localhost:%s/image-list?fields=id,image,state" % port)
-        req.add_header('Accept', 'text/html')
+        req.add_header('Accept', self.browser_accept)
         f = request.urlopen(req)
         self.assertEqual(f.info().get('Content-Type'),
                          'text/plain; charset=UTF-8')
@@ -152,8 +167,8 @@ class TestWebApp(tests.DBTestCase):
         req = request.Request(
             "http://localhost:%s/node-list?node_id=%s" % (port,
                                                           '0000000000'))
-        f = request.urlopen(req)
         req.add_header('Accept', 'application/json')
+        f = request.urlopen(req)
         self.assertEqual(f.info().get('Content-Type'),
                          'application/json')
         data = f.read()
diff --git a/nodepool/webapp.py b/nodepool/webapp.py
index 68ab269dd..28947115e 100644
--- a/nodepool/webapp.py
+++ b/nodepool/webapp.py
@@ -117,9 +117,9 @@ class WebApp(threading.Thread):
         :param request: The incoming request
         :return str: Best guess of either 'pretty' or 'json'
         '''
-        best = request.accept.best_match(
-            ['application/json', 'text/plain'])
-        if best == 'application/json':
+        acceptable = request.accept.acceptable_offers(
+            ['text/html', 'text/plain', 'application/json'])
+        if acceptable[0][0] == 'application/json':
             return 'json'
         else:
             return 'pretty'
diff --git a/requirements.txt b/requirements.txt
index 156589828..54f795723 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -13,4 +13,4 @@ diskimage-builder>=2.0.0
 voluptuous
 kazoo
 Paste
-WebOb>=1.2.3
+WebOb>=1.8.1