[fix] Agent cookies are not correctly attached to right domains/paths on redirect

[robaca]

Describe the bug

Node.js version: 20.11.0

OS version: MacOS Sonoma 14.5

Description:

We are using superagent agent to perform an OpenID authorization code flow before interface tests, crossing domain boundaries on multiple redirects. After upgrading from superagent 8.0.8 to 9.0.2, this fails because of missing cookies in the latter redirects.

After adding some console.logs to the agent lib, it looks like in 8.0.8 the _saveCookies() method is not storing the right url.hostname/path but null/null, which means that all cookies are attached later irrespective of the domain, while in 9.0.2 it's passing wrong domains and paths into this.jar.setCookies(), so that they are not attached later.

Actual behavior

Example: agent starts in domain www.service.org which sets a session Cookie, then redirects to oidc.service.org for authentication, which itself sets cookies. Afterwards the same agent is used to post login credentials on oidc.service.org, which causes a redirect back to www.service.org. Now the session cookie is not passed to www.service.org, as it has been saved in the cookie jar with the redirection host/path for oidc.service.org.

Expected behavior

Cookies set in the response of any redirecting request that have no domain part by themselves should be saved with the domain of that exact request, not with that of the followup request.

Checklist

• [X] I have searched through GitHub issues for similar issues.
• [X] I have completely read through the README and documentation.
• [X] I have tested my code with the latest version of Node.js and this package and confirmed it is still not working.

[robaca]

btw we cannot workaround setting a domain as our service is multi-tenant and the express-session middleware does not support this. Created https://github.com/expressjs/session/issues/988 in addition.

[robaca]

I created a failing test here: https://github.com/ladjs/superagent/commit/22f6632d8df6ef08de23ceb19d4a21a5099a1d64

[robaca]

I tried a fix here: https://github.com/ladjs/superagent/compare/master...robaca:superagent:master

For some reason the multipart tests are failing for me locally (maybe because of Node version), otherwise looks promising. I didn't check the PR rules beforehands, so it's most probably not yet ready for a PR.

[robaca]

btw. I get the same error in supertest. If I use it with my patched version and change one line in supertest/lib/agent.js to req.on('pre-redirect', this._saveCookies.bind(this)), it's working for me.