openstack/swift
Code Issues Proposed changes

Fix up some Content-Type handling in account/container listings

Browse Source 
Update content type on 204 (not just 200) to properly handle HEAD
requests from xml/txt listings.

Add "Vary: Accept" header to listings, since otherwise, browsers may
serve the wrong content type from cache (even though we *would have*
sent the *right* type if it actually sent the request).

Change-Id: Iaa333aaca36a8dc2df65d38ef2173e3a6e2000ee
This commit is contained in:
Tim Burke 2018-01-10 06:16:41 +00:00
parent a5afe76758
commit 3b65a5998c
2 changed files with 73 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
swift/common/middleware/listing_formats.py
View File

@ -22,7 +22,7 @@ from swift.common.http import HTTP_NO_CONTENT
from swift.common.request_helpers import get_param
from swift.common.swob import HTTPException, HTTPNotAcceptable, Request, \
RESPONSE_REASONS, HTTPBadRequest, wsgi_quote, wsgi_to_bytes
from swift.common.utils import RESERVED, get_logger
from swift.common.utils import RESERVED, get_logger, list_from_csv
#: Mapping of query string ``format=`` values to their corresponding
@ -167,6 +167,7 @@ class ListingFilter(object):
return err(env, start_response)
params = req.params
can_vary = 'format' not in params
params['format'] = 'json'
req.params = params
@ -187,11 +188,22 @@ class ListingFilter(object):
elif header == 'content-length':
header_to_index[header] = i
resp_length = int(value)
elif header == 'vary':
header_to_index[header] = i
if not status.startswith('200 '):
if not status.startswith(('200 ', '204 ')):
start_response(status, headers)
return resp_iter
if can_vary:
if 'vary' in header_to_index:
value = headers[header_to_index['vary']][1]
if 'accept' not in list_from_csv(value.lower()):
headers[header_to_index['vary']] = (
'Vary', value + ', Accept')
else:
headers.append(('Vary', 'Accept'))
if resp_content_type != 'application/json':
start_response(status, headers)
return resp_iter

60
test/unit/common/middleware/test_listing_formats.py
View File

@ -16,7 +16,7 @@
import json
import unittest
from swift.common.swob import Request, HTTPOk
from swift.common.swob import Request, HTTPOk, HTTPNoContent
from swift.common.middleware import listing_formats
from swift.common.request_helpers import get_reserved_name
from test.unit import debug_logger
@ -106,6 +106,64 @@ class TestListingFormats(unittest.TestCase):
self.assertEqual(self.fake_swift.calls[-1], (
'GET', '/v1/a?format=json'))
def test_valid_content_type_on_txt_head(self):
self.fake_swift.register('HEAD', '/v1/a', HTTPNoContent, {
'Content-Length': str(len(self.fake_account_listing)),
'Content-Type': 'application/json'}, self.fake_account_listing)
req = Request.blank('/v1/a', method='HEAD')
resp = req.get_response(self.app)
self.assertEqual(resp.body, b'')
self.assertEqual(resp.headers['Content-Type'],
'text/plain; charset=utf-8')
self.assertIn('Vary', resp.headers)
# Even though the client didn't send an Accept header, the response
# could change *if a subsequent request does*, so include Vary: Accept
self.assertEqual(resp.headers['Vary'], 'Accept')
self.assertEqual(self.fake_swift.calls[-1], (
'HEAD', '/v1/a?format=json'))
def test_valid_content_type_on_xml_head(self):
self.fake_swift.register('HEAD', '/v1/a', HTTPNoContent, {
'Content-Length': str(len(self.fake_account_listing)),
'Content-Type': 'application/json'}, self.fake_account_listing)
req = Request.blank('/v1/a?format=xml', method='HEAD')
resp = req.get_response(self.app)
self.assertEqual(resp.body, b'')
self.assertEqual(resp.headers['Content-Type'],
'application/xml; charset=utf-8')
# query param overrides header, so it won't vary
self.assertNotIn('Vary', resp.headers)
self.assertEqual(self.fake_swift.calls[-1], (
'HEAD', '/v1/a?format=json'))
def test_update_vary_if_present(self):
self.fake_swift.register('HEAD', '/v1/a', HTTPNoContent, {
'Content-Length': str(len(self.fake_account_listing)),
'Content-Type': 'application/json',
'Vary': 'Origin'}, self.fake_account_listing)
req = Request.blank('/v1/a', method='HEAD')
resp = req.get_response(self.app)
self.assertEqual(resp.body, b'')
self.assertEqual(resp.headers['Content-Type'],
'text/plain; charset=utf-8')
self.assertEqual(resp.headers['Vary'], 'Origin, Accept')
self.assertEqual(self.fake_swift.calls[-1], (
'HEAD', '/v1/a?format=json'))
def test_update_vary_does_not_duplicate(self):
self.fake_swift.register('HEAD', '/v1/a', HTTPNoContent, {
'Content-Length': str(len(self.fake_account_listing)),
'Content-Type': 'application/json',
'Vary': 'Accept'}, self.fake_account_listing)
req = Request.blank('/v1/a', method='HEAD')
resp = req.get_response(self.app)
self.assertEqual(resp.body, b'')
self.assertEqual(resp.headers['Content-Type'],
'text/plain; charset=utf-8')
self.assertEqual(resp.headers['Vary'], 'Accept')
self.assertEqual(self.fake_swift.calls[-1], (
'HEAD', '/v1/a?format=json'))
def test_valid_account_with_reserved(self):
body_len = len(self.fake_account_listing_with_reserved)
self.fake_swift.register(