got request for host in hosts file throws ENOTFOUND

[fluffynuts]

Describe the bug

• Node.js version: 12.16.2 (also tested with 14.0.0)
• OS & version: windows 10

Actual behavior

Attempting a got request against a host which is registered in my hosts files fails with an ENOTFOUND; where doing an https.get for the same host and path does not.

This used to work with got 10, but got 10 hangs on node 12 on an ubuntu 14 machine, which happens to be my target for this project :/

Expected behavior

Should be able to retrieve data from a server by hostname according to system hosts file

Code to reproduce

(not so much code as steps):

• set up an http server listening on 127.0.0.1
• create a hostfile rebind for some other hostname
• attempt got to that server

Checklist

• [x] I have read the documentation.
• [x] I have tried my code with the latest version of Node.js and Got.

[szmarczak]

(not so much code as steps):

Please include the code and the entire hosts file.

[fluffynuts]

as per description:

// this works:
        const opts = {
            host: "foo.dev.local",
            path: "/user"
        }

        const response = await new Promise((resolve, reject) => {
            let result = {
                body: "",
                statusCode: 0
            };
            try {
                h.get(opts, res => {
                    res.on("data", d => result.body += d.toString());
                    res.on("end", () => {
                        result.statusCode = 200;
                        resolve(result);
                    });
                    res.on("error", e => {
                        // can we get the status from e?
                        result.statusCode = 500;
                        reject(result);
                    });
                });
            } catch (err) {
                reject(err);
            }
        });
// this does not:
const response2 = await got("https://goo.dev.local/user");

hosts file:

127.0.0.1   foo.dev.local

[szmarczak]

I need the server code too.

[fluffynuts]

I originally left out code because the situation is easy enough to replicate, but specific to the host system; however, as per your request, here's a full set of steps:

1. update \windows\system32\drivers\etchosts to contain:

127.0.0.1      server.dev.local

2. create a new project:

mkdir got-issue
cd got-issue
npm init -y
npm install --save got express

3. add the app.js file:

var app = require("express")();
var got = require("got");
var http = require("http");

app.get('/user', (req, res) => {
  res.send({
    id: 1,
    name: "Han",
    likes: "to shoot first"
  });
});

app.get("/fails", (req, res) => {
  got("http://server.dev.local/user")
    .then(result => res.send(result.body))
    .catch(err => {
      res.status(500);
      res.send(err);
    });
});

app.get("/by-ip", (req, res) => {
  got("http://127.0.0.1/user")
    .then(result => res.send(result.body))
    .catch(err => {
      res.status(500);
      res.send(err);
    });
});

app.get("/by-http", (req, res) => {
  http.get("http://server.dev.local/user", r => {
    let data = "";
    r.on("data", d => data += d.toString());
    r.on("end", () => res.send(data));
  });
});

var server = app.listen(80, () => {
  const address = server.address();
  console.log(`listening on ${address.address}:${address.port}`);
});

4. update package.json, adding the 'start' script:

{
  "name": "got-issue",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "start": "node app.js"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "dependencies": {
    "express": "^4.17.1",
    "got": "^11.0.2"
  }
}

5. Run it

npm start

6. Create a batch file:

curl http://localhost/user
curl http://localhost/fails
curl http://localhost/by-ip
curl http://localhost/by-http

7. Run the batch file

C:\code\scratch\got-issue>curl http://localhost/user
{"id":1,"name":"Han","likes":"to shoot first"}

C:\code\scratch\got-issue>curl http://localhost/fails
{"name":"RequestError","code":"ENOTFOUND","timings":{"start":1587992852528,"socket":1587992852529,"lookup":1587992852534,"error":1587992852534,"abort":1587992852535,"phases":{"wait":1,"dns":5,"total":7}}}

C:\code\scratch\got-issue>curl http://localhost/by-ip
{"id":1,"name":"Han","likes":"to shoot first"}

C:\code\scratch\got-issue>curl http://localhost/by-http
{"id":1,"name":"Han","likes":"to shoot first"}

[szmarczak]

I cannot reproduce even with your example:

The server is running on Windows 10, with the hosts entry described.

$ curl http://localhost/fails
{"id":1,"name":"Han","likes":"to shoot first"}

[szmarczak]

Can you show your entire hosts file?

[fluffynuts]

no, I can't share all of that. A single hosts entry should be enough to demonstrate this, as per the details above. Suffice to say that, for this test, I had to add the single entry for server.dev.local to make the ones that pass, pass. That's all that should be required.

[szmarczak]

no, I can't share all of that.

Then can you temporarily replace its contents to only contain the localhost entry and the server.dev.local? Make sure to reboot.

[fluffynuts]

ok, so with a hosts file consisting of exactly:

