Skip to content

Exception thrown in logger when trying to build URL for message #1035

@mattbaileyuk

Description

@mattbaileyuk

Checks

Describe the bug (be clear and concise)

The recently-published 3.0.1 of http-proxy-middleware includes a change to the logger (#1001) which attempts to build a valid URL object using parts coming into proxyRes.

However, we're seeing this blow up in some of our unit tests:

Node server exiting due to exception: TypeError [ERR_INVALID_URL]: Invalid URL
    at new NodeError (node:internal/errors:405:5)
    at new URL (node:internal/url:676:13)
    at ProxyServer.<anonymous> (/Users/matt/myapplication/node_modules/http-proxy-middleware/dist/plugins/default/logger-plugin.js:35:24)
    at ProxyServer.emit (/Users/matt/myapplication/node_modules/eventemitter3/index.js:204:33)
    at OverriddenClientRequest.<anonymous> (/Users/matt/myapplication/node_modules/http-proxy/lib/http-proxy/passes/web-incoming.js:173:27)
    at OverriddenClientRequest.emit (node:events:517:28)
    at respond (/Users/matt/myapplication/node_modules/nock/lib/playback_interceptor.js:307:11)
    at Timeout.cb [as _onTimeout] (/Users/matt/myapplication/node_modules/nock/lib/common.js:605:9)
    at listOnTimeout (node:internal/timers:569:17)
    at process.processTimers (node:internal/timers:512:7) {
  input: 'undefined//undefined/management/users',
  code: 'ERR_INVALID_URL'
}

The change does not cope with parts of req being missing in proxyRes, such as req.protocol and req.host as observed here. In 3.0.0 this would just get included in the log message as undefined//undefined/management/users and now it breaks http-proxy-middleware, so we have a breaking change here, even if it's just for more usual behaviour.

I think this is because nock isn't returning all the bits of req that this code expects (no protocol or hostname but does send pathname) - or not in the right format, as I'm unsure if proxyRes.req is being built or just copied in from the incoming message. I think the code could be updated to follow what was done with the port and make it conditional, maybe defaulting to http://localhost to make the URL valid.

Step-by-step reproduction instructions

1. Send a message to an Express route which then uses `http-proxy-middleware` to send a message on to another endpoint
2. Have an endpoint which sends a response where the `req` object doesn't include all the properties such as `protocol`. I believe mocking framework like `nock` would do this.

Expected behavior (be clear and concise)

The logger can cope with bits of req not being present when trying to build a valid URL and does not just blow up (especially if you don't even have logging enabled, as in our case)

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

Our CommonJS module requires `http-proxy-middleware`, pulling in `^3.0.0` in `package.json`

What http-proxy-middleware configuration are you using?

const proxyOptions = {
      target: 'https://' + hostname + path,
      changeOrigin: true,
      on: {
        proxyReq: fixRequestBody
      },
      ws: true,
      secure: true
    }


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

```shell
Node v18.20.4

Additional context (optional)

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions