Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

should_bypass_proxies not thread-safe #4997

Open
bmerry opened this issue Feb 22, 2019 · 0 comments
Open

should_bypass_proxies not thread-safe #4997

bmerry opened this issue Feb 22, 2019 · 0 comments

Comments

@bmerry
Copy link
Contributor

bmerry commented Feb 22, 2019

requests.utils.should_bypass_proxies temporary modifies the environment, calls into httplib and then restores the environment. However, it takes no locks while doing so, so two threads both using requests could lead to a permanent change to the environment. Even with a lock, any other thread that depends on the environment may observe the change (e.g. it may launch curl in a subprocess, and curl will then pick up this no_proxy setting).

Expected Result

The environment is not modified by requests.

Actual Result

I haven't observed this bug in practice - it's a theoretical race condition I can see in the code. In the reproduction code below, I believe the following sequence could occur:

  1. Thread 1 enters the set_environ context manager, sets old_value = None, os.environ['no_proxy'] = 'xyz'.
  2. Thread 2 enters set_environ, sets old_value = 'xyz', `os.environ['no_proxy'] = 'xyz'.
  3. Thread 1 exits, deletes os.environ['no_proxy'].
  4. Thread 1 exits, sets os.environ['no_proxy'] = 'xyz'.

Now after all the requests have been finished, the environment has been modified.

Reproduction Steps

I think this code can in theory trigger the race condition, although I haven't observed it. It needs an HTTP server on localhost:8080 (or just edit the URL)

#!/usr/bin/env python3

import threading
import os

import requests

def my_thread():
    session = requests.Session()
    for i in range(100):
        with session.get('http://localhost:8080/', proxies={'no_proxy': 'xyz'}) as resp:
            pass

threads = [threading.Thread(target=my_thread) for _ in range(16)]
for thread in threads:
    thread.start()
for thread in threads:
    thread.join()
print('no_proxy:', os.environ.get('no_proxy', 'not set'))

System Information

$ python -m requests.help
{
  "chardet": {
    "version": "3.0.4"
  },
  "cryptography": {
    "version": ""
  },
  "idna": {
    "version": "2.7"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.6.6"
  },
  "platform": {
    "release": "4.15.0-45-generic",
    "system": "Linux"
  },
  "pyOpenSSL": {
    "openssl_version": "",
    "version": null
  },
  "requests": {
    "version": "2.21.0"
  },
  "system_ssl": {
    "version": "1010007f"
  },
  "urllib3": {
    "version": "1.24.1"
  },
  "using_pyopenssl": false
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant