enable curl cookie engine

[krichprollsch]

Enabling Curl cookie engine brings advantages:

• handle cookies during a redirection: when a srv redirects including cookies, curl sends back the cookies correctly during the next request
• benefit curl's cookie parsing: we now use curl's lib to parse cookies instead of parsing them from headers manually still use our own parsing

[krichprollsch]

I'm not sure if there is a link between same_site and subdomain from Curl's parsing....

[krichprollsch]

How to test cookie handling on redirection?
https://httpbin.io/cookies/set?cookie_key=cookie_value returns a Set-Cookie header with a 301 response.

'use scrict'

import puppeteer from 'puppeteer-core';

const browser = await puppeteer.connect({
  browserWSEndpoint: 'ws://127.0.0.1:9222',
});

const context = await browser.createBrowserContext();
const page = await context.newPage();

await page.goto("https://httpbin.io/cookies/set?cookie_key=cookie_value", {waitUntil: 'load'});

// received cookies
console.log(await context.cookies());

await page.close();
await context.close();
await browser.disconnect();

[karlseguin]

It doesn't look like it supports the samesite attribute. I tested it out and looked at the source code, couldn't find anything.

The 2nd's field, the domain boolean, is used to indicate whether or not subdomains should be included. In the original, we capture that via a leading dot on the domain (as per the specs). You can ignore it, because CURL also puts the leading dot in the first field. In other words, I believe that this is currently true with the libcurl cookies:

var domain = curl_cookie[0];
var subdomain = curl_cookie[1];
if (subdomain == "true") {
   assert(domain[0] == '.');
} else {
  assert(domain[0] != '.');
}

Anyways, unless I'm missing something, I feel that the choice is using libcurl's cookie engine, or supporting the samesite attribute. Seems like a simple choice to use libcurl, except for this issue.

Pretty sure we can support redirect cookies with the original code, by placing the existing "Set-Cookie" block inside the existing redirect drain. Might need to grab the URL also:

if (transfer._redirecting) {
    {
      // add this
      const SET_COOKIE_LEN = "set-cookie:".len;
      if (header.len > SET_COOKIE_LEN) {
          // ...
      }
   }
    return buf_len;
}

[krichprollsch]

Pretty sure we can support redirect cookies with the original code, by placing the existing "Set-Cookie" block inside the existing redirect drain. Might need to grab the URL also:

I'm not that sure: we will add the cookies returned by the server in our JAR, that's correct, but since Curl handles the redirection itself, it will not inject them on the redirected request, except if we enable the cookie engine.

[krichprollsch]

But maybe we can use both:

• enable curl cookie engine to benefit of cookie parsing between redirection
• parsing ourselves Set-Cookie headers even on redirection with your trick
• cleaning and injecting cookies ourselves for all other requests

[krichprollsch]

PR updated this way:

• parse set-cookies for all responses, included redirections
• keep curl's cookies engine on to let it handles cookies on redirections
• split cookie from all headers to inject them using CURLOPT_COOKIE (it avoid double cookie: header on redirection
• cleanup internal CURLOPT_COOKIELIST before setting cookie to avoid double cookie set.

It seems to work correctly except one case: if you set manually a cookie (A=A) and the server sets one with the same name (A=B) and it redirects, then the redirect request will send the cookie twice with the 2 values (eg. Cookie: A=A; A=B).

[krichprollsch]

It seems to work correctly except one case: if you set manually a cookie (A=A) and the server sets one with the same name (A=B) and it redirects, then the redirect request will send the cookie twice with the 2 values (eg. Cookie: A=A; A=B).

ok, I already have the case with Google search 🤦