SalesforceCommerceCloud / pwa-kit

React-based JavaScript frontend framework to create a progressive web app (PWA) storefront for Salesforce B2C Commerce.
https://developer.salesforce.com/docs/commerce/pwa-kit-managed-runtime/guide/pwa-kit-overview.html
BSD 3-Clause "New" or "Revised" License
284 stars 134 forks source link

[BUG] Commas in cookie values "break" request processor locally #1139

Open johnboxall opened 1 year ago

johnboxall commented 1 year ago

Summary

The presence of a comma in a cookie value "breaks" the processing of the request of the cookie.

Though this behaviour seems/is RFC compliant, in the wild, it isn't uncommon for folks to stuff JSON into a cookie.

This behaviour is also different from the request processor when deployed on MRT, where commas in cookie values aren't a problem.

Steps To Reproduce

Create a new project (npx pwa-kit-create-app) with the default settings.

Update the request-processor.js:

export const processRequest = ({headers, setRequestClass, path, querystring}) => {
    const cookie = headers.getHeader('cookie')
    console.log('Request Processor:', {cookie})
    if (cookie) {
        setRequestClass(cookie)
    }

    return {
        path,
        querystring
    }
}

Make a HTTP request:

curl 'http://localhost:3000/cookie' \
    -sH 'Cookie: k1=v1; k2=v2,v3; k3=v4' | jq

Observe the cookie header value is truncated: k1=v1; k2=v2 where we expect k1=v1; k2=v2,v3; k3=v4.

If you want you add the following handler to ssr.js to validate things work correctly when deployed to MRT:

app.get('/cookie', (_, res) => {
    res.json({
        cookie: res.locals.requestClass || ''
    })
})

After deploy this bundle to a environment, make an HTTP request and observe the entire cookie value is returned as expected.


It isn't immediately clear to me where to look to resolve this issue.

We have some special logic for handlers headers with the request processor:

https://github.com/SalesforceCommerceCloud/pwa-kit/blob/00ff2bdfca5c95f725e7e63adff847495f809fbf/packages/pwa-kit-runtime/src/ssr/server/build-remote-server.js#L368-L567 https://github.com/SalesforceCommerceCloud/pwa-kit/blob/develop/packages/pwa-kit-runtime/src/utils/ssr-request-processing.js

Its likely the case that the invalid cookie values are getting munched at a lower level.

bfeister commented 1 year ago

Thanks @johnboxall was this at the root / related to the support call yesterday?

git2gus[bot] commented 1 year ago

This issue has been linked to a new work item: W-13055556