Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Add option to trim off baseUrl? #225

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

Closed
glenjamin opened this issue Dec 10, 2017 · 4 comments
Closed

Add option to trim off baseUrl? #225

glenjamin opened this issue Dec 10, 2017 · 4 comments

Comments

@glenjamin
Copy link

Setup

  • http-proxy-middleware: 0.17.4
  • server: express 4.16.2

proxy middleware configuration

proxy({
target:'http://www.example.org',
changeOrigin:true,
onProxyReq: addAuthHeadersOrSomething
});

server mounting

var app = express();
app.use("/api", apiProxy);
app.listen(3000);

Suggested behavior

Whenever I use this module, I tend to end up with the config roughly like the above. The webserver is proxying API requests back to some other origin, normally adding some authentication headers on the way past.

In this setup, I generally want the express mountpoint to be stripped away, which can be done via pathRewrite, but only by giving the middleware knowledge of where it is mounted.

Ideally I want the proxy to not need to know where it is mounted, and just strip the mountpoint away. I have found a way to do this inside onProxyReq:

// Trim middleware's mount path off the front of the proxy path
proxyReq.path = proxyReq.path.substring(req.baseUrl.length);

Would you be open to making this a built-in option?

@chimurai
Copy link
Owner

Hi @glenjamin .

Glad you're pointing this out.
This is actually on my todo list to make HPM more agnostic.

Some use cases:

app.use("/api", proxy({target:'http://www.example.org'}));
// https://localhost:3000/api/foo/bar  -> // http://www.example.org/foo/bar

app.use("/api", proxy({target:'http://www.example.org/api'}));
// https://localhost:3000/api/foo/bar  -> // http://www.example.org/api/foo/bar

app.use(proxy({target:'http://www.example.org'}));
// https://localhost:3000/foo/bar  -> // http://www.example.org/foo/bar

app.use(proxy({target:'http://www.example.org/api'}));
// https://localhost:3000/foo/bar  -> // http://www.example.org/api/foo/bar

Unfortunately this'll introduce a breaking change.

I've been thinking what the best strategy is...

  1. Introduce breaking change in v0.18.0. Fine according to https://semver.org . But probably this breaking change will surprise developers.

or

  1. Bump current functionality to v1.0.0 and mark it as stable with some bugfixes.
  2. v2.0.0 will get the agnostic usage.
  3. Remove some bloat/syntax sugar which conflicts with the new changes.

Some investigation is needed how this might affect the pathRewrite and router option.

Popular libs shouldn't be affected to much when they use the path-less express mounting :

Some libs can benefit from this change probably:

Please let me know what you think.

@glenjamin
Copy link
Author

My initial instinct is that that's being a little bit too clever.

A single new option called something like discardMountPath could be added which wouldn't break back-compat and if enabled would trigger the behaviour in your first two examples.

My experience with breaking changes has been that it can cause large knock on effects onto the ecosystem which are hard to predict when making them.

Introducing a new off-by-default option might not be as good for discoverability, but adding a prominent example into the README may help there.

@chimurai
Copy link
Owner

The examples are actually the default behavior of http-proxy. (Which I later found out)

Together with these two http-proxy options, the behavior can be tweaked.

prependPath: true/false, Default: true - specify whether you want to prepend the target's path to the proxy path
ignorePath: true/false, Default: false - specify whether you want to ignore the proxy path of the incoming request 

Thought it was a bug (#17) :) and "fixed" it with this line of code:

req.url = (req.originalUrl || req.url)

Like you said; Probably bit too clever. Or maybe insufficiently documented.

The original behavior is actually in line with nginx's proxy_pass behavior: http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_pass

Seen quite some issues when devs configure it the same way like in nginx...
Most common one: app.use("/api", proxy({target:'http://www.example.org/api'}));

With the current implementation, it will be proxied like:
http://localhost:3000/api/foo/bar -> http://www.example.org/api/api/foo/bar

Will have to think how this can be implemented smoothly...

@maxjf1
Copy link

maxjf1 commented Mar 22, 2019

I'm having an similar problem

import express, { Router } from 'express'
import proxyMiddleware from 'http-proxy-middleware'
import httpProxy from 'http-proxy' // used in solution B)

//Create proxy instance for solution B)
const proxy = httpProxy.createProxyServer({ secure: false })

// The Express APP
const app = express()

// An express router
const apiRouter = Router()

// my custom proxy middleware
function myProxyMiddleware(url){
    const proxyRouter= Router()

    // A)Not working solution
    router.use('/',proxyMiddleware({ target: url, changeOrigin: true }))

    // B)Working as expected solution
    router.all('*', (req, res) => {
        return proxy.web(req, res, { target: url, changeOrigin: true)
    })

return router
}

// api router using middleware
api.use('/endpoint', myProxyMiddleware('http://example.org/api'))

// app using api router
app.use('/myapi/v1', api)
app.server.listen(8080);

Where i'm trying to apply the solution A, for better integration
The problem is the URL mapping, it's not working as expected:

Using B)
localhost:8080/myapi/v1/endpoint => example.org/api
localhost:8080/myapi/v1/endpoint/foo/bar => example.org/api/foo/bar

Using A)
localhost:8080/myapi/v1/endpoint => example.org/api/myapi/v1/endpoint
localhost:8080/myapi/v1/endpoint/foo/bar => example.org/api/myapi/v1/endpoint/foo/bar

The middleware is using req.originalUrl - /myapi/v1/endpoint/foo/bar instead of req.path - /foo/bar

an solution that seems to work is

 router.use('/',(req, res, next) => {
        req.originalUrl = req.path
        next()
    },proxyMiddleware({ target: url, changeOrigin: true }))

Repository owner locked and limited conversation to collaborators Apr 18, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

3 participants