From 1d7fd6c8b305351ba34280492fa3a9cf968cfbb8 Mon Sep 17 00:00:00 2001 From: mlcrazy Date: Tue, 6 Jun 2017 17:27:25 -0400 Subject: [PATCH] Fixes error swallowing in set_environ --- AUTHORS.rst | 1 + requests/utils.py | 14 +++++++------- tests/test_utils.py | 30 ++++++++++++++++++++++++++++-- 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index 7e0bddf0..0be2effc 100755 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -183,3 +183,4 @@ Patches and Suggestions - Shmuel Amar (`@shmuelamar `_) - Gary Wu (`@garywu `_) - Ryan Pineo (`@ryanpineo `_) +- Matt Liu (`@mlcrazy `_) diff --git a/requests/utils.py b/requests/utils.py index 25af9923..1e4960d7 100644 --- a/requests/utils.py +++ b/requests/utils.py @@ -612,18 +612,18 @@ def set_environ(env_name, value): the environment variable 'env_name'. If 'value' is None, do nothing""" - if value is not None: + value_changed = value is not None + if value_changed: old_value = os.environ.get(env_name) os.environ[env_name] = value try: yield finally: - if value is None: - return - if old_value is None: - del os.environ[env_name] - else: - os.environ[env_name] = old_value + if value_changed: + if old_value is None: + del os.environ[env_name] + else: + os.environ[env_name] = old_value def should_bypass_proxies(url, no_proxy): diff --git a/tests/test_utils.py b/tests/test_utils.py index 41858b37..b3f398ee 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import os +import copy from io import BytesIO import pytest @@ -17,7 +18,7 @@ from requests.utils import ( requote_uri, select_proxy, should_bypass_proxies, super_len, to_key_val_list, to_native_string, unquote_header_value, unquote_unreserved, - urldefragauth, add_dict_to_cookiejar) + urldefragauth, add_dict_to_cookiejar, set_environ) from requests._internal_utils import unicode_is_ascii from .compat import StringIO, cStringIO @@ -651,4 +652,29 @@ def test_should_bypass_proxies_win_registry(url, expected, override, monkeypatch.setenv('NO_PROXY', '') monkeypatch.setattr(winreg, 'OpenKey', OpenKey) monkeypatch.setattr(winreg, 'QueryValueEx', QueryValueEx) - assert should_bypass_proxies(url, no_proxy=None) == expected + + +@pytest.mark.parametrize( + 'env_name, value', ( + ('no_proxy', '192.168.0.0/24,127.0.0.1,localhost.localdomain'), + ('no_proxy', None), + ('a_new_key', '192.168.0.0/24,127.0.0.1,localhost.localdomain'), + ('a_new_key', None), + )) +def test_set_environ(env_name, value): + """Tests set_environ will set environ values and will restore the environ.""" + environ_copy = copy.deepcopy(os.environ) + with set_environ(env_name, value): + assert os.environ.get(env_name) == value + + assert os.environ == environ_copy + + +def test_set_environ_raises_exception(): + """Tests set_environ will raise exceptions in context when the + value parameter is None.""" + with pytest.raises(Exception) as exception: + with set_environ('test1', None): + raise Exception('Expected exception') + + assert 'Expected exception' in str(exception.value)