[BUG] Netlify Functions event.headers.cookie sometimes suddenly used comma instead of semicolon

The site name is https://ratapp.dev.

The background:

We parse cookies from event.headers.cookie by taking the string and splitting it at every ; (semicolon). This worked for a year without any problems.

The problem:

Starting from the 21st of April we started getting the wrong values from our cookies. Upon investigating and trying to print out the event.headers.cookie string, it showed that cookies in Netlify were being separated by , (commas). This was causing the extraction to fail. In netlify dev, this was working correctly, however, in production it was being shown with the commas. This was seemingly random, sometimes showing up correctly (5% of the time) and showing up wrong 95% of the time. Attached below is a screenshot from the logs, showing once the output with commas (7pm) and once with semicolons (11:40 pm).

Incorrect:

Correct:

We put some logic in place to temp-fix it, by checking if we can split by semicolon, if not split by comma, but this adds a few hundred milliseconds to our response time. This seems like a bug, as CLI is doing it correctly, and the standard says semicolon.

Thank you!

1 Like

We’re seeing this issue as well and we now have a very less-than-ideal patch in to a library that we’re using (NextAuth) to handle it.

Any idea on a long-term solution? I believe that semicolon is the accepted route here.

1 Like

Here’s a thread on NextAuth’s issues with others having the same issue, scoped to Netlify:

1 Like

The only thing I can chime in here with is our temp-fix, which is anything but elegant, but seems to have worked. (Internal logging shows it the comma issue popped up about 42 times today, but it handled it correctly.)

What we do is, we check if we can split it by ; getting anything sensible, if not we split by ,.
We used to do this with a regexp, but for simplicity when trying to do a quickfix, we changed it to split.

// this is where we pass event.header.cookie in
function getCookies(cookies) {

    let cookiesToDict = cookies
        .split(';')
        .reduce((res, c) => {
            const [key, val] = c.trim().split('=').map(decodeURIComponent)

            return Object.assign(res, {
                [key]: val
            })

        }, {});
    // sessionID is a cookie we know will be there. If it isn't, we know something is up.
    if (cookiesToDict['sessionID'] == undefined) {
        let cookiesToDict = cookies
            .split(',')
            .reduce((res, c) => {
                const [key, val] = c.trim().split('=').map(decodeURIComponent)

                return Object.assign(res, {
                    [key]: val
                })

            }, {});
        return cookiesToDict
    } else {
        return cookiesToDict

    }
}

We noticed we were getting comma-separated cookie values intermittently last night and then by this morning nearly every request was coming in with comma-separated cookies. The issue manifested as a CSRF validation failure in NextAuth preventing all logins. Here’s a redacted example of the cookie object NextAuth constructed from the cookie string:

{
  "__Host-next-auth.csrf-token": "a-csrf-token,__Secure-next-auth.callback-url=our-callback-url,another-cookie=41c8-8139-808e49def69d"
}

It’s the key of just one cookie and the value is the rest of the cookie as one big string.

hi there, thanks for reporting this! i am going to check with the team on this and report back when i hear some news. thanks for your patience!

4 Likes

For others hitting this thread, I also posted our “quick (temporary) fix” to the Github issue here: Using NextAuth on Subdomains / Redirect loop back to Login Page · Issue #1888 · nextauthjs/next-auth · GitHub

good news all, we have pinpointed the source of the problem - and we are coordinating work to roll out a fix. I don’t have an ETA yet, so please don’t stop using your workarounds. We’ll announce here as soon as there is more info to share! Again, thanks for your patience and big :raised_hands: for alerting us to this bug!

5 Likes

Thanks for working on this :heart_eyes: It started off affecting ~10% of our traffic but as of yesterday it is affecting nearly 100%, but the temporary fix posted above by @thomas.milewski is working for now.

2 Likes

hi everyone! we have an update! We have a fix in place for this - could you take a look and see if things work now without your workaround? :crossed_fingers:

please let us know asap if not!

thanks.

Hi Perry!

Of all the requests we’ve received today, especially since your reply, none triggered the workaround, so all of them came in correctly with semicolons!

Thank you!!

Did you manage to ever find out what the problem was?

Hi Julian! What a relief :sweat_smile: it was actually a very weird tricky bug - one little piece of code in a next related library didn’t play with another little piece of code in one of our services. I can’t say more than that, but we feel confident we know what happened and are taking steps to make sure it doesn’t happen again. sorry you bumped into this and glad we were able to help quickly before we hit the weekend!

Hi! I was partly responsible for the bug in a service we launched recently as part of improving our CDN.

The gist is this excerpt from RFC 7540 Section 8.1.2.5

To allow for better compression efficiency, the Cookie header field MAY be split into separate header fields, each with one or more cookie-pairs. If there are multiple Cookie header fields after decompression, these MUST be concatenated into a single octet string using the two-octet delimiter of 0x3B, 0x20 (the ASCII string "; ") before being passed into a non-HTTP/2 context, such as an HTTP/1.1 connection, or a generic HTTP server application.

We received multiple Cookie headers via HTTP/2, but did not ensure to combine them when downgrading the request to HTTP/1.1 internally, as this RFC requires. We fixed the service responsible to make sure we only ever have a single Cookie header in HTTP/1.1 requests.

This misbehaviour actually also triggered a bug in some of our NextJS interop code that we’re fixing too, but have mitigated by the fix above.

Thanks for notfying us of this issue and being patient with the fix!

3 Likes

thanks for sharing all that detailed context, marcus!

1 Like

Yes, thank you for the context! Everything is looking good on our end. Thanks again!

1 Like