Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

Specifying an HTTPS proxy for the API client #36

Closed
buschtoens opened this issue Dec 28, 2017 · 13 comments
Closed

Specifying an HTTPS proxy for the API client #36

buschtoens opened this issue Dec 28, 2017 · 13 comments

Comments

@buschtoens
Copy link

buschtoens commented Dec 28, 2017

I originally opened #33 to enable users to specify an HTTPS proxy that should be used by the API client, but the PR was closed down in #33 (comment):

I'm probably going to implement this in a different way, since got is an implementation detail (and worse: likely to be removed). Can you open an issue instead?

I personally don't care about any of the other options offered by got while using the Cloudflare API and don't think any other user would.

@terinjokes I'd be happy to open another PR that implements proxy support, like you would want it, since I'm interested in moving this forward quickly. ⚡ 😄

I think allowing users to pass an HTTPS agent offers the greatest flexibility to users, since some might need to tunnel through proprietary proxy protocols. The agent option is understood universally by all underlying HTTP libs, like got or request.

Usage example

Using caw
import Cloudflare from 'cloudflare':
import caw from 'caw';

const cf = new Cloudflare({
  email: 'you@example.com',
  key: '<your API key here>',
  agent: caw({ protocol: 'https' }) // Cloudflare API is HTTPS only
});
Using tunnel-agent directly
import Cloudflare from 'cloudflare':
import TunnelAgent from 'tunnel-agent';

const cf = new Cloudflare({
  email: 'you@example.com',
  key: '<your API key here>',
  agent: TunnelAgent.httpsOverHttp({ port: 80, host: 'proxy.example.com' })
});
@buschtoens
Copy link
Author

buschtoens commented Dec 28, 2017

We could even go a step further and read the HTTPS_PROXY and NO_PROXY environment variables ourselves and set up the proxy accordingly. This is what request does and I think it makes for a great user experience, since it just works. ™

In that case, we should also add a proxy option that behaves just like requests's one and would only take effect, if the agent option is not defined.

Usage example

import Cloudflare from 'cloudflare':

const cf = new Cloudflare({
  email: 'you@example.com',
  key: '<your API key here>',
  proxy: 'http://proxy.example.com'
});

@buschtoens
Copy link
Author

@terinjokes Happy new year. 🎆
Were you able to make up your mind yet?

@terinjokes
Copy link
Contributor

Thanks for the great write up. My apologies in the delay in getting back to you.

Like you, I like software that just works, and I plan on supporting the HTTP_PROXY environment variable. I actually thought I had already supported it, but clearly I was mistaken.

I suspect most users won't be using them, but for reasons other than proxying, I do plan on accepting an option for agents as well. I'm working this weekend on refactoring the how I deal with global and call-specific options, so I'll likely get the environment variable support in first.

@buschtoens
Copy link
Author

Awesome, thank you!

btw, I just discovered env-proxy-agent through nodejs/node#15620 (comment). If you don't want to implement all the env logic yourself, this looks like a great utility.

@terinjokes
Copy link
Contributor

terinjokes commented Jan 9, 2018 via email

@buschtoens
Copy link
Author

Hmmm, good question. The Node 8 HTTP/2 module has no notion of agents. This would require some more trickery.

It also depends on whether the proxy itself runs on HTTP/2 or /1.

A minimal and probably not really useful example for an HTTP/2 proxy taken from the Node docs.

const http2 = require('http2');

const client = http2.connect('http://localhost:8001');

// Must not specify the ':path' and ':scheme' headers
// for CONNECT requests or an error will be thrown.
const req = client.request({
  ':method': 'CONNECT',
  ':authority': `localhost:${port}`
});

req.on('response', (headers) => {
  console.log(headers[http2.constants.HTTP2_HEADER_STATUS]);
});
let data = '';
req.setEncoding('utf8');
req.on('data', (chunk) => data += chunk);
req.on('end', () => {
  console.log(`The server says: ${data}`);
  client.destroy();
});
req.end('Jane');

Since most users probably won't use an HTTP/2 proxy, but a regular HTTP/1 one, we need to work with a custom createConnection callback. The following code isn't tested, but something along these lines should work:

import http2 from 'http2';
import net from 'net';

const proxy = 'localhost:3128';

const client = http2.connect('https://api.cloudflare.com', {
  createConnection(url, options) {
    const socket = net.connect(proxy);
    socket.write(`CONNECT ${url.hostname}:${url.port} HTTP/1.1\r\n`); // maybe HTTP/2 ?
    // do some buffering here
    return socket;
  }
});

@terinjokes
Copy link
Contributor

terinjokes commented Jan 9, 2018

@buschtoens I mean what does Chrome or Firefox do? Do they avoid connecting over HTTP2 when the system has an HTTP/S proxy?

terinjokes added a commit that referenced this issue Jan 10, 2018
By default, use the system-defined HTTPS proxy settings when connecting
to the Cloudflare API. This module will use the proxy defined by
HTTPS_PROXY unless the Cloudflare API host is matched by the pattern
specified in NO_PROXY.

Bug: #36
@terinjokes
Copy link
Contributor

@buschtoens Can you try the patches/http-proxy branch? This should automatically configure an agent based on the HTTPS_PROXY and NO_PROXY environment variables.

@terinjokes
Copy link
Contributor

@buschtoens Any luck?

@buschtoens
Copy link
Author

buschtoens commented Jan 10, 2018

I was caught up in meetings at work today. Sorry. I'll check this our first thing in the morning, when I'm behind the corporate proxy again.

I left two comments on your commit, but other than that, LGTM.

terinjokes added a commit that referenced this issue Jan 10, 2018
By default, use the system-defined HTTPS proxy settings when connecting
to the Cloudflare API. This module will use the proxy defined by
HTTPS_PROXY unless the Cloudflare API host is matched by the pattern
specified in NO_PROXY.

Bug: #36
@buschtoens
Copy link
Author

Just gave your branch a go. Works perfectly. 🌈

@terinjokes
Copy link
Contributor

terinjokes commented Jan 11, 2018 via email

terinjokes added a commit that referenced this issue Jan 11, 2018
By default, use the system-defined HTTPS proxy settings when connecting
to the Cloudflare API. This module will use the proxy defined by
HTTPS_PROXY unless the Cloudflare API host is matched by the pattern
specified in NO_PROXY.

Bug: #36
@terinjokes
Copy link
Contributor

Released in v.2.4.0

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

No branches or pull requests

2 participants