Skip to content

Commit

Permalink
Handle HTTP headers per RFC 8187
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ostackbrian committed May 17, 2018
1 parent b9c6db6 commit ee029a9
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 7 deletions.
23 changes: 21 additions & 2 deletions glanceclient/common/http.py
Expand Up @@ -24,6 +24,7 @@
from oslo_utils import netutils
import requests
import six
import six.moves.urllib.parse as urlparse

try:
import json
Expand Down Expand Up @@ -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):
Expand Down
14 changes: 11 additions & 3 deletions glanceclient/tests/unit/test_http.py
Expand Up @@ -216,10 +216,15 @@ def test_http_encoding(self):

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):
Expand Down Expand Up @@ -466,4 +471,7 @@ def test_expired_token_has_changed(self):
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'])
2 changes: 1 addition & 1 deletion lower-constraints.txt
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Expand Up @@ -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
Expand Down

0 comments on commit ee029a9

Please sign in to comment.