vcrpy icon indicating copy to clipboard operation
vcrpy copied to clipboard

https request always triggers connect due to SSL verification for record_mode=new_episodes

Open gernot-h opened this issue 4 years ago • 5 comments

I'm using vcrpy 4.0.2 with requests 2.20.1 (urllib 1.24, Python 3.6.10, OpenSUSE 15.1).

My goal is to cache REST replies from a database.

All works perfect when I use an http connection or record_mode=once.

When I switch to https and record_mode=new_episodes, I noticed that requests take too long even if they are available in the cassette.

Here's my test code:

import vcr 
import requests 
with vcr.use_cassette("test.yml", record_mode="new_episodes"): 
     requests.get("https://www.heise.de", verify=False)

This codes triggers an SSL handshake with the server for me, even if the request is available in the cassette. When changing to default record_mode, it does not connect to the server. In both cases, the request is replayed from the cassette and no real payload is transferred over the network.

A bit of tracing showed that for https connections, VCRConnection::connect() is called by urllib3's HTTPSConnectionPool::_validate_conn() before the actual request is performed.

Shortened version of VCRConnection::connect():

    def connect(self, *args, **kwargs):
        if hasattr(self, "_vcr_request") and self.cassette.can_play_response_for(self._vcr_request):
            return

        if self.cassette.write_protected:
            return

        from vcr.patch import force_reset
        with force_reset():
            return self.real_connection.connect(*args, **kwargs)

In the https case with record_mode=new_episodes, the _vcr_request attribute is not set and the cassette is not write_protected, so it decides to pass to the base class for real_connection. With default record_mode, the cassette is write-protected, so it returns without doing a real connection.

(For the http case, urllib3's HTTPConnectionPool::_validate_conn() is called which is empty.)

As you can see above, verify=False doesn't help.

gernot-h avatar Mar 04 '20 07:03 gernot-h

Side node: this issue had a severe performance impact for me because I had many small, independent request.get() calls in my code.

After refactoring my code to use a requests.Session(), the impact is fairly minimal now - the unnecessary SSL handshake will only occur once for session setup.

However, triggering an SSL connection for a request which is already available in a cassette is still an unexpected behaviour in my opinion.

gernot-h avatar Mar 04 '20 11:03 gernot-h

@gernot-h Thank you dearly for posting this, I was investigating this same behavior. 🙏

ryanalane avatar Nov 29 '20 21:11 ryanalane

I had the same issue, I'm getting rid of the additional request with this fixture:

@pytest.fixture
def live_client(mocker):
    client = Client()
    mocker.patch.object(
        connectionpool.HTTPSConnectionPool, "_validate_conn", return_value=None
    )
    return client

ljnsn avatar Apr 01 '22 08:04 ljnsn

Is there a fix for this in the works? I had to use the following fixture to get tests working for new requests:

@pytest.fixture
def mocked_urllib3_validate(monkeypatch):
    def validate(conn):
        return None
    
    from urllib3 import connectionpool
    monkeypatch.setattr(connectionpool.HTTPSConnectionPool, "_validate_conn", validate)

CaptainDriftwood avatar Dec 21 '22 22:12 CaptainDriftwood

I get the same error as #533 and found unexpected additional requests being recorded just after I updated the cassettes. In my case, the number of requests should not change once a cassette is saved, so allow_playback_repeats might not be a reasonable solution. After some investigation I found this issue is the cause.

I'm using pytest-vcr and can get rid of this issue by this fixture to add custom pathces to vcr config:

from unittest.mock import MagicMock
from urllib3.connectionpool import HTTPSConnectionPool

@pytest.fixture(scope="module")
def vcr_config(vcr_update: bool) -> dict[str, Any]:
    return = {
        "custom_patches": [
            # https://github.com/kevin1024/vcrpy/issues/515
            (HTTPSConnectionPool, "_validate_conn", MagicMock()),
        ],
    }

Just as @gernot-h mentioned, triggering an SSL connection for a request which is already available in a cassette might be an unexpected behaviour.

Maybe vcrpy should fix this by adding this patch to the default urllib3 patchers?

https://github.com/kevin1024/vcrpy/blob/42d79b1102a1907376212e0a505851f4f8e81f5d/vcr/patch.py#L339-L353

vickyliin avatar Mar 08 '23 12:03 vickyliin