openstack
/
glance
Code Issues Proposed changes

Prepare for WebOb 1.8.1 

WebOb 1.8.1 removes a function (deprecated for a long time) that
Glance uses for negotiating 'Accept-Language' headers.  The removed
function is replaced by a function that implements the "Lookup"
scheme defined in RFC 4647.

Glance wraps the webob.Request object and provides a convenience
method around the old function.  This patch uses the new webob
function in the convenience method and wraps it in such a way that
the convenience method preserves its current behavior (at least with
respect to the Glance tests).  But ... "Lookup" is compliant with RFC
7231, whereas the old function was not.  This means that given
sufficiently complex Accept-Language header values (containing
q-values), the best match under webob 1.8.1 will be different from
the best match under webob 1.7.x.  Given that the best match will,
however, be RFC 7231 compliant, this is an acceptable change.

One further complication is that the lookup() function is not
available in webob 1.7.x.  Thus this patch handles both versions.  I
suggest that as soon as WebOb===1.8.1 is merged into
lower_constraints, the code be simplified to handle 1.8.x only, and
the following release note be submitted with that patch:

Negotiation of the 'Accept-Language' header now follows the "Lookup"
matching scheme described in `RFC 4647, section 3.4
<https://tools.ietf.org/html/rfc4647.html#section-3.4>`_.  The
"Lookup" scheme is one of the algorithms suggested in `RFC 7231,
section 5.3.5
<https://tools.ietf.org/html/rfc7231.html#section-5.3.5>`_.  (This is
due to a change in an underlying library, which previously used a
matching scheme that did not conform to `RFC 7231
<https://tools.ietf.org/html/rfc7231.html>`_.)

Change-Id: Ib9735b394a134de9a53d577b4d8b5a9c760b7780
Closes-bug: #1765748
This commit is contained in:
Brian Rosmaita 2018-04-27 16:46:14 -04:00
parent 75b180fe78
commit 73cc41c9ce
2 changed files with 66 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
glance/common/wsgi.py
View File

@ -53,6 +53,12 @@ from glance import i18n
from glance.i18n import _, _LE, _LI, _LW
try:
from webob.acceptparse import AcceptLanguageValidHeader # noqa
USING_WEBOB_1_8 = True
except ImportError:
USING_WEBOB_1_8 = False
bind_opts = [
cfg.HostAddressOpt('bind_host',
default='0.0.0.0',
@ -1024,7 +1030,7 @@ class Request(webob.Request):
else:
return content_type
def best_match_language(self):
def _best_match_language_1_7(self):
"""Determines best available locale from the Accept-Language header.
:returns: the best language match or None if the 'Accept-Language'
@ -1035,6 +1041,29 @@ class Request(webob.Request):
langs = i18n.get_available_languages('glance')
return self.accept_language.best_match(langs)
def _best_match_language_1_8(self):
"""Determines best available locale from the Accept-Language header.
:returns: the best language match or None if the 'Accept-Language'
header was not available in the request.
"""
if not self.accept_language:
return None
langs = i18n.get_available_languages('glance')
# NOTE(rosmaita): give the webob lookup() function a sentinal value
# for default so we can preserve the behavior of this function as
# indicated by the current unit tests. See Launchpad bug #1765748.
best_match = self.accept_language.lookup(langs, default='fake_LANG')
if best_match == 'fake_LANG':
best_match = None
return best_match
def best_match_language(self):
if USING_WEBOB_1_8:
return self._best_match_language_1_8()
else:
return self._best_match_language_1_7()
def get_range_from_request(self, image_size):
"""Return the `Range` in a request."""

49
glance/tests/unit/common/test_wsgi.py
View File

@ -184,16 +184,31 @@ class RequestTest(test_utils.BaseTestCase):
req = wsgi.Request.blank('/', headers={'Accept-Language': 'unknown'})
self.assertIsNone(req.best_match_language())
@mock.patch.object(webob.acceptparse.AcceptLanguage, 'best_match')
def test_best_match_language_unknown(self, mock_best_match):
def test_best_match_language_unknown(self):
# Test that we are actually invoking language negotiation by webop
request = wsgi.Request.blank('/')
accepted = 'unknown-lang'
request.headers = {'Accept-Language': accepted}
mock_best_match.return_value = None
# TODO(rosmaita): simplify when lower_constraints has webob >= 1.8.1
try:
from webob.acceptparse import AcceptLanguageValidHeader # noqa
cls = webob.acceptparse.AcceptLanguageValidHeader
funcname = 'lookup'
# Bug #1765748: see comment in code in the function under test
# to understand why this is the correct return value for the
# webob 1.8.x mock
retval = 'fake_LANG'
except ImportError:
cls = webob.acceptparse.AcceptLanguage
funcname = 'best_match'
retval = None
self.assertIsNone(request.best_match_language())
with mock.patch.object(cls, funcname) as mocked_function:
mocked_function.return_value = retval
self.assertIsNone(request.best_match_language())
mocked_function.assert_called_once()
# If Accept-Language is missing or empty, match should be None
request.headers = {'Accept-Language': ''}
@ -392,19 +407,27 @@ class ResourceTest(test_utils.BaseTestCase):
resource, request)
self.assertEqual(message_es, str(e))
@mock.patch.object(webob.acceptparse.AcceptLanguage, 'best_match')
@mock.patch.object(i18n, 'translate')
def test_translate_exception(self, mock_translate, mock_best_match):
def test_translate_exception(self, mock_translate):
# TODO(rosmaita): simplify when lower_constraints has webob >= 1.8.1
try:
from webob.acceptparse import AcceptLanguageValidHeader # noqa
cls = webob.acceptparse.AcceptLanguageValidHeader
funcname = 'lookup'
except ImportError:
cls = webob.acceptparse.AcceptLanguage
funcname = 'best_match'
mock_translate.return_value = 'No Encontrado'
mock_best_match.return_value = 'de'
with mock.patch.object(cls, funcname) as mocked_function:
mock_translate.return_value = 'No Encontrado'
mocked_function.return_value = 'de'
req = wsgi.Request.blank('/tests/123')
req.headers["Accept-Language"] = "de"
req = wsgi.Request.blank('/tests/123')
req.headers["Accept-Language"] = "de"
e = webob.exc.HTTPNotFound(explanation='Not Found')
e = wsgi.translate_exception(req, e)
self.assertEqual('No Encontrado', e.explanation)
e = webob.exc.HTTPNotFound(explanation='Not Found')
e = wsgi.translate_exception(req, e)
self.assertEqual('No Encontrado', e.explanation)
def test_response_headers_encoded(self):
# prepare environment