`proxies` input argument is mutated

[milanboers]

The input argument to proxies is mutated when environment proxy variables are present. See the reproduction steps.

This may be different than what users are expecting. It can lead to unexpected behavior when re-using the argument that was passed.

Expected Result

No mutation of the input argument:

>>> proxies = {'dummy': 'dummy'}
>>> os.environ['http_proxy'] = 'http://dummy'
>>> requests.get('https://python.org',proxies=proxies)
<Response [200]>
>>> proxies
{'dummy': 'dummy'}

Actual Result / Reproduction steps

>>> proxies = {'dummy': 'dummy'}
>>> os.environ['http_proxy'] = 'http://dummy'
>>> requests.get('https://python.org',proxies=proxies)
<Response [200]>
>>> proxies
{'dummy': 'dummy', 'http': 'http://dummy'}

System Information

$ python -m requests.help

{
  "chardet": {
    "version": null
  },
  "charset_normalizer": {
    "version": "2.0.12"
  },
  "cryptography": {
    "version": ""
  },
  "idna": {
    "version": "3.3"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.10.4"
  },
  "platform": {
    "release": "5.10.102.1-microsoft-standard-WSL2",
    "system": "Linux"
  },
  "pyOpenSSL": {
    "openssl_version": "",
    "version": null
  },
  "requests": {
    "version": "2.27.1"
  },
  "system_ssl": {
    "version": "30000020"
  },
  "urllib3": {
    "version": "1.26.9"
  },
  "using_charset_normalizer": true,
  "using_pyopenssl": false
}

[nateprewitt]

Hi @milanboers, the behavior you're seeing is documented and standard across other HTTP tools, so I'm not sure it's broadly unexpected. We can't remove this behavior since a significant portion of users rely on it. If you don't want this functionality, you can set trust_env to False to disable this as documented here.

[milanboers]

Hi @nateprewitt , not sure if I made myself clear enough. The issue is not that proxy settings are taken from the environment, but that the input argument itself is mutated. I don't see this documented and am not aware of other tools that do the same.

[nateprewitt]

Thanks for clarifying, I agree that behavior is a bit odd in the context of the top-level apis. merge_environment_settings is intended to mutate the input parameters and so I don't think we'd want to change that behavior. We could potentially create a copy of the input proxies here, but I think that also has potential to be breaking.

The best option here for now may be to make sure this is documented. I'd be willing to accept a PR with this information in the proxies section.

[milanboers]

I see. From the code it appears unintentional though:

• All other parameters to merge_environment_settings (url, stream, verify, cert) are not mutated, only proxies is.
• The merged settings are returned from the function. Why return anything if the input arguments are supposed to be mutated instead?
• The settings that are returned are not the same as the mutation:

>>> session.proxies['session'] = 'http://session'
>>> os.environ['http_proxy'] = 'http://env'
>>> proxies = {'input': 'http://input'}
>>> session.merge_environment_settings('http://myurl', proxies, None, None, None)
{'verify': True, 'proxies': OrderedDict([('session', 'http://session'), ('input', 'http://input'), ('http', 'http://env')]), 'stream': False, 'cert': None}
>>> proxies
{'input': 'http://input', 'http': 'http://env'}

[nateprewitt]

• All other parameters to merge_environment_settings (url, stream, verify, cert) are not mutated, only proxies is.

Well, to that point, proxies is the only mutable argument for merge_environment_settings. Everything else is immutable due to booleans and strings being references.

The merged settings are returned from the function. Why return anything if the input arguments are supposed to be mutated instead?

Because not everything is mutable.

The settings that are returned are not the same as the mutation:

Yeah, I agree this is a bit odd, but it's been the behavior for 9 years. I'm not entirely against improving it, but as I said above, this is existing functionality in a tool the majority of the Python ecosystem relies on. We cannot change this without a major version bump which isn't on the immediate horizon.

I think the actions for the moment are documentation updates. I can reopen this as a breaking tracking feature, but there isn't a current timeline for addressing it.