From ee029a9b927a41c028427f8afc1821ed914e6d47 Mon Sep 17 00:00:00 2001 From: Brian Rosmaita Date: Tue, 15 May 2018 16:52:58 -0400 Subject: [PATCH] Handle HTTP headers per RFC 8187 According to RFC 8187, HTTP headers should use 7-bit ASCII encoding. The glanceclient was encoding them as UTF-8, which can leave the 8th bit nonzero when representing unicode, and which presents problems for any recipient following the standard and decoding the headers as ASCII. This change requires keystoneauth1 3.6.2, which has a fix for a bug that made it unable to handle bytes in headers. The dependency is a patch bumping the keystoneauth1 version in upper-constraints. Depends-on: https://review.openstack.org/#/c/569138/ Change-Id: I0d14974126fcb20e23a37347f4f1756c323cf2f5 Closes-bug: #1766235 --- glanceclient/common/http.py | 23 +++++++++++++++++++++-- glanceclient/tests/unit/test_http.py | 14 +++++++++++--- lower-constraints.txt | 2 +- requirements.txt | 2 +- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/glanceclient/common/http.py b/glanceclient/common/http.py index fc635fff..84cdc698 100644 --- a/glanceclient/common/http.py +++ b/glanceclient/common/http.py @@ -24,6 +24,7 @@ from oslo_utils import importutils from oslo_utils import netutils import requests import six +import six.moves.urllib.parse as urlparse try: import json @@ -53,8 +54,26 @@ def encode_headers(headers): :returns: Dictionary with encoded headers' names and values """ - return dict((encodeutils.safe_encode(h), encodeutils.safe_encode(v)) - for h, v in headers.items() if v is not None) + # NOTE(rosmaita): This function's rejection of any header name without a + # corresponding value is arguably justified by RFC 7230. In any case, that + # behavior was already here and there is an existing unit test for it. + + # Bug #1766235: According to RFC 8187, headers must be encoded as ASCII. + # So we first %-encode them to get them into range < 128 and then turn + # them into ASCII. + if six.PY2: + # incoming items may be unicode, so get them into something + # the py2 version of urllib can handle before percent encoding + encoded_dict = dict((urlparse.quote(encodeutils.safe_encode(h)), + urlparse.quote(encodeutils.safe_encode(v))) + for h, v in headers.items() if v is not None) + else: + encoded_dict = dict((urlparse.quote(h), urlparse.quote(v)) + for h, v in headers.items() if v is not None) + + return dict((encodeutils.safe_encode(h, encoding='ascii'), + encodeutils.safe_encode(v, encoding='ascii')) + for h, v in encoded_dict.items()) class _BaseHTTPClient(object): diff --git a/glanceclient/tests/unit/test_http.py b/glanceclient/tests/unit/test_http.py index cec94e05..efd15bfb 100644 --- a/glanceclient/tests/unit/test_http.py +++ b/glanceclient/tests/unit/test_http.py @@ -216,10 +216,15 @@ class TestClient(testtools.TestCase): def test_headers_encoding(self): value = u'ni\xf1o' - headers = {"test": value, "none-val": None} + headers = {"test": value, "none-val": None, "Name": "value"} encoded = http.encode_headers(headers) - self.assertEqual(b"ni\xc3\xb1o", encoded[b"test"]) + # Bug #1766235: According to RFC 8187, headers must be + # encoded as 7-bit ASCII, so expect to see only displayable + # chars in percent-encoding + self.assertEqual(b"ni%C3%B1o", encoded[b"test"]) self.assertNotIn("none-val", encoded) + self.assertNotIn(b"none-val", encoded) + self.assertEqual(b"value", encoded[b"Name"]) @mock.patch('keystoneauth1.adapter.Adapter.request') def test_http_duplicate_content_type_headers(self, mock_ksarq): @@ -466,4 +471,7 @@ class TestClient(testtools.TestCase): http_client.auth_token = unicode_token http_client.get(path) headers = self.mock.last_request.headers - self.assertEqual(b'ni\xc3\xb1o', headers['X-Auth-Token']) + # Bug #1766235: According to RFC 8187, headers must be + # encoded as 7-bit ASCII, so expect to see only displayable + # chars in percent-encoding + self.assertEqual(b'ni%C3%B1o', headers['X-Auth-Token']) diff --git a/lower-constraints.txt b/lower-constraints.txt index c3bea383..eac0111d 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -23,7 +23,7 @@ Jinja2==2.10 jsonpatch==1.16 jsonpointer==1.13 jsonschema==2.6.0 -keystoneauth1==3.4.0 +keystoneauth1==3.6.2 linecache2==1.0.0 MarkupSafe==1.0 mccabe==0.2.1 diff --git a/requirements.txt b/requirements.txt index e849053f..25b670a0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,7 @@ # process, which may cause wedges in the gate later. pbr!=2.1.0,>=2.0.0 # Apache-2.0 PrettyTable<0.8,>=0.7.1 # BSD -keystoneauth1>=3.4.0 # Apache-2.0 +keystoneauth1>=3.6.2 # Apache-2.0 requests>=2.14.2 # Apache-2.0 warlock<2,>=1.2.0 # Apache-2.0 six>=1.10.0 # MIT