Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

test-http-dns-fails breaks if DNS server send redirects answers on invalid hostnames #8056

Closed
misterdjules opened this issue Aug 1, 2014 · 5 comments
Milestone

Comments

@misterdjules
Copy link

In test-http-dns-fails, the test performs two HTTP requests and makes sure that:

  1. The response callback is not called.
  2. The error event is emitted.

Here's the relevant code:

function httpreq(count) {
  if (1 < count) return;

  var req = http.request({
    host: 'not-a-real-domain-name.nobody-would-register-this-as-a-tld',
    port: 80,
    path: '/',
    method: 'GET'
  }, function(res) {
    resDespiteError = true;
  });

  req.on('error', function(e) {
    console.log(e.message);
    assert.strictEqual(e.code, 'ENOTFOUND');
    hadError++;
    httpreq(count + 1);
  });

  req.end();
}

httpreq(0);


process.on('exit', function() {
  assert.equal(false, resDespiteError);
  assert.equal(2, hadError);
});

The problem with this test is that DNS services providers such as level3 answer queries for non-existing hostnames with the IP of their search portal. Currently, the machine that runs our Jenkin's SmartOS node is setup to use this DNS service, and thus the test fails. It doesn't fail all the time because this node is also configured to use another DNS service that does not reply with redirects. If level3's DNS server is commented out from /etc/resolv.conf, the test works as expected.

We could change the configuration of the Jenkin's SmartOS node, but it will still fail if we move the Jenkins node to another machine with the same DNS config issues.

It seems these days I want to solve every testing issue with LD_PRELOAD, but it could be a good solution to test DNS failures properly. We could override getaddrinfo using LD_PRELOAD to make sure the dns lookup fails when we invoke the test. This method doesn't work on Windows, but we might be able to do something equivalent with SetWindowsHookEx.

What do you think?

@trevnorris
Copy link

The test is located in test/internet, which I think are held under special circumstances.

@tjfontaine What do you think?

@tjfontaine
Copy link

I am of two minds, on the one hand I like being able to explicitly test dns failures in a more predictable way. So long term having something that LD_PRELOADs and mucks with this to get back consistent results. In the short term I think I will just standardize all the jenkins nodes to use googles dns

@misterdjules
Copy link
Author

@tjfontaine Alright, can you please close this issue when the changes to the jenkins nodes are done? I'll create a more long term issue about having a tool that gives us the ability to mock external system APIs. Thank you!

@misterdjules misterdjules added this to the 0.11.15 milestone Jan 15, 2015
@misterdjules
Copy link
Author

Reopening as it's still an issue. @tjfontaine What would it take to standardize all the jenkins nodes to use google DNS servers?

@tjfontaine
Copy link

I fixed this for the current slaves, we could also maybe make the change happen for all tests in test/common.js by setting require('dns').setServers() -- not sure how I feel entirely about it, but it's a possibility

joaocgreis added a commit that referenced this issue Jul 16, 2015
- `test-crypto-domains` was fixed by
2afa3d8

- All tests under linux appear to be fixed and have not failed recently
on Jenkins

- `test-http-dns-fail` was fixed by the DNS configuration change
mentioned in #8056

Fixes #25656
Fixes #25673

Reviewed-By: Alexis Campailla <alexis@janeasystems.com>
PR-URL: #25676
jBarz pushed a commit to ibmruntimes/node that referenced this issue Nov 4, 2016
- `test-crypto-domains` was fixed by
nodejs/node-v0.x-archive@2afa3d8

- All tests under linux appear to be fixed and have not failed recently
on Jenkins

- `test-http-dns-fail` was fixed by the DNS configuration change
mentioned in nodejs#8056

Fixes nodejs#25656
Fixes nodejs#25673

Reviewed-By: Alexis Campailla <alexis@janeasystems.com>
PR-URL: nodejs#25676
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants