libcpr / cpr

C++ Requests: Curl for People, a spiritual port of Python Requests.
https://docs.libcpr.org/
Other
6.49k stars 922 forks source link

Making multiple requests through a session uses response cookies on subsequent requests #997

Closed amacdougall1 closed 6 months ago

amacdougall1 commented 9 months ago

Description

The bug is that when making a first request through a Session then making a second request through the same Session, cookies from the first request are saved and used in the second request.

The situation that brought this to me attention is while writing a web scraper, some endpoint returned an unauthorised on the first try, reauthentication is done and the request made again using the same session.

While this in retrospect makes sense as the session is specified stateful - as someone that has come over from PHP, Guzzle and other libraries use cookie jars to enable this behaviour and there is no indication the session handles expiring or storing cookies beyond explicitly setting them. Furthermore CURL doesn't enable its cookie engine by default so it was assumed this was the case for Sessions.

Example/How to Reproduce

cpr::Session sess;
sess.SetUrl("http://www.httpbin.org/cookies/set?cookies=yummy");

cpr::DebugCallback callback = {
    [](cpr::DebugCallback::InfoType type, std::string data, intptr_t userdata) {
        if(
            type != cpr::DebugCallback::InfoType::HEADER_OUT
            && type != cpr::DebugCallback::InfoType::HEADER_IN
        ) {
            return;
        }

        std::cout << data << "\n";
    }
};

sess.SetDebugCallback(callback);

sess.Get();
sess.Get();

See 2nd request sends the yummy cookie

Possible Fix

Add to docs that it enables a cookie engine by default.

Where did you get it from?

GitHub (branch e.g. master)

Additional Context/Your Environment

OS: Debian 11 Version: 1.10.5

COM8 commented 9 months ago

@amacdougall1 thanks for reporting and describing your use case here.

I see your point but for me the current situation is more intuitive and I by now had multiple cases where I needed to set an authentication token (cookie) once and then cpr::Session remembers it in all successive requests (the reason for a session besides keeping the connection open).

I switched this report to a discussion. If there is a majority of people who would welcome making cpr::Cookies inside cpr::Session not persistent (until ~March 2024), I'm willing to change it.

One other thing to keep in mind here: What should happen with all the other options e.g. cpr::Body or cpr::Header? Should they also not be persistent? Or should we strive for uniformity here where everything is persistent?

WSL-Ben commented 7 months ago

Hi. Personally, I'm in support of each request within a single session to not persist any data unless otherwise instructed to do so.

My reasoning for this position is that because C++ programs are typically (or at least more so than other languages) performance orientated, having the default behavior of sending a lot of potentially unnecessary data runs counter to this use-case. This is further compounded by the fact that you have to use a Session object in order to re-use an open connection.

Other than that, it's behaviour that is generally in-line with other clients across various languages and probably what the user of the library expects out of the box.

As a follow up question - is there any way to disable this behaviour currently?

COM8 commented 6 months ago

Sorry for the delayed response. I've been on vacation :)

@WSL-Ben thanks for your reply! I see your point and to answer your question, no there is currently no way to disable this behaviour.

As a middle ground in the future the addition of such an option could a good solution for it. This wouldn't change the current default of things (not leading to such a drastic API change).

But for now I'm closing this discussion as nice to have, but not planed as not enough people wanted it.