koichik / node-tunnel

Node HTTP/HTTPS Agents for tunneling proxies
MIT License
537 stars 78 forks source link

Request path contains unescaped characters. #12

Closed waynejuckts closed 6 years ago

waynejuckts commented 8 years ago

I always get Request path contains unescaped characters. while sending a request? I just use the default example

simonbernard commented 8 years ago

I had a similar problem:

TypeError: Request path contains unescaped characters.
    at new ClientRequest (_http_client.js:51:11)
    at TunnelingAgent.exports.request (http.js:31:10)
    at TunnelingAgent.createSocket (C:\Dev\node\node_modules\global-tunnel\node_modules\tunnel\lib\tunnel.js:117:24)
    at TunnelingAgent.createSecureSocket [as createSocket] (C:\Dev\node\node_modules\global-tunnel\node_modules\tunnel\lib\tunnel
.js:189:40)
    at TunnelingAgent.addRequest (C:\Dev\node\node_modules\global-tunnel\node_modules\tunnel\lib\tunnel.js:80:7)
    at new ClientRequest (_http_client.js:133:16)
    at Object.exports.request (http.js:31:10)
    at Object.globalTunnel._defaultedAgentRequest (C:\Dev\node\node_modules\global-tunnel\index.js:213:37)
    at Object.exports.request (https.js:163:15)
    at Object.globalTunnel._defaultedAgentRequest (C:\Dev\node\node_modules\global-tunnel\index.js:213:37)

I tried to figure out the problem and think I found it in tunnel.js around line 106:

    var connectOptions = mergeOptions({}, self.proxyOptions, {
        method: 'CONNECT',
        path: options.host + ':' + options.port,
        agent: false
    });

I changed it to:

    var connectOptions = mergeOptions({}, self.proxyOptions, {
        method: 'CONNECT',
        path: options.host.host + ':' + options.host.port,
        agent: false
    });

This fixes the issue for me. Is it a feasible fix? Shall I submit a Pull request for this?

waynejuckts commented 8 years ago

oh nice! Would be great thanks :)

simonbernard commented 8 years ago

@koichik What do you think?

koichik commented 8 years ago

@waynejuckts Thanks for the report.

@simonbernard Can you explain the problem in detail? I can't understand where options.host.{host,port} come from.

simonbernard commented 8 years ago

Hi @koichik

Well, I try.

I am using global-tunnel. This replaces addRequest and createSocket methods from request library. Code looks like this:

/**
 * Override createConnection and addRequest methods on the supplied agent.
 * http.Agent and https.Agent will set up createConnection in the constructor.
 */
function mixinProxying(agent, proxyOpts) {
    agent.proxy = proxyOpts;

    var orig = _.pick(agent, 'createConnection', 'addRequest');

    // Make the tcp or tls connection go to the proxy, ignoring the
    // destination host:port arguments.
    agent.createConnection = function (port, host, options) {
        return orig.createConnection.call(this,
            this.proxy.port, this.proxy.host, options);
    };

    // tell the proxy where we really want to go by fully-qualifying the path
    // part. Force a localAddress if one was configured
    agent.addRequest = function (req, host, port, localAddress) {
        req.path = this.proxy.innerProtocol + '//' + host + ':' + port + req.path;

        if (this.proxy.localAddress) {
            localAddress = this.proxy.localAddress;
        }
        return orig.addRequest.call(this, req, host, port, localAddress);
    };
}

So, if I get it right, this calls addRequest from tunnel library with some objects req, host and port. host object looks like this:

{ _defaultAgent:
   Agent {
     domain: null,
     _events: { free: [Function] },
     _eventsCount: 1,
     _maxListeners: undefined,
     defaultPort: 443,
     protocol: 'https:',
     options: { path: null },
     requests: {},
     sockets: {},
     freeSockets: {},
     keepAliveMsecs: 1000,
     keepAlive: false,
     maxSockets: Infinity,
     maxFreeSockets: 256,
     maxCachedSessions: 100,
     _sessionCache: { map: {}, list: [] } },
  host: 'my.host.com',
  port: '443',
  method: 'POST',
  path: '/the/path/i/want/to/request',
  headers:
   { Authorization: 'Basic zE1NjVkMzFiZmIxZGU5YWEzZjQ5YTZmZjI2ZA==',
     'X-ApiVersion': '4',
     Accept: 'application/xml',
     'Content-Type': 'application/json',
     'Content-Length': '30' },
  agent:
   TunnelingAgent {
     options: { proxy: [Object], maxSockets: undefined },
     proxyOptions:
      { host: 'my.proxy.com',
        port: 3129,
        protocol: 'http:',
        innerProtocol: 'https:' },
     maxSockets: Infinity,
     requests: [],
     sockets: [],
     _events: { free: [Function: onFree] },
     _eventsCount: 1,
     request: [Function],
     createSocket: [Function: createSecureSocket] } }

Port is undefined.

So, in createSocket, connectOptions set's up the path as follows:

var connectOptions = mergeOptions({}, self.proxyOptions, {
    method: 'CONNECT',
    path: options.host + ':' + options.port,
    agent: false
});

But port is undefined here and the actual proxy port is part of the host object.

What do you think?

koichik commented 8 years ago

@simonbernard Thanks. global-agent@1.2.0 depends on tunnel@0.0.2, but it doesn't support node v0.12 or later. If you use node v0.12 or later, please try tunnel@0.0.4.

simonbernard commented 8 years ago

OK, got it. So in my case it seems a problem of global-tunnel. I will check their issues and ask to update the dependency.

vvo commented 7 years ago

I guess this is an issue of global-tunnel and should be closed? I stumble on this issue and first thought that node-tunnel was broken. WDYT?

koichik commented 6 years ago

@vvo : Thanks, closing.