Ensure correct content-type in container HEAD response
A failing CORS test in the gate discovered that we were responding application/json to ?format=txt requests (which is maybe not even a valid value for that qs param?), but only when running with eventlet==0.38.0 This avoids the problem of backend container server HEADs no longer having 'Content-Length: 0' by fixing the client HEAD resp headers before we check for chunked-transfer resp. Drive-By: refactor listing_formats to use HeaderKeyDict and always set Content-Length explicitly Co-Authored-By: Alistair Coles <alistairncoles@gmail.com> Co-Authored-By: Matthew Oliver <matt@oliver.net.au> Change-Id: If724485e1425d1481d10b9255436301e346f07e8
This commit is contained in:
Clay Gerrard
Matthew Oliver
parent
ffbf17e47c
commit
fa889358ac
@ -18,6 +18,7 @@ import six
|
||||
from xml.etree.cElementTree import Element, SubElement, tostring
|
||||
|
||||
from swift.common.constraints import valid_api_version
|
||||
from swift.common.header_key_dict import HeaderKeyDict
|
||||
from swift.common.http import HTTP_NO_CONTENT
|
||||
from swift.common.request_helpers import get_param
|
||||
from swift.common.swob import HTTPException, HTTPNotAcceptable, Request, \
|
||||
@ -178,52 +179,39 @@ class ListingFilter(object):
|
||||
start_response(status, headers)
|
||||
return resp_iter
|
||||
|
||||
header_to_index = {}
|
||||
resp_content_type = resp_length = None
|
||||
for i, (header, value) in enumerate(headers):
|
||||
header = header.lower()
|
||||
if header == 'content-type':
|
||||
header_to_index[header] = i
|
||||
resp_content_type = value.partition(';')[0]
|
||||
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 ', '204 ')):
|
||||
start_response(status, headers)
|
||||
return resp_iter
|
||||
|
||||
headers_dict = HeaderKeyDict(headers)
|
||||
resp_content_type = headers_dict.get(
|
||||
'content-type', '').partition(';')[0]
|
||||
resp_length = headers_dict.get('content-length')
|
||||
|
||||
if can_vary:
|
||||
if 'vary' in header_to_index:
|
||||
value = headers[header_to_index['vary']][1]
|
||||
if 'vary' in headers_dict:
|
||||
value = headers_dict['vary']
|
||||
if 'accept' not in list_from_csv(value.lower()):
|
||||
headers[header_to_index['vary']] = (
|
||||
'Vary', value + ', Accept')
|
||||
headers_dict['vary'] = value + ', Accept'
|
||||
else:
|
||||
headers.append(('Vary', 'Accept'))
|
||||
headers_dict['vary'] = 'Accept'
|
||||
|
||||
if resp_content_type != 'application/json':
|
||||
start_response(status, headers)
|
||||
start_response(status, list(headers_dict.items()))
|
||||
return resp_iter
|
||||
|
||||
if req.method == 'HEAD':
|
||||
headers_dict['content-type'] = out_content_type + '; charset=utf-8'
|
||||
# proxy logging (and maybe other mw?) seem to be good about
|
||||
# sticking this on HEAD/204 but we do it here to be responsible
|
||||
# and explicit
|
||||
headers_dict['content-length'] = 0
|
||||
start_response(status, list(headers_dict.items()))
|
||||
return resp_iter
|
||||
|
||||
if resp_length is None or \
|
||||
resp_length > MAX_CONTAINER_LISTING_CONTENT_LENGTH:
|
||||
start_response(status, headers)
|
||||
return resp_iter
|
||||
|
||||
def set_header(header, value):
|
||||
if value is None:
|
||||
del headers[header_to_index[header]]
|
||||
else:
|
||||
headers[header_to_index[header]] = (
|
||||
headers[header_to_index[header]][0], str(value))
|
||||
|
||||
if req.method == 'HEAD':
|
||||
set_header('content-type', out_content_type + '; charset=utf-8')
|
||||
set_header('content-length', None) # don't know, can't determine
|
||||
start_response(status, headers)
|
||||
int(resp_length) > MAX_CONTAINER_LISTING_CONTENT_LENGTH:
|
||||
start_response(status, list(headers_dict.items()))
|
||||
return resp_iter
|
||||
|
||||
body = b''.join(resp_iter)
|
||||
@ -237,7 +225,7 @@ class ListingFilter(object):
|
||||
except ValueError:
|
||||
# Static web listing that's returning invalid JSON?
|
||||
# Just pass it straight through; that's about all we *can* do.
|
||||
start_response(status, headers)
|
||||
start_response(status, list(headers_dict.items()))
|
||||
return [body]
|
||||
|
||||
if not req.allow_reserved_names:
|
||||
@ -257,16 +245,16 @@ class ListingFilter(object):
|
||||
body = json.dumps(listing).encode('ascii')
|
||||
except KeyError:
|
||||
# listing was in a bad format -- funky static web listing??
|
||||
start_response(status, headers)
|
||||
start_response(status, list(headers_dict.items()))
|
||||
return [body]
|
||||
|
||||
if not body:
|
||||
status = '%s %s' % (HTTP_NO_CONTENT,
|
||||
RESPONSE_REASONS[HTTP_NO_CONTENT][0])
|
||||
|
||||
set_header('content-type', out_content_type + '; charset=utf-8')
|
||||
set_header('content-length', len(body))
|
||||
start_response(status, headers)
|
||||
headers_dict['content-type'] = out_content_type + '; charset=utf-8'
|
||||
headers_dict['content-length'] = len(body)
|
||||
start_response(status, list(headers_dict.items()))
|
||||
return [body]
|
||||
|
||||
|
||||
|
||||
@ -21,7 +21,7 @@ from uuid import uuid4
|
||||
from string import ascii_letters
|
||||
|
||||
import six
|
||||
from six.moves import range
|
||||
from six.moves import range, urllib
|
||||
from swift.common.middleware.acl import format_acl
|
||||
from swift.common.utils import distribute_evenly
|
||||
|
||||
@ -99,6 +99,45 @@ class TestAccount(unittest.TestCase):
|
||||
new_metadata = self.get_meta().keys()
|
||||
self.clear_meta(new_metadata)
|
||||
|
||||
def test_GET_HEAD_content_type(self):
|
||||
|
||||
def send_req(url, token, parsed, conn, method, params):
|
||||
qs = '?%s' % urllib.parse.urlencode(params) if params else ''
|
||||
conn.request(method, parsed.path + qs, '', {'X-Auth-Token': token})
|
||||
return check_response(conn)
|
||||
|
||||
resp = retry(send_req, 'GET', {})
|
||||
self.assertEqual(resp.status, 204)
|
||||
self.assertEqual(resp.getheader('Content-Type'),
|
||||
'text/plain; charset=utf-8')
|
||||
self.assertEqual(resp.getheader('Content-Length'), '0')
|
||||
|
||||
resp = retry(send_req, 'HEAD', {})
|
||||
self.assertEqual(resp.status, 204)
|
||||
self.assertEqual(resp.getheader('Content-Type'),
|
||||
'text/plain; charset=utf-8')
|
||||
self.assertEqual(resp.getheader('Content-Length'), '0')
|
||||
|
||||
resp = retry(send_req, 'GET', {'format': 'json'})
|
||||
self.assertEqual(resp.status, 200)
|
||||
self.assertEqual(resp.getheader('Content-Type'),
|
||||
'application/json; charset=utf-8')
|
||||
resp = retry(send_req, 'HEAD', {'format': 'json'})
|
||||
self.assertEqual(resp.status, 204)
|
||||
self.assertEqual(resp.getheader('Content-Type'),
|
||||
'application/json; charset=utf-8')
|
||||
self.assertEqual(resp.getheader('Content-Length'), '0')
|
||||
|
||||
resp = retry(send_req, 'GET', {'format': 'xml'})
|
||||
self.assertEqual(resp.status, 200)
|
||||
self.assertEqual(resp.getheader('Content-Type'),
|
||||
'application/xml; charset=utf-8')
|
||||
resp = retry(send_req, 'HEAD', {'format': 'xml'})
|
||||
self.assertEqual(resp.status, 204)
|
||||
self.assertEqual(resp.getheader('Content-Type'),
|
||||
'application/xml; charset=utf-8')
|
||||
self.assertEqual(resp.getheader('Content-Length'), '0')
|
||||
|
||||
def test_metadata(self):
|
||||
if tf.skip:
|
||||
raise SkipTest
|
||||
|
||||
@ -24,7 +24,7 @@ from test.functional import check_response, cluster_info, retry, \
|
||||
import test.functional as tf
|
||||
|
||||
import six
|
||||
from six.moves import range
|
||||
from six.moves import range, urllib
|
||||
|
||||
|
||||
def setUpModule():
|
||||
@ -111,6 +111,70 @@ class TestContainer(unittest.TestCase):
|
||||
# retry despite the request having been successfully processed.
|
||||
self.assertIn(resp.status, (204, 404))
|
||||
|
||||
def test_GET_HEAD_content_type(self):
|
||||
|
||||
def send_req(url, token, parsed, conn, method, container, params):
|
||||
qs = '?%s' % urllib.parse.urlencode(params) if params else ''
|
||||
conn.request(method, parsed.path + '/' + container + qs, '',
|
||||
{'X-Auth-Token': token})
|
||||
return check_response(conn)
|
||||
|
||||
resp = retry(send_req, 'GET', self.name, {})
|
||||
# GET is still 204 if there's no objects!?
|
||||
self.assertEqual(resp.status, 204)
|
||||
# we respond text/plain by default
|
||||
self.assertEqual(resp.getheader('Content-Type'),
|
||||
'text/plain; charset=utf-8')
|
||||
self.assertEqual(resp.getheader('Content-Length'), '0')
|
||||
|
||||
resp = retry(send_req, 'HEAD', self.name, {})
|
||||
# HEAD will *always* 204
|
||||
self.assertEqual(resp.status, 204)
|
||||
self.assertEqual(resp.getheader('Content-Type'),
|
||||
'text/plain; charset=utf-8')
|
||||
self.assertEqual(resp.getheader('Content-Length'), '0')
|
||||
|
||||
def put_object(url, token, parsed, conn, container, obj_name):
|
||||
conn.request('PUT', '/'.join((parsed.path, container, obj_name)),
|
||||
'', {'X-Auth-Token': token})
|
||||
return check_response(conn)
|
||||
|
||||
resp = retry(put_object, self.name, 'obj1')
|
||||
self.assertEqual(resp.status, 201)
|
||||
|
||||
resp = retry(send_req, 'GET', self.name, {})
|
||||
self.assertEqual(resp.status, 200)
|
||||
self.assertEqual(resp.getheader('Content-Type'),
|
||||
'text/plain; charset=utf-8')
|
||||
|
||||
resp = retry(send_req, 'HEAD', self.name, {})
|
||||
# HEAD will *always* 204
|
||||
self.assertEqual(resp.status, 204)
|
||||
self.assertEqual(resp.getheader('Content-Type'),
|
||||
'text/plain; charset=utf-8')
|
||||
self.assertEqual(resp.getheader('Content-Length'), '0')
|
||||
|
||||
# and we can ask for our preferred encoding format
|
||||
resp = retry(send_req, 'GET', self.name, {'format': 'json'})
|
||||
self.assertEqual(resp.status, 200)
|
||||
self.assertEqual(resp.getheader('Content-Type'),
|
||||
'application/json; charset=utf-8')
|
||||
resp = retry(send_req, 'HEAD', self.name, {'format': 'json'})
|
||||
self.assertEqual(resp.status, 204)
|
||||
self.assertEqual(resp.getheader('Content-Type'),
|
||||
'application/json; charset=utf-8')
|
||||
self.assertEqual(resp.getheader('Content-Length'), '0')
|
||||
|
||||
resp = retry(send_req, 'GET', self.name, {'format': 'xml'})
|
||||
self.assertEqual(resp.status, 200)
|
||||
self.assertEqual(resp.getheader('Content-Type'),
|
||||
'application/xml; charset=utf-8')
|
||||
resp = retry(send_req, 'HEAD', self.name, {'format': 'xml'})
|
||||
self.assertEqual(resp.status, 204)
|
||||
self.assertEqual(resp.getheader('Content-Type'),
|
||||
'application/xml; charset=utf-8')
|
||||
self.assertEqual(resp.getheader('Content-Length'), '0')
|
||||
|
||||
def test_multi_metadata(self):
|
||||
if tf.skip:
|
||||
raise SkipTest
|
||||
|
||||
@ -16,6 +16,7 @@
|
||||
import json
|
||||
import unittest
|
||||
|
||||
from swift.common.header_key_dict import HeaderKeyDict
|
||||
from swift.common.swob import Request, HTTPOk, HTTPNoContent
|
||||
from swift.common.middleware import listing_formats
|
||||
from swift.common.request_helpers import get_reserved_name
|
||||
@ -108,8 +109,8 @@ class TestListingFormats(unittest.TestCase):
|
||||
|
||||
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)
|
||||
'Content-Length': '0',
|
||||
'Content-Type': 'application/json'}, b'')
|
||||
req = Request.blank('/v1/a', method='HEAD')
|
||||
resp = req.get_response(self.app)
|
||||
self.assertEqual(resp.body, b'')
|
||||
@ -122,10 +123,48 @@ class TestListingFormats(unittest.TestCase):
|
||||
self.assertEqual(self.fake_swift.calls[-1], (
|
||||
'HEAD', '/v1/a?format=json'))
|
||||
|
||||
def test_text_content_type_on_invalid_format_qs(self):
|
||||
self.fake_swift.register('HEAD', '/v1/a/c', HTTPNoContent, {
|
||||
'Content-Type': 'application/json'}, b'')
|
||||
req = Request.blank('/v1/a/c?format=foo', method='HEAD')
|
||||
resp = req.get_response(self.app)
|
||||
self.assertEqual(resp.body, b'')
|
||||
self.assertEqual(resp.headers['Content-Length'], '0')
|
||||
self.assertEqual(resp.headers['Content-Type'],
|
||||
'text/plain; charset=utf-8')
|
||||
self.assertEqual(self.fake_swift.calls[-1], (
|
||||
'HEAD', '/v1/a/c?format=json'))
|
||||
|
||||
def test_accept_content_type_on_missing_qs(self):
|
||||
self.fake_swift.register('HEAD', '/v1/a/c', HTTPNoContent, {
|
||||
'Content-Type': 'application/json'}, b'')
|
||||
req = Request.blank('/v1/a/c', method='HEAD',
|
||||
headers={'Accept': 'application/xml'})
|
||||
resp = req.get_response(self.app)
|
||||
self.assertEqual(resp.body, b'')
|
||||
self.assertEqual(resp.headers['Content-Length'], '0')
|
||||
self.assertEqual(resp.headers['Content-Type'],
|
||||
'application/xml; charset=utf-8')
|
||||
self.assertEqual(self.fake_swift.calls[-1], (
|
||||
'HEAD', '/v1/a/c?format=json'))
|
||||
|
||||
def test_accept_ignored_on_invalid_qs(self):
|
||||
self.fake_swift.register('HEAD', '/v1/a/c', HTTPNoContent, {
|
||||
'Content-Type': 'application/json'}, b'')
|
||||
req = Request.blank('/v1/a/c?format=foo', method='HEAD',
|
||||
headers={'Accept': 'application/xml'})
|
||||
resp = req.get_response(self.app)
|
||||
self.assertEqual(resp.body, b'')
|
||||
self.assertEqual(resp.headers['Content-Length'], '0')
|
||||
self.assertEqual(resp.headers['Content-Type'],
|
||||
'text/plain; charset=utf-8')
|
||||
self.assertEqual(self.fake_swift.calls[-1], (
|
||||
'HEAD', '/v1/a/c?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)
|
||||
'Content-Length': '0',
|
||||
'Content-Type': 'application/json'}, b'')
|
||||
req = Request.blank('/v1/a?format=xml', method='HEAD')
|
||||
resp = req.get_response(self.app)
|
||||
self.assertEqual(resp.body, b'')
|
||||
@ -136,11 +175,28 @@ class TestListingFormats(unittest.TestCase):
|
||||
self.assertEqual(self.fake_swift.calls[-1], (
|
||||
'HEAD', '/v1/a?format=json'))
|
||||
|
||||
def test_valid_content_type_on_xml_head_with_no_content_length(self):
|
||||
# note: eventlet 0.38.0 stopped including content-length with 204
|
||||
# responses
|
||||
self.fake_swift.register('HEAD', '/v1/a', HTTPNoContent, {
|
||||
'Content-Type': 'application/json'}, b'')
|
||||
req = Request.blank('/v1/a?format=xml', method='HEAD')
|
||||
status, headers, body = req.call_application(self.app)
|
||||
self.assertEqual(b''.join(body), b'')
|
||||
headers_dict = dict(headers)
|
||||
self.assertEqual(headers_dict.get('Content-Length'), '0', headers)
|
||||
self.assertEqual(headers_dict['Content-Type'],
|
||||
'application/xml; charset=utf-8')
|
||||
# query param overrides header, so it won't vary
|
||||
self.assertNotIn('Vary', HeaderKeyDict(headers_dict))
|
||||
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-Length': '0',
|
||||
'Content-Type': 'application/json',
|
||||
'Vary': 'Origin'}, self.fake_account_listing)
|
||||
'Vary': 'Origin'}, b'')
|
||||
req = Request.blank('/v1/a', method='HEAD')
|
||||
resp = req.get_response(self.app)
|
||||
self.assertEqual(resp.body, b'')
|
||||
@ -150,11 +206,28 @@ class TestListingFormats(unittest.TestCase):
|
||||
self.assertEqual(self.fake_swift.calls[-1], (
|
||||
'HEAD', '/v1/a?format=json'))
|
||||
|
||||
def test_add_vary_when_content_type_not_json(self):
|
||||
self.fake_swift.register('HEAD', '/v1/a', HTTPNoContent, {
|
||||
'Content-Length': '0',
|
||||
'Content-Type': 'text/plain'}, b'')
|
||||
req = Request.blank('/v1/a', method='HEAD')
|
||||
resp = req.get_response(self.app)
|
||||
self.assertEqual(resp.body, b'')
|
||||
# We actually returned early, we didn't change things in the
|
||||
# request, but added the vary to let the cache know this
|
||||
# request could vary based on Accept as we didn't pass in
|
||||
# a format.
|
||||
self.assertEqual(resp.headers['Content-Type'],
|
||||
'text/plain')
|
||||
self.assertEqual(resp.headers['Vary'], '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-Length': '0',
|
||||
'Content-Type': 'application/json',
|
||||
'Vary': 'Accept'}, self.fake_account_listing)
|
||||
'Vary': 'Accept'}, b'')
|
||||
req = Request.blank('/v1/a', method='HEAD')
|
||||
resp = req.get_response(self.app)
|
||||
self.assertEqual(resp.body, b'')
|
||||
|
||||
Loading…
Reference in New Issue
Block a user