127.0.0.1       server.dev.local

the test app fails, both before and after a reboot.

[fluffynuts]

I've found the error. It's in your upstream cacheable-lookup and I've raised a PR to fix it: https://github.com/szmarczak/cacheable-lookup/pull/23

Please +1 or something there so that perhaps this gets accepted (in one form or another) sooner rather than later.

[fluffynuts]

imo, this is a result of trying to be "too clever": node's dns module has a really simple lookup function which resolves via system methods (so hosts file entries are observed). cacheable-lookup is doing manual work to read from the system hosts file which means it is vulnerable to

• internal bugs (like the one I've PR'd against, where the author forgets that text files can have different line endings on different systems)
• external bugs / quirks, like readFile automatically using the host OS's line endings for text
• does not update if the hosts file is updated -- it's read once at spin-up, as far as I can see and the ttl is set to infinity for these hosts, so updates to ips won't be reflected there either.

I understand what cacheable-lookup is trying to do -- in-memory cache for dns requests -- but this could have been done more robustly and simply without trying to be smart about hosts files and the like.

[szmarczak]

but this could have been done more robustly and simply without trying to be smart about hosts files and the like.

You are actually wrong. There are other places to retrieve the entries. The hosts file is only a part of it. The kernel does not respond with a information whether the entry is coming from the hosts file, custom DNS rules (e.g. suffixes), internal OS cache or direct DNS query.

For reference see https://github.com/sindresorhus/got/issues/1175

Note that process bindings are not so fast. Not to mention that the underlying OS implementation doesn't support TTL.

does not update if the hosts file is updated

You're wrong here also.

https://github.com/szmarczak/cacheable-lookup/blob/35b035201c8ff16579d8d0447e120b754a66c459/source/hosts-resolver.js#L37-L46

I've found the error. It's in your upstream cacheable-lookup and I've raised a PR to fix it:

Thanks for taking the time to locate the bug! I really appreciate it :) Will look at this ASAP.

[fluffynuts]

You're wrong here also.

szmarczak/cacheable-lookup:source/hosts-resolver.js@35b0352#L37-L46

It seems that I'm wrong in theory only (I can see the watcher, but it appears to _not_ be working); I did this test to repro:

1. create an hosts file with only 127.0.0.1 server.dev.local, unix line-endings
2. run the test from the api and test.bat above, and I get a response ({"id":1,"name":"Han","likes":"to shoot first"})
3. modify that file to have 196.25.1.1 server.dev.local
4. run the test from the api and test.bat above and I still get a response ({"id":1,"name":"Han","likes":"to shoot first"}), when I shouldn't
5. restart the api, and re-run the test, and the call fails (as it should, because 196.25.1.1 is a high-level dns server for my country, South Africa) -- I get {"name":"RequestError","code":"ECONNREFUSED","timings":{"start":1588691301843,"socket":1588691301844,"error":1588691303997,"abort":1588691303997,"phases":{"wait":1,"total":2154}}}
6. In addition, copying back in a valid hosts file with the api running from 5 does not allow it to recover (I get ECONNREFUSED)

You are actually wrong. There are other places to retrieve the entries. The hosts file is only a part of it. The kernel does not respond with a information whether the entry is coming from the hosts file, custom DNS rules (e.g. suffixes), internal OS cache or direct DNS query.

I'm well aware that the hosts file is only part of dns resolution. So if I understand correctly, the reasons for customising behavior of dns.lookup are:

• to resolve hosts with implied suffixes
• to provide extra information to the caller about the resolved address
  ?

I can see the use-case for the first from the linked issue, and I _guess_ someone might be interested in the latter. The first sounds like you've had to work around a dns bug (imo, dns.lookup should "just work" with the same resolution as ping might, but perhaps there are "good reasons" for behaving differently?). I would guess that most clients wouldn't care about extended information as to _where_ the resolution came from, but I suppose if you have that info, then might as well pass it on 🤷

[fluffynuts]

@szmarczak would you like me to raise a separate issue about the hosts-file watcher not triggering a refresh?

[szmarczak]

You sure that it doesn't watch it? By default Node.js performs the check every 5s. I haven't changed this setting.

[szmarczak]

If it still doesn't update the entries, please make another issue :)

[fluffynuts]

ok, I was testing quite quickly, so let me try a little slower (:

[fluffynuts]

yeah, I'm afraid that, even waiting 30s, the updated hosts entries aren't picked up. I'll raise another issue. Perhaps it's related to node on windows or something like that... Anyways, I'll add all the details in a new issue.

[fluffynuts]

I've raised https://github.com/szmarczak/cacheable-lookup/issues/24 -- please let me know if you need more info (:

[szmarczak]

Fixed in cacheable-lookup.