Fix response code 301 processing
In _http_request() method if we got 301 status code after http request, we try to get a new location in a wrong way - (`resp['location']`). Now we use `getheader('location')` to new location from headers. Added tests to _http_request() method (was mocked in existing tests). Closes-Bug: #1246402 Change-Id: Ic69a27cd85299b0c1c60d9a2b9a870ad66152f6f
This commit is contained in:
parent
b44af3e40a
commit
efc9c0d51b
@ -121,6 +121,10 @@ class HTTPClient(object):
|
||||
LOG.debug('\n'.join(dump))
|
||||
|
||||
def _make_connection_url(self, url):
|
||||
# if we got absolute http path, we should do nothing with it
|
||||
if url.startswith('http://') or url.startswith('https://'):
|
||||
return url
|
||||
|
||||
(_class, _args, _kwargs) = self.connection_params
|
||||
base_url = _args[2]
|
||||
return '%s/%s' % (base_url.rstrip('/'), url.lstrip('/'))
|
||||
@ -167,7 +171,8 @@ class HTTPClient(object):
|
||||
raise exc.from_response(resp)
|
||||
elif resp.status in (301, 302, 305):
|
||||
# Redirected. Reissue the request to the new location.
|
||||
return self._http_request(resp['location'], method, **kwargs)
|
||||
new_location = resp.getheader('location')
|
||||
return self._http_request(new_location, method, **kwargs)
|
||||
elif resp.status == 300:
|
||||
raise exc.from_response(resp)
|
||||
|
||||
|
@ -16,6 +16,7 @@
|
||||
from tuskarclient.tests import utils as tutils
|
||||
|
||||
from tuskarclient.common import http
|
||||
from tuskarclient import exc
|
||||
|
||||
import mock
|
||||
|
||||
@ -113,3 +114,93 @@ class HttpClientRawRequestTest(tutils.TestCase):
|
||||
args['expected_args']['headers'] = {'Content-Type':
|
||||
'conflicting_header_value'}
|
||||
self.raw_request_calls_http_request(**args)
|
||||
|
||||
|
||||
class HttpClientHTTPRequestTest(tutils.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(HttpClientHTTPRequestTest, self).setUp()
|
||||
|
||||
self.client = http.HTTPClient('http://localhost')
|
||||
self.call_args = {
|
||||
'provided_method': 'GET',
|
||||
'expected_method': 'GET',
|
||||
'provided_url': '/',
|
||||
'expected_url': '/',
|
||||
'provided_args': {
|
||||
'headers': {
|
||||
'User-Agent': 'python-tuskarclient',
|
||||
},
|
||||
},
|
||||
'expected_args': {
|
||||
'headers': {
|
||||
'Content-Type': 'application/octet-stream'},
|
||||
},
|
||||
}
|
||||
|
||||
self.mock_response = mock.MagicMock()
|
||||
self.mock_response.read = lambda *args: None
|
||||
|
||||
self.mock_response_2 = mock.MagicMock()
|
||||
self.mock_response_2.read = lambda *args: None
|
||||
|
||||
self.mock_request = mock.MagicMock()
|
||||
|
||||
self.client.get_connection = mock.MagicMock(
|
||||
return_value=self.mock_request,
|
||||
)
|
||||
|
||||
def test_raw_request_status_200(self):
|
||||
self.mock_request.getresponse = lambda: self.mock_response
|
||||
self.mock_response.status = 200
|
||||
|
||||
args = self.call_args.copy()
|
||||
resp, body_iter = self.client._http_request(
|
||||
args['provided_url'],
|
||||
args['provided_method'],
|
||||
**args['provided_args'])
|
||||
self.assertEqual(resp.status, 200)
|
||||
|
||||
def test_raw_request_status_300(self):
|
||||
self.mock_request.getresponse = lambda: self.mock_response
|
||||
self.mock_response.status = 300
|
||||
|
||||
args = self.call_args.copy()
|
||||
self.assertRaises(exc.HTTPMultipleChoices, self.client._http_request,
|
||||
args['provided_url'], args['provided_method'],
|
||||
**args['provided_args'])
|
||||
|
||||
def test_raw_request_status_301(self):
|
||||
new_location = 'http://new_location.com'
|
||||
self.mock_response.getheader.return_value = new_location
|
||||
self.mock_response.status = 301
|
||||
self.mock_response_2.status = 200
|
||||
|
||||
self.mock_request.getresponse.side_effect = [
|
||||
self.mock_response, self.mock_response_2]
|
||||
|
||||
args = self.call_args.copy()
|
||||
|
||||
resp, body_iter = self.client._http_request(
|
||||
args['provided_url'],
|
||||
args['provided_method'],
|
||||
**args['provided_args'])
|
||||
|
||||
self.assertEqual(self.mock_request.getresponse.call_count, 2)
|
||||
self.mock_response.getheader.assert_called_called_with('location')
|
||||
self.mock_request.request.assert_called_with(
|
||||
args['provided_method'],
|
||||
new_location,
|
||||
**args['provided_args']
|
||||
)
|
||||
self.assertEqual(resp.status, 200)
|
||||
|
||||
def test_raw_request_status_500(self):
|
||||
self.mock_request.getresponse = lambda: self.mock_response
|
||||
self.mock_response.status = 500
|
||||
|
||||
args = self.call_args.copy()
|
||||
self.assertRaises(exc.HTTPInternalServerError,
|
||||
self.client._http_request,
|
||||
args['provided_url'], args['provided_method'],
|
||||
**args['provided_args'])
|
||||
|
Loading…
x
Reference in New Issue
Block a user