Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 29 additions & 23 deletions requests/sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -527,18 +527,16 @@ def request(self, method, url,
)
prep = self.prepare_request(req)

proxies = proxies or {}

settings = self.merge_environment_settings(
prep.url, proxies, stream, verify, cert
)

# Send the request.
send_kwargs = {
'timeout': timeout,
'allow_redirects': allow_redirects,
'verify': verify,
'proxies': proxies,
'stream': stream,
'cert': cert,
}
send_kwargs.update(settings)

# Send the request.
resp = self.send(prep, **send_kwargs)

return resp
Expand Down Expand Up @@ -628,23 +626,28 @@ def send(self, request, **kwargs):

:rtype: requests.Response
"""
# Set defaults that the hooks can utilize to ensure they always have
# the correct parameters to reproduce the previous request.
kwargs.setdefault('stream', self.stream)
kwargs.setdefault('verify', self.verify)
kwargs.setdefault('cert', self.cert)
kwargs.setdefault('proxies', self.rebuild_proxies(request, self.proxies))

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor Author

@mateusduboli mateusduboli Mar 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before #5681, the send method was not using the environment variables at all, this was here to ensure that those would be read at some point.

However, this MR also moves the call for merge_environment_settings to send, thus rebuilding proxies is not necessary. Before, it was only issued in the request method.


# It's possible that users might accidentally send a Request object.
# Guard against that specific failure case.
if isinstance(request, Request):
raise ValueError('You can only send PreparedRequests.')

# Set defaults that the hooks can utilize to ensure they always have
# the correct parameters to reproduce the previous request.
stream = kwargs.get('stream')
verify = kwargs.get('verify')
cert = kwargs.get('cert')
proxies = kwargs.get('proxies')

# Set up variables needed for resolve_redirects and dispatching of hooks
allow_redirects = kwargs.pop('allow_redirects', True)
stream = kwargs.get('stream')
hooks = request.hooks

# Merge with environment variables
settings = self.merge_environment_settings(
request.url, proxies, stream, verify, cert
)
kwargs.update(settings)
Copy link
Copy Markdown
Contributor Author

@mateusduboli mateusduboli Jan 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I don't like to reuse kwargs too much, but it reduce the amount of changes


# Get the appropriate adapter to use
adapter = self.get_adapter(url=request.url)

Expand Down Expand Up @@ -704,6 +707,15 @@ def merge_environment_settings(self, url, proxies, stream, verify, cert):

:rtype: dict
"""

# Merge all the kwargs.
proxies = merge_setting(proxies, self.proxies)
stream = merge_setting(stream, self.stream)
verify = merge_setting(verify, self.verify)
cert = merge_setting(cert, self.cert)

proxies = proxies or {}

# Gather clues from the surrounding environment.
if self.trust_env:
# Set environment's proxies.
Expand All @@ -716,13 +728,7 @@ def merge_environment_settings(self, url, proxies, stream, verify, cert):
# with cURL.
if verify is True or verify is None:
verify = (os.environ.get('REQUESTS_CA_BUNDLE') or
os.environ.get('CURL_CA_BUNDLE'))

# Merge all the kwargs.
proxies = merge_setting(proxies, self.proxies)
stream = merge_setting(stream, self.stream)
verify = merge_setting(verify, self.verify)
cert = merge_setting(cert, self.cert)
os.environ.get('CURL_CA_BUNDLE') or verify)

return {'verify': verify, 'proxies': proxies, 'stream': stream,
'cert': cert}
Expand Down
27 changes: 27 additions & 0 deletions tests/test_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,33 @@ def test_respect_proxy_env_on_request(self, httpbin):
session = requests.Session()
session.request(method='GET', url=httpbin())

def test_merge_environment_settings_precedence(self):
session = requests.Session()
merge_settings_args = {
'url':'http://localhost:1',
'proxies':{
'http': 'http://argument'
},
'stream':None,
'verify':None,
'cert':None,
}
with override_environ(http_proxy='http://environment'):
session.proxies.update(http='http://attribute')
settings = session.merge_environment_settings(**merge_settings_args)
assert settings['proxies']['http'] == 'http://argument'

merge_settings_args['proxies'] = None
settings = session.merge_environment_settings(**merge_settings_args)
assert settings['proxies']['http'] == 'http://attribute'

session.proxies = None
settings = session.merge_environment_settings(**merge_settings_args)
assert settings['proxies']['http'] == 'http://environment'

settings = session.merge_environment_settings(**merge_settings_args)
assert settings['proxies'] == {}

def test_basicauth_with_netrc(self, httpbin):
auth = ('user', 'pass')
wrong_auth = ('wronguser', 'wrongpass')
Expand Down