Reset environment proxy config for some cache and range tests.

[mk-pmb]

The trick that made those tests pass on my test computer.

[mk-pmb]

From the failing Travis log:

test/express.js ....................................... 0/1 830ms
  express
  not ok Error: listen EADDRINUSE

I'll just re-commit and hope the next test run has better luck with the random numbers. This might be more interesting:

test/express-error.js ................................. 3/3
connections property is deprecated. Use getConnections() method

[jfhbrook]

Environment proxy config? what?

[mk-pmb]

The environment variables http_proxy, https_proxy, ftp_proxy and similar. Sometimes they are used in all-uppercase, could be a windows variation. It's a traditional feature of some network programs and libs, including the request package: "Controlling proxy behaviour using environment variables".

[jfhbrook]

Interesting.

Is it possible to set these while running tests? Say, NO_PROXY='*' npm test? That might be a better solution, especially if someone did want to apply proxy settings for God knows what reason.

[jfhbrook]

The express test is probably easy to fix by doing a search in the test file for connection and naively changing it to getConnections(). That'd be my guess.

[mk-pmb]

I think a good solution would be to refactor a lot of these patterns into a test helper module (maybe setup-loopback-listen):

• Find a random port (retry on EADDRINUSE), or get an OS-assigned address (demo below)
• If given just options instead of an ecstatic instance, create one with these options. (Might be abstracted to support other (req, res) handlers or middleware.)
• Configure events and listen. When listening:
  • Lookup port if set by OS
  • Provide a pre-configured instance of request, and the base URL where the ecstatic instance is listening. (Also refs to the server and the ecstatic instance. The latter because a subsequent test shouldn't have to know whether it was supplied or created with options.)

That pre-configured instance of request could bypass env proxy settings:

Furthermore, the proxy configuration option can be explicitly set to false / null to opt out of proxying altogether for that request.

I'm not eager to refactor all those tests myself, so anyone interested please jump in if @jfhbrook approves of the idea.

Since I can't find a mention of OS-assigned ports in the http module docs (only in net), here's how it works on my Ubuntu. Notice that it binds to localhost, where current tests seem to bind to 0.0.0.0/:: (any adapter).

var srv = require('http').createServer();
srv.listen(0, 'localhost', function serverReady(err) {
  var addr = srv.address(),
    baseUrl = 'http://' + addr.address + ':' + addr.port + '/';
  console.log(baseUrl);   // http://127.0.0.1:43482/
  srv.close();
});

[mk-pmb]

Now back to your questions:

Is it possible to set these while running tests?

It is. The post above shows a more elegant way than using process.env. However, even that one only lasts "while running tests":

$ export DUMMY=hi; echo a $DUMMY; nodejs -e '
>   console.log(1, process.env.DUMMY);
>   delete process.env.DUMMY;
>   console.log(2, process.env.DUMMY);
>   '; echo b $DUMMY
a hi
1 'hi'
2 undefined
b hi

Say, NO_PROXY='*' npm test?

That should work as well. I used http_proxy= npm test to verify my suspicion. You could even modify package.json to:

    "test": "env http_proxy= https_proxy= tap test/*.js"

… but that would kill any chance to set a custom proxy.

That might be a better solution, especially if someone did want to apply proxy settings for God knows what reason.

Then tests should at least warn that proxy settings are in effect. If the tests default to using system proxy settings, they should be liberal in what they accept from that proxy. Giving some thousand lines of terminal output complaining about transport headers isn't what I had expected from "running the tests before I start tinkering, just to be sure." ;-)

If the proxy settings should be test-specific, the request approach seems fit. For someone who wants to test a special proxy, we should allow them to specify one explicitly for the ecstatic tests, to have a conscious decision about the interception and the potential for test failures. We could use an environment variable ecstatic_proxy that would be parsed by the setup-loopback-listen helper. I'll patch my PR to allow a custom ecstatic_proxy.

[dotnetCarpenter]

@jekrb did some tests for setting the port via an environment variable in process-env-port.js, using child_process.spawn. You could easily use the same approach to test proxy environment variables per test.

About the 0.0.0.0 pattern, I think it's a unix misconception about localhost. Unfortunately ecstatic support this by outputting that pattern every time you start ecstatic from the terminal. Basically, 127.0.0.1 = localhost and 0.0.0.0 = any address (which unix translate to localhost because listening to 0.0.0.0 doesn't make sense). On Windows listening to 0.0.0.0 will fail. I guess different flavors of linux have different take on this, but Ubuntu should follow the unix approach.

[mk-pmb]

In case there should ever be a security flaw in ecstatic or the file system it uses for its tests, there may be effective differences between listening on "localhost" or "any address". However, for tests of external listening, here's my example code adapted:

var srv = require('http').createServer();
srv.listen(0, '', function serverReady(err) {
  var addr = srv.address(), host = String(addr.address), baseUrl;
  if (host.match(/^[0\.:]+$/)) { host = 'localhost'; }
  // ^-- e.g. "0.0.0.0", "::", "::0"
  baseUrl = 'http://' + host + ':' + addr.port + '/';
  console.log(baseUrl);   // http://localhost:48592/
  srv.close();
}); 

[jfhbrook]

refactor a lot of these patterns into a test helper module

Yeah, that seems reasonable. I'll turn this into an issue.

"test": "env http_proxy= https_proxy= tap test/*.js"

This would seem like the way to go, except I don't think I can support both windows and nix with that.

[jfhbrook]

I think the best approach here is to document the issue in CONTRIBUTING.md. I'm not sure how to reproduce the behavior you were seeing, so I'll leave that up to you.

I'm going to close this PR for now, not because HTTP_PROXY isn't an issue, but because I don't think this is the right PR to merge. I'll make a separate issue for this, and link in this thread.

[jfhbrook]

https://github.com/jfhbrook/node-ecstatic/issues/180

[jfhbrook]

https://github.com/jfhbrook/node-ecstatic/issues/181