From 3b65a5998cc921d2763cf1a9ec1e40b88491262d Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Wed, 10 Jan 2018 06:16:41 +0000 Subject: [PATCH] Fix up some Content-Type handling in account/container listings 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 --- swift/common/middleware/listing_formats.py | 16 ++++- .../common/middleware/test_listing_formats.py | 60 ++++++++++++++++++- 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/swift/common/middleware/listing_formats.py b/swift/common/middleware/listing_formats.py index 926bdbf2c0..bf402ff75e 100644 --- a/swift/common/middleware/listing_formats.py +++ b/swift/common/middleware/listing_formats.py @@ -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 diff --git a/test/unit/common/middleware/test_listing_formats.py b/test/unit/common/middleware/test_listing_formats.py index 22cf3e55f7..7adb868669 100644 --- a/test/unit/common/middleware/test_listing_formats.py +++ b/test/unit/common/middleware/test_listing_formats.py @@ -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(