Skip to content

Commit

Permalink
Raise an exception on https redirects for PUT/POST
Browse files Browse the repository at this point in the history
POST and PUT requests are modified by clients when redirections happen.
A common problem with python-gitlab is a misconfiguration of the server
URL: the http to https redirection breaks some requests.

With this change python-gitlab should detect problematic redirections,
and raise a proper exception instead of failing with a cryptic error.

Closes #565
  • Loading branch information
Gauvain Pocentek committed Aug 24, 2018
1 parent 80a68f9 commit a221d7b
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 19 deletions.
48 changes: 29 additions & 19 deletions gitlab/__init__.py
Expand Up @@ -28,6 +28,7 @@
import gitlab.config
from gitlab.const import * # noqa
from gitlab.exceptions import * # noqa
from gitlab import utils # noqa

__title__ = 'python-gitlab'
__version__ = '1.5.1'
Expand All @@ -39,6 +40,9 @@
warnings.filterwarnings('default', category=DeprecationWarning,
module='^gitlab')

REDIRECT_MSG = ('python-gitlab detected an http to https redirection. You '
'must update your GitLab URL to use https:// to avoid issues.')


def _sanitize(value):
if isinstance(value, dict):
Expand Down Expand Up @@ -394,6 +398,26 @@ def _build_url(self, path):
else:
return '%s%s' % (self._url, path)

def _check_redirects(self, result):
# Check the requests history to detect http to https redirections.
# If the initial verb is POST, the next request will use a GET request,
# leading to an unwanted behaviour.
# If the initial verb is PUT, the data will not be send with the next
# request.
# If we detect a redirection to https with a POST or a PUT request, we
# raise an exception with a useful error message.
if result.history and self._base_url.startswith('http:'):
for item in result.history:
if item.status_code not in (301, 302):
continue
# GET methods can be redirected without issue
if result.request.method == 'GET':
continue
# Did we end-up with an https:// URL?
location = item.headers.get('Location', None)
if location and location.startswith('https://'):
raise RedirectError(REDIRECT_MSG)

def http_request(self, verb, path, query_data={}, post_data=None,
streamed=False, files=None, **kwargs):
"""Make an HTTP request to the Gitlab server.
Expand All @@ -417,27 +441,11 @@ def http_request(self, verb, path, query_data={}, post_data=None,
GitlabHttpError: When the return code is not 2xx
"""

def sanitized_url(url):
parsed = six.moves.urllib.parse.urlparse(url)
new_path = parsed.path.replace('.', '%2E')
return parsed._replace(path=new_path).geturl()

url = self._build_url(path)

def copy_dict(dest, src):
for k, v in src.items():
if isinstance(v, dict):
# Transform dict values in new attributes. For example:
# custom_attributes: {'foo', 'bar'} =>
# custom_attributes['foo']: 'bar'
for dict_k, dict_v in v.items():
dest['%s[%s]' % (k, dict_k)] = dict_v
else:
dest[k] = v

params = {}
copy_dict(params, query_data)
copy_dict(params, kwargs)
utils.copy_dict(params, query_data)
utils.copy_dict(params, kwargs)

opts = self._get_session_opts(content_type='application/json')

Expand All @@ -462,7 +470,7 @@ def copy_dict(dest, src):
req = requests.Request(verb, url, json=json, data=data, params=params,
files=files, **opts)
prepped = self.session.prepare_request(req)
prepped.url = sanitized_url(prepped.url)
prepped.url = utils.sanitized_url(prepped.url)
settings = self.session.merge_environment_settings(
prepped.url, {}, streamed, verify, None)

Expand All @@ -472,6 +480,8 @@ def copy_dict(dest, src):
while True:
result = self.session.send(prepped, timeout=timeout, **settings)

self._check_redirects(result)

if 200 <= result.status_code < 300:
return result

Expand Down
4 changes: 4 additions & 0 deletions gitlab/exceptions.py
Expand Up @@ -41,6 +41,10 @@ class GitlabAuthenticationError(GitlabError):
pass


class RedirectError(GitlabError):
pass


class GitlabParsingError(GitlabError):
pass

Expand Down
20 changes: 20 additions & 0 deletions gitlab/utils.py
Expand Up @@ -15,6 +15,8 @@
# You should have received a copy of the GNU Lesser General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

import six


class _StdoutStream(object):
def __call__(self, chunk):
Expand All @@ -31,3 +33,21 @@ def response_content(response, streamed, action, chunk_size):
for chunk in response.iter_content(chunk_size=chunk_size):
if chunk:
action(chunk)


def copy_dict(dest, src):
for k, v in src.items():
if isinstance(v, dict):
# Transform dict values to new attributes. For example:
# custom_attributes: {'foo', 'bar'} =>
# "custom_attributes['foo']": "bar"
for dict_k, dict_v in v.items():
dest['%s[%s]' % (k, dict_k)] = dict_v
else:
dest[k] = v


def sanitized_url(url):
parsed = six.moves.urllib.parse.urlparse(url)
new_path = parsed.path.replace('.', '%2E')
return parsed._replace(path=new_path).geturl()

0 comments on commit a221d7b

Please sign in to comment.