Response interceptor copyHeaders logic only removes first character of cookie domain because of non-greedy regex

[kevin-mitchell]

Checks

• I understand project setup issues should be asked on StackOverflow or in GitHub Discussions.
• I updated to latest http-proxy-middleware.

Describe the bug (be clear and concise)

The regex here https://github.com/chimurai/http-proxy-middleware/blob/master/src/handlers/response-interceptor.ts#L120 only removes the first character of the cookie domain in the Set-Cookie header, e.g. ... domain=magento2.docker; ... is rewritten ... domain=agento2.docker; ...

Step-by-step reproduction instructions

1. Get a response `Set-Cookie` header that includes as "PHPSESSID=123456d50ac9173b202d9734e756fd3a; expires=Tue, 27 Feb 2024 06:39:57 GMT; Max-Age=3600; path=/; domain=magento2.docker; secure; HttpOnly; SameSite=Lax"
2. Note that the cookie ends up after being processed by the proxy as 'PHPSESSID=123456d50ac9173b202d9734e756fd3a; expires=Tue, 27 Feb 2024 06:39:57 GMT; Max-Age=3600; path=/; agento2.docker; secure; HttpOnly; SameSite=Lax'

Expected behavior (be clear and concise)

If the intended behavior here is to completely remove the domain from the cookie (which doesn't seem right to me based on default config, but that's a separate issue I guess?), then it should be entirely removed.

How is http-proxy-middleware used in your project?

I'm using it directly for localhost to rewrite magento2.docker -> localhost:3000

What http-proxy-middleware configuration are you using?

return createProxyMiddleware(pathToProxy, {
      target: proxyURL.origin,
      changeOrigin: true,
      cookieDomainRewrite: false,
      cookiePathRewrite: false,
      router: {
        [`localhost:3000`]: proxyURL.origin,

      },
      secure: false,
      /**
       * IMPORTANT: avoid res.end being called automatically
       * */
      selfHandleResponse: true, // res.end() will be called internally by responseInterceptor()
}

What OS/version and node/version are you seeing the problem?

Node v18.10.0

Additional context (optional)

It seems perhaps the regex should be "greedy", and the ? should be removed perhaps? Honestly I don't exactly know what the intention is for this logic.

[Korhm]

Hello.

I've the exact same behavior using http-proxy-middleware used as dev proxy for an Angular app.

node --version
v20.11.1

npx ng --version
18.0.6

proxy.conf.mjs

import { responseInterceptor } from 'http-proxy-middleware';

export default {
  "/api/v1/**": {
    "target": "**redacted**",
    "secure": true,
    "changeOrigin": true,
    "logger": console,
    "ws": true,
    selfHandleResponse: true, // res.end() will be called internally by responseInterceptor()
    configure: (proxy, _options) => {
      proxy.on("proxyRes", responseInterceptor(async (responseBuffer, proxyRes, req, serverResponse) => {
          // body manipulation here

          return responseBuffer
      }));
    }
  }
}

Cookie sent by the server :
session=xxxxxxxxxxxxxxxxxxx; Domain=mydomain.com; Path=/api/v1; Expires=Thu, 29 Aug 2024 06:47:29 GMT; HttpOnly; Secure; SameSite=None

Cookie returned to Angular app
session=xxxxxxxxxxxxxxxxxxx; ydomain.com; Path=/api/v1; Expires=Thu, 29 Aug 2024 06:47:29 GMT; HttpOnly; Secure; SameSite=Non

[sleepingevil]

Hi, I believe we managed to track down the source of this bug with my team. We forked the project and managed to fix the the domain overwrite.

Fork available here

We made 2 changes:

1. We removed the hard-coded piece of code that was blindly using the regex mentioned above to always remove the domain from all set-cooke headers.
2. We changed the following line of code in http-proxy-middleware.ts to respect the options passed into the createProxyMiddleware factory, and pass it down to the underlying http-proxy package. This way, those that want to remove the domain from set-cookie headers can do so using the options passed into createProxyMiddleware.

This means that the code will start to respect the documentation on this page where it says that the default behaviour is not to remove the domains. See: option.cookieDomainRewrite.

Our requirement was actually to keep the same domain that came back on proxyRes, but this can be used to alter the domain, as well.

I'd love to raise a PR if that's ok with @chimurai. Suggestions are welcomed, and encouraged. 😉

[sleepingevil]

Hi @chimurai

Looks like you already had a PR with a similar fix here: #806

Would it be possible to merge this, please, so we can stop using our forked version? I'm sure others would appreciate it, too.

On a separate, but related note:

It doesn't address the fact though, that you still don't get to control the cookie domain overwrites, because the config that you're accepting on createProxyMiddleware, namely the changeOrigin config is not passed to the underlying http-proxy package. My fork, addresses this issue, too.

I think all you need to do is pass the config down to http-proxy here: https://github.com/chimurai/http-proxy-middleware/blob/master/src/http-proxy-middleware.ts#L26

The line would change from this.proxy = httpProxy.createProxyServer({}); to this.proxy = httpProxy.createProxyServer(options);

Would you accept my PR to fix this, too? I'd be more than happy to raise it.