openstack/python-glanceclient
Code Issues Proposed changes

v2: Content-Type: application/octet-stream header always added

Browse Source 
The bug: any existing Content-Type header cannot be found because the
call to headers.get() fails. Therefore we end up with two Content-Type
headers because a new one (applicaion/octet-stream) gets added
unconditionally. The cause: the strings (keys and values) in the headers
dict are converted from unicode sequences of type <str> to utf-8
sequences of type <bytes>. This happens in safe_encode()
(oslo_utils/encodeutils.py:66). <str> != <bytes> even if they appear to
have the same characters.

Hence, for python 3.x, _set_common_request_kwargs() adds content-type
to header even if custom content-type is set in the request.

This results in unsupported media type exception when glance client
is used with keystoneauth and python 3.x

The fix: follow the directions in encode_headers().
It says to do this just before sending the request. Honor this principle;
do not encode headers and then perform more business logic on them.

Change-Id: Idf6079b32f70bc171f5016467048e917d42f296d
Closes-bug: #1641239
Co-Authored-By: Pushkar Umaranikar <pushkar.umaranikar@intel.com>
This commit is contained in:
ckonstanski 2016-11-11 18:39:34 -07:00 committed by Eric Fried
parent 7df87fd4a2
commit 03900522d4
4 changed files with 136 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
glanceclient/common/http.py
View File

@ -315,16 +315,18 @@ class SessionClient(adapter.Adapter, _BaseHTTPClient):
super(SessionClient, self).__init__(session, **kwargs)
def request(self, url, method, **kwargs):
headers = encode_headers(kwargs.pop('headers', {}))
headers = kwargs.pop('headers', {})
kwargs['raise_exc'] = False
data = self._set_common_request_kwargs(headers, kwargs)
try:
resp = super(SessionClient, self).request(url,
method,
headers=headers,
data=data,
**kwargs)
# NOTE(pumaranikar): To avoid bug #1641239, no modification of
# headers should be allowed after encode_headers() is called.
resp = super(SessionClient,
self).request(url,
method,
headers=encode_headers(headers),
data=data,
**kwargs)
except ksa_exc.ConnectTimeout as e:
conn_url = self.get_endpoint(auth=kwargs.get('auth'))
conn_url = "%s/%s" % (conn_url.rstrip('/'), url.lstrip('/'))

31
glanceclient/tests/functional/base.py
View File

@ -10,8 +10,10 @@
# License for the specific language governing permissions and limitations
# under the License.
import glanceclient
from keystoneauth1 import loading
from keystoneauth1 import session
import os
import os_client_config
from tempest.lib.cli import base
@ -60,3 +62,30 @@ class ClientTestBase(base.ClientTestBase):
def glance(self, *args, **kwargs):
return self.clients.glance(*args,
**kwargs)
def glance_pyclient(self):
ks_creds = dict(
auth_url=self.creds["auth_url"],
username=self.creds["username"],
password=self.creds["password"],
project_name=self.creds["project_name"])
keystoneclient = self.Keystone(**ks_creds)
return self.Glance(keystoneclient)
class Keystone(object):
def __init__(self, **kwargs):
loader = loading.get_plugin_loader("password")
auth = loader.load_from_options(**kwargs)
self.session = session.Session(auth=auth)
class Glance(object):
def __init__(self, keystone, version="2"):
self.glance = glanceclient.Client(
version,
session=keystone.session)
def find(self, image_name):
for image in self.glance.images.list():
if image.name == image_name:
return image
return None

61
glanceclient/tests/functional/test_http_headers.py Normal file
View File

@ -0,0 +1,61 @@
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from glanceclient.tests.functional import base
import time
IMAGE = {"protected": False,
"disk_format": "qcow2",
"name": "glance_functional_test_image.img",
"visibility": "private",
"container_format": "bare"}
class HttpHeadersTest(base.ClientTestBase):
def test_encode_headers_python(self):
"""Test proper handling of Content-Type headers.
encode_headers() must be called as late as possible before a
request is sent. If this principle is violated, and if any
changes are made to the headers between encode_headers() and the
actual request (for instance a call to
_set_common_request_kwargs()), and if you're trying to set a
Content-Type that is not equal to application/octet-stream (the
default), it is entirely possible that you'll end up with two
Content-Type headers defined (yours plus
application/octet-stream). The request will go out the door with
only one of them chosen seemingly at random.
This test uses a call to update() because it sets a header such
as the following (this example may be subject to change):
Content-Type: application/openstack-images-v2.1-json-patch
This situation only occurs in python3. This test will never fail
in python2.
There is no test against the CLI because it swallows the error.
"""
# the failure is intermittent - try up to 6 times
for attempt in range(0, 6):
glanceclient = self.glance_pyclient()
image = glanceclient.find(IMAGE["name"])
if image:
glanceclient.glance.images.delete(image.id)
image = glanceclient.glance.images.create(name=IMAGE["name"])
self.assertTrue(image.status == "queued")
try:
image = glanceclient.glance.images.update(image.id,
disk_format="qcow2")
except Exception as e:
self.assertFalse("415 Unsupported Media Type" in e.details)
time.sleep(5)

36
glanceclient/tests/unit/test_http.py
View File

@ -20,6 +20,7 @@ import fixtures
from keystoneauth1 import session
from keystoneauth1 import token_endpoint
import mock
from oslo_utils import encodeutils
import requests
from requests_mock.contrib import fixture
import six
@ -207,6 +208,41 @@ class TestClient(testtools.TestCase):
self.assertEqual(b"ni\xc3\xb1o", encoded[b"test"])
self.assertNotIn("none-val", encoded)
@mock.patch('keystoneauth1.adapter.Adapter.request')
def test_http_duplicate_content_type_headers(self, mock_ksarq):
"""Test proper handling of Content-Type headers.
encode_headers() must be called as late as possible before a
request is sent. If this principle is violated, and if any
changes are made to the headers between encode_headers() and the
actual request (for instance a call to
_set_common_request_kwargs()), and if you're trying to set a
Content-Type that is not equal to application/octet-stream (the
default), it is entirely possible that you'll end up with two
Content-Type headers defined (yours plus
application/octet-stream). The request will go out the door with
only one of them chosen seemingly at random.
This situation only occurs in python3. This test will never fail
in python2.
"""
path = "/v2/images/my-image"
headers = {
"Content-Type": "application/openstack-images-v2.1-json-patch"
}
data = '[{"value": "qcow2", "path": "/disk_format", "op": "replace"}]'
self.mock.patch(self.endpoint + path)
sess_http_client = self._create_session_client()
sess_http_client.patch(path, headers=headers, data=data)
# Pull out the headers with which Adapter.request was invoked
ksarqh = mock_ksarq.call_args[1]['headers']
# Only one Content-Type header (of any text-type)
self.assertEqual(1, [encodeutils.safe_decode(key)
for key in ksarqh.keys()].count(u'Content-Type'))
# And it's the one we set
self.assertEqual(b"application/openstack-images-v2.1-json-patch",
ksarqh[b"Content-Type"])
def test_raw_request(self):
"""Verify the path being used for HTTP requests reflects accurately."""
headers = {"Content-Type": "text/plain"}