chimurai / http-proxy-middleware

:zap: The one-liner node.js http-proxy middleware for connect, express, next.js and more
MIT License
10.6k stars 828 forks source link

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

Open kevin-mitchell opened 4 months ago

kevin-mitchell commented 4 months ago

Checks

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; ...

Screenshot 2024-02-27 at 2 53 25 PM

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.