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] Support Non-ASCII characters in HTTP Request Header Values #1739

Closed johnboxall closed 1 month ago

johnboxall commented 7 months ago

Stacked CDN setup may add a HTTP header for a geo-located region that can include special characters:

# Akamai
X-Akamai-Edgescape: Yucatán

# CloudFlare
CF-Region: Île-de-France

Requests with these non-ASCII characters in the header values return an empty HTTP 500 response from MRT.

Steps To Reproduce

curl -D- -so/dev/null 'https://pwa-kit.mobify-storefront.com/'  -H 'cf-region: Île-de-France' -H 'x-mobify-cachebreaker: 1'
HTTP/2 500 

Workaround

If you need headers like this, you can currently work around the issue with a little bit of Request Processor magic:

// request-processor.js
export const processRequest = function regionForwarder({
    headers,
    path,
    querystring,
    setRequestClass
}) {
    let region = headers.getHeader('cf-region')
    if (region) {
        region = Buffer.from(region, 'ascii').toString('utf8')
        headers.deleteHeader('cf-region')
        setRequestClass(new URLSearchParams({region}) + '')
    }
    return {path, querystring}
}

And then grab the request processor value in Express.js:

// ssr.js
app.get('/', function renderRegion(_, res) {
  const requestClass = Object.fromEntries(new URLSearchParams(res.locals.requestClass))
  return res.json({requestClass})
})

Note that while locally, you can modify the header directly in the request processor rather than using the request class, this doesn't work on MRT. It is also useful to set the request class in this workaround to make sure that MRT's cache key respects the region.

mgalassi commented 7 months ago

Hi @johnboxall, I think it happens to any other possibile header as well and I believe the chance it may happen for the Cookie one is often quite high (as cookies aren't necessarily under one's control).

Here an example of how to reproduce it:

curl -D- -so/dev/null 'https://pwa-kit.mobify-storefront.com/' -H 'cookie: _sn_m={"regionCode":"Île-de-France"};' -H 'x-mobify-cachebreaker: 1'

(note the 'Î' character)

Given how cookies are handled by MRT and Express, is the suggested workaround still valid? If it is, maybe we should convert the chars in a sort of middleware function in order to use it in all the other endpoint handlers as well?

Thank you for your insight John!

johnboxall commented 7 months ago

Hey @mgalassi, yes, you're correct! This bug can also manifest itself on headers controlled by the client like Cookie.

If you don't use cookies to conditionally alter rendering, then you can drop them entirely in the Request Processor to avoid the problem:

// request-processor.js
export const processRequest = function cookieDeleter({
    headers,
    path,
    querystring,
}) {
    headers.deleteHeader('cookie')
    return {path, querystring}
}

If you're not using them in ssr.js, then no need to pass them through the request processor.

As an alternative, you can also disable the allow_cookies setting on your environment:

https://developer.salesforce.com/docs/commerce/pwa-kit-managed-runtime/references/mrt-admin?meta=projects_target_partial_update

git2gus[bot] commented 5 months ago

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

joeluong-sfcc commented 1 month ago

Fixed with https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2009