Bug: Redbean's Fetch overwrites multiple Set-Cookie headers

[VicarEscaped]

Contact Details

No response

What happened?

When executing request with Fetch i expect too see multiple values of Set-Cookie header in header's table.
Actually, there is only one value of Set-Cookie header. I suppose it happens because table's keys are uniq.

Version

redbean 3.0.0

What operating system are you seeing the problem on?

No response

Relevant log output

❯ curl -sS -D - https://www.google.com/ -o /dev/null
HTTP/2 200 
date: Tue, 08 Oct 2024 21:04:01 GMT
expires: -1
cache-control: private, max-age=0
content-type: text/html; charset=ISO-8859-1
content-security-policy-report-only: object-src 'none';base-uri 'self';script-src 'nonce-F3SbG0JLGf3cgw_W8FSBXQ' 'strict-dynamic' 'report-sample' 'unsafe-eval' 'unsafe-inline' https: http:;report-uri https://csp.withgoogle.com/csp/gws/other-hp
accept-ch: Sec-CH-Prefers-Color-Scheme
p3p: CP="This is not a P3P policy! See g.co/p3phelp for more info."
server: gws
x-xss-protection: 0
x-frame-options: SAMEORIGIN
set-cookie: AEC=AVYB7cpSMVnO5NZoCnoWq2weiXceEf6ivK7T1FaTXck_bCwjeBft-7OVcXo; expires=Sun, 06-Apr-2025 21:04:01 GMT; path=/; domain=.google.com; Secure; HttpOnly; SameSite=lax
set-cookie: NID=518=Agb8Mf14XH0QfQ0tSSifSvx0PFb0hbQsveG5Zez9vljDh2f-34dt6FLMbxLiptl6kcte4XapTCn0SVVd4_3A6vzJu0epcXl-wEsW3AqNreIEdx1tp2GMEpsvyXfJ2hA5N5BM1cEki33DvXNTC5sRJZ6uiWys0C1utd06TjTuhIHc--b2NyMbHhHedQ4iRabI22hP; expires=Wed, 09-Apr-2025 21:04:01 GMT; path=/; domain=.google.com; HttpOnly
alt-svc: h3=":443"; ma=2592000,h3-29=":443"; ma=2592000
accept-ranges: none
vary: Accept-Encoding



>: s,h,b=Fetch("https://www.google.com")
>: h
{
  Connection="close",
  Date="Tue, 08 Oct 2024 21:17:13 GMT",
  Expires="-1",
  P3P="CP=\"This is not a P3P policy! See g.co/p3phelp for more info.\"",
  Server="gws",
  Vary="Accept-Encoding",
  ["Accept-CH"]="Sec-CH-Prefers-Color-Scheme",
  ["Accept-Ranges"]="none",
  ["Alt-Svc"]="h3=\":443\"; ma=2592000,h3-29=\":443\"; ma=2592000",
  ["Cache-Control"]="private, max-age=0",
  ["Content-Security-Policy-Report-Only"]="object-src 'none';base-uri 'self';script-src 'nonce-jOUfnku46xv46Jhf0t2hXw' 'strict-dynamic' 'report-sample' 'unsafe-eval' 'unsafe-inline' https: http:;report-uri https://csp.withgoogle.com/csp/gws/other-hp",
  ["Content-Type"]="text/html; charset=ISO-8859-1",
  ["Set-Cookie"]="NID=518=OO0NO8v2X2bzZhEQEF9UnC2p4SkyLYsWUUYHwXayiW-dXn-IWrjFlG9oDTxKaiCtKB7bqj28JfX6s5rjnipRWd_t_7x5gUVe0v8wMKI4Qo2HhPMnSly7Fn6FFoE59erbrDiHRSaaB_YXHed-nljfXGdtTZuxmOZERfr-3iDnNj8FrcH5irc3HOKoDkf05h65sQ-s; expires=Wed, 09-Apr-2025 21:17:13 GMT; path=/; domain=.google.com; HttpOnly",
  ["Transfer-Encoding"]="chunked",
  ["X-Frame-Options"]="SAMEORIGIN",
  ["X-XSS-Protection"]="0"
}

[pkulchenko]

This has been discussed on discord about a year ago, but there is no obvious solution that is good. Here is the extract from the Discord thread:

I see a couple of options: (1) add Set-Cookie to the list of "repeatable" headers and don't do anything else; the values will be folder in a comma-separated list; (2) implement (1), but also have a special process for it to fold the results into a table instead of a comma-separated list. Not ideal, because it breaks the existing code, but would follow the specs. The first option is the simplest and would work with the existing code as well as Fetch, but would not follow SHOULD NOT recommendation from RFC6265. it's probably recommended because Expires is the attribute that is clearly allowed to have commas, even though cookie-name/cookie-value can't include commas and Expires can probably be parsed to handle (may need to check path also).

I'm open to suggestions.

[VicarEscaped]

Thank you for pointing out that there is a discord server. somehow i missed that source of information.

Let’s fixes dont’s:

• Combine multiple set-cookie into one with “ ,” delimiter. Reason: RFC6265.
• Change set-cookie headers table value from str to table. Reason: breaks backwards compatibility.
• Extend Fetch return value with new object (e.g. status, headers, body, cookies = Fetch(“https://google.com”)). Reason: breaks backwards compatibility.

I’ve come up with two ideas.

Set-Cookie header renaming

Create Set-Cookie iterator. During creating headers table push found Set-Cookie with new name (e.g. Set-Cookie1, Set-Cookie2). After processing multiple Set-Cookie change first or last Set-Cookie new name back to original Set-Cookie. It depends on how now populates headers table. This prevents breaking backwards compatibility.
At last, provider code snippet in help.txt, that merge multiple Set-Cookie headers into single table.

function mergeCookieHeaders(headers)
    local c = {}
    for k,v in pairs(headers) do
        if k:find(“^Set%-Cookie”) ~= nil then
            table.insert(c, v)
        end
    end
    return c
end

Cons: messing around with not original server headers.

CookieJar

In various languages (e.g. java[1], python[2], go[3]) for cookies use concept of cookiejar. Fetch could extends config table with new optional key (e.g. jar, cookiejar). This key holds user defined table and populates it with found values of Set-Cookie headers.

>: local cookiejar = {}
>: s,h,b = Fetch(“https://google.com”, {method=“GET”, headers={[“Content-Type”]=“application/json”}, cookiejar=cookiejar})
>: print(cookiejar)
>: {“key=value; Domain=.some.com”, “key2=value2; Domain=.some.com”}

Pros: keep backwards compatibility.
Cons: Questionable logic, that need/must be implemented. Should cookiejar be bi-directional (e.g. used for getting and setting cookies)? Should use of cookiejar disable setting Cookie header in Fetch’s config?

[1] https://github.com/rest-assured/rest-assured/wiki/Usage#cookies
[2] https://requests.readthedocs.io/en/stable/_modules/requests/cookies/
[3] https://pkg.go.dev/net/http/cookiejar