mutating requests in before_record_request mutates the actual request returned to the code

[graingert]

[kevin1024]

Does it also affect the request sent to the server? Should only affect the response that is recorded.

Do you mean if you grab the cassette? with vcr.use_cassett('foo.yml') as cass and then do cass.request?

[graingert]

Yes it effects the request to the server. You're missing a deepcopy

[colonelpanic8]

This is by design. I'm pretty sure that @kevin1024 is correct that before_record_request is only applied after the real request is made (https://gist.github.com/99942d7e60364ae3c11a the call to append is where before_record_request happens).

What IS true is that if you were to pass a body object to a some library and then mutate that object in before_record_request, it would come back as mutated to whomever is making the request. I can see an argument that the request/response should be deep copied before this callbacks are applied.

[graingert]

Yes that's what I mean

What IS true is that if you were to pass a body object to a some library and then mutate that object in before_record_request, it would come back as mutated to whomever is making the request. I can see an argument that the request/response should be deep copied before this callbacks are applied.

[colonelpanic8]

@graingert I'd welcome a fix that does a deepcopy if that is what you want. I'm probably going to do a release tomorrow after I've given people some time to look at what I did for #177. LMK if you want this to make it in to that...

[colonelpanic8]

I actually don't think that this is an issue... request copies headers, and the body is required to be a string by the point it gets there. Since it doesn't modify the request it makes to the server I don't see where there is a problem here.

[graingert]

This definitely caught me out. Perhaps an immutable dict or namedtuple would be better than deep copy?

[neozenith]

A lot of changes have happened to VCRpy since this ticket was opened. As this ticket has become stale, would you mind closing it if it is no longer needed / relevant?

If I haven't heard anything in a week I'll mark it as closed as we have a lot of old tickets that need to be groomed to make it easier to see what is still relevant.

However if it is still needed, please feel free to re-open or create a new ticket.

Thanks! 🙏

[butlertron]

This is still relevant, or at least, I'm running into the same issue. I have found that this can be fixed by changing https://github.com/kevin1024/vcrpy/blob/master/vcr/config.py#L219 from

request = copy.copy(request)

to

request = copy.deepcopy(request)

I believe that the reason deepcopy solves it is due to the complexity of the request object, including the HeadersDict class. A workaround is to make the deepcopy inside the before_record_request handler within my code. If this makes sense, I can look into a PR addressing this.

edit: the same approach is applied in https://github.com/kevin1024/vcrpy/blob/master/vcr/cassette.py#L232 with the response, it's just missing from the request.

[abramclark]

I just ran into this as well. I tried to prevent all headers from being recorded with this simple code:

def before_record(req):
    req.headers.clear()
    return req

VCR = vcr.VCR(before_record_request=before_record)

But it clobbered the headers of the outgoing request. This was pretty unexpected, and I had to fix it with this awkward code that uses another import:

from vcr.request import HeadersDict


def before_record(req):
    req.headers = HeadersDict()
    return req

[hartwork]

There is a related pull request #702 by @abramclark now. Would be great to have your review, I'm not entirely sure I already see the full picture. Thanks for your help! :pray:

CC @jairhenrique @kevin1024