Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V3: base path not stripped from WebSocket upgrade requests #978

Open
2 tasks done
s100 opened this issue Apr 2, 2024 · 3 comments
Open
2 tasks done

V3: base path not stripped from WebSocket upgrade requests #978

s100 opened this issue Apr 2, 2024 · 3 comments

Comments

@s100
Copy link

s100 commented Apr 2, 2024

Checks

Describe the bug (be clear and concise)

When http-proxy-middleware@3 receives a HTTP request, it passes the HTTP request URL to your configured pathFilter and uses this to determine whether or not to proxy this request through. With most requests, the base URI on which the middleware is mounted is removed from the URL. But for a WebSocket upgrade the base URI is left on.

Step-by-step reproduction instructions

This isn't a fully functional reproduce but it should give an idea. I have HPM configured like so:

app.use('/a/b/c', createProxyMiddleware({
  target: 'http://localhost:4415',
  ws: true,
  pathFilter: pathname => {
    console.log(pathname)
    return pathname.startsWith('/api') ||
      pathname.startsWith('/raw-ws')
})

Then the application listens on http://localhost:5056.

Expected behavior (be clear and concise)

When I make a HTTP request to http://localhost:5056/a/b/c/api/whatever, the console outputs:

/api/whatever

, my filter returns true and HPM proxies the request through, correctly. Note how "/a/b/c" has been removed from pathname.

But when my application tries to open a web socket on http://localhost:5056/a/b/c/raw-ws, the console outputs:

/a/b/c/raw-ws

, the filter returns false, and my filter does not proxy the request through. Currently I am working around this by altering my filter to say:

pathname.startsWith('/api') ||
  pathname.startsWith('/a/b/c/raw-ws')

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

*****@9.0.688
`-- [email protected]

What http-proxy-middleware configuration are you using?

{
  target: 'http://localhost:4415',
  ws: true,
  pathFilter: pathname => {
    console.log(pathname)
    return pathname.startsWith('/api') ||
      pathname.startsWith('/raw-ws')
}

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

Windows, Node.js 18.17.1

Additional context (optional)

Big fan of the V3 upgrade because it's taken a lot of manual base path management off my hands! Just this last little bit :)

@chimurai
Copy link
Owner

Thanks for the feedback @s100!

In the HPM v3, the path argument in pathFilter is provided by the webserver you are using (req.url).
Same goes for the path stripping behaviour and the full path with WebSockets.

Across different servers paths are getting quite messy (to get the full path as seen in the logger) 😅

proxyServer.on('proxyRes', (proxyRes: any, req: any, res) => {
// BrowserSync uses req.originalUrl
// Next.js doesn't have req.baseUrl
const originalUrl = req.originalUrl ?? `${req.baseUrl || ''}${req.url}`;
const exchange = `[HPM] ${req.method} ${originalUrl} -> ${proxyRes.req.protocol}//${proxyRes.req.host}${proxyRes.req.path} [${proxyRes.statusCode}]`;
logger.info(exchange);

I'll have to investigate this and decide whether to keep the new behaviour with extra documentation or patch the path.

@s100
Copy link
Author

s100 commented Apr 11, 2024

Thanks for the update. By the way I noticed a small but significant typo in my original issue, which I have now corrected.

@Chiragasourabh
Copy link

Chiragasourabh commented Jun 13, 2024

I'm facing the same issue. In my opinion, both the proxied HTTP and WebSocket URLs should follow the same format. Otherwise, the upstream URLs will differ for both.

As a workaround, I'm using custom pathRewrite to remove the base path from the URL, if it exists, to make the URLs consistent for both HTTP and WebSocket requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants