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
This commit is contained in:
parent
0d5f50b4e2
commit
a668411978
@ -25,6 +25,10 @@ from nodepool import zk
|
|||||||
class TestWebApp(tests.DBTestCase):
|
class TestWebApp(tests.DBTestCase):
|
||||||
log = logging.getLogger("nodepool.TestWebApp")
|
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):
|
def test_image_list(self):
|
||||||
configfile = self.setup_config('node.yaml')
|
configfile = self.setup_config('node.yaml')
|
||||||
pool = self.useNodepool(configfile, watermark_sleep=1)
|
pool = self.useNodepool(configfile, watermark_sleep=1)
|
||||||
@ -41,7 +45,18 @@ class TestWebApp(tests.DBTestCase):
|
|||||||
"http://localhost:%s/image-list" % port)
|
"http://localhost:%s/image-list" % port)
|
||||||
# NOTE(ianw): we want pretty printed text/plain back, but
|
# NOTE(ianw): we want pretty printed text/plain back, but
|
||||||
# simulating a normal web-browser request.
|
# 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)
|
f = request.urlopen(req)
|
||||||
self.assertEqual(f.info().get('Content-Type'),
|
self.assertEqual(f.info().get('Content-Type'),
|
||||||
'text/plain; charset=UTF-8')
|
'text/plain; charset=UTF-8')
|
||||||
@ -62,7 +77,7 @@ class TestWebApp(tests.DBTestCase):
|
|||||||
|
|
||||||
req = request.Request(
|
req = request.Request(
|
||||||
"http://localhost:%s/image-list?fields=id,image,state" % port)
|
"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)
|
f = request.urlopen(req)
|
||||||
self.assertEqual(f.info().get('Content-Type'),
|
self.assertEqual(f.info().get('Content-Type'),
|
||||||
'text/plain; charset=UTF-8')
|
'text/plain; charset=UTF-8')
|
||||||
@ -152,8 +167,8 @@ class TestWebApp(tests.DBTestCase):
|
|||||||
req = request.Request(
|
req = request.Request(
|
||||||
"http://localhost:%s/node-list?node_id=%s" % (port,
|
"http://localhost:%s/node-list?node_id=%s" % (port,
|
||||||
'0000000000'))
|
'0000000000'))
|
||||||
f = request.urlopen(req)
|
|
||||||
req.add_header('Accept', 'application/json')
|
req.add_header('Accept', 'application/json')
|
||||||
|
f = request.urlopen(req)
|
||||||
self.assertEqual(f.info().get('Content-Type'),
|
self.assertEqual(f.info().get('Content-Type'),
|
||||||
'application/json')
|
'application/json')
|
||||||
data = f.read()
|
data = f.read()
|
||||||
|
@ -117,9 +117,9 @@ class WebApp(threading.Thread):
|
|||||||
:param request: The incoming request
|
:param request: The incoming request
|
||||||
:return str: Best guess of either 'pretty' or 'json'
|
:return str: Best guess of either 'pretty' or 'json'
|
||||||
'''
|
'''
|
||||||
best = request.accept.best_match(
|
acceptable = request.accept.acceptable_offers(
|
||||||
['application/json', 'text/plain'])
|
['text/html', 'text/plain', 'application/json'])
|
||||||
if best == 'application/json':
|
if acceptable[0][0] == 'application/json':
|
||||||
return 'json'
|
return 'json'
|
||||||
else:
|
else:
|
||||||
return 'pretty'
|
return 'pretty'
|
||||||
|
@ -13,4 +13,4 @@ diskimage-builder>=2.0.0
|
|||||||
voluptuous
|
voluptuous
|
||||||
kazoo
|
kazoo
|
||||||
Paste
|
Paste
|
||||||
WebOb>=1.2.3
|
WebOb>=1.8.1
|
||||||
|
Loading…
x
Reference in New Issue
Block a user