`@std/http/file-server` shouldn't print network (non-loopback) address when host is loopback-only

[jsejcksn]

Describe the bug

Steps to Reproduce

Run the file server with permission --allow-sys=networkInterfaces and set the --host to a loopback value (e.g. localhost, 127.0.0.1, etc.).

deno run --allow-net=localhost --allow-read --allow-sys=networkInterfaces jsr:@std/http/file-server --host=localhost

Expected behavior

The onListen event handler should not print a network (non-loopback) address in the informational output — because it will not be reachable:

Listening on:
- Local: http://127.0.0.1:8000

Observed behavior

Non-loopback addresses are printed and represented as reachable:

Listening on:
- Local: http://127.0.0.1:8000
- Network: http://10.0.1.100:8000

Environment

• OS: macOS 14.5
• deno version: 1.45.4
• std version: 2024.07.26

[jsejcksn]

I like the recent progressive enhancement contributed by Luca in:

• fix(http): only show LAN address when --allow-sys is provided #5547

The onListen event handler currently looks like this:

function onListen({ port, hostname }: { port: number; hostname: string }) {
  let networkAddress: string | undefined = undefined;
  if (
    Deno.permissions.querySync({ name: "sys", kind: "networkInterfaces" })
      .state === "granted"
  ) {
    networkAddress = getNetworkAddress();
  }
  const protocol = useTls ? "https" : "http";
  let message = `Listening on:\n- Local: ${protocol}://${hostname}:${port}`;
  if (networkAddress && !DENO_DEPLOYMENT_ID) {
    message += `\n- Network: ${protocol}://${networkAddress}:${port}`;
  }
  console.log(message);
}

Perhaps we could add an additional condition to invalidate loopback addresses. Here's a regex-based example of what I'm proposing (however, the regular expression shown is just a placeholder for this example — it will need to be more thoroughly considered and tested):

  let message = `Listening on:\n- Local: ${protocol}://${hostname}:${port}`;
- if (networkAddress && !DENO_DEPLOYMENT_ID) {
+ const loopbackHostnameRegexp = /^(127\.[\d.]+|[0:]+1|localhost)$/i;
+ if (
+   networkAddress && !DENO_DEPLOYMENT_ID &&
+   !loopbackHostnameRegexp.test(hostname)
+ ) {
    message += `\n- Network: ${protocol}://${networkAddress}:${port}`;

If welcome, I'd be happy to create a draft PR from the code above so that iteration can continue.

[kt3k]

The suggestion sounds reasonable to me.

One thing to note here is that CLI currently replace "0.0.0.0" with "localhost" to workaround some issue in windows (So the check of localhost can be false positive). We might also need to address that issue.

ref: https://github.com/denoland/deno/blob/06b6352292b69359768c99a1fc984fa4bdcd07c9/ext/http/00_serve.ts#L657-L662

[jsejcksn]

One thing to note here is that CLI currently replace "0.0.0.0" with "localhost" to workaround some issue in windows (So the check of localhost can be false positive). We might also need to address that issue.

ref: https://github.com/denoland/deno/blob/06b6352292b69359768c99a1fc984fa4bdcd07c9/ext/http/00_serve.ts#L657-L662

@kt3k Thanks for pointing that out. It appears that the hostname substitution happens before onListen is invoked — so it's already too late to discriminate the destructured hostname parameter. When you said "We might also need to address that issue", did you have something in mind — like an alternate method for determining the original hostname value?

[kt3k]

When you said "We might also need to address that issue", did you have something in mind — like an alternate method for determining the original hostname value?

I think it might be reasonable to move the hostname substitution inside the default onListen handler (like around line 669).

[jsejcksn]

I think it might be reasonable to move the hostname substitution inside the default onListen handler (like around line 669).

@kt3k Understood. This seems to require coordinated changes in both respositories denoland/deno and denoland/std. Will you coordinate the changes, or is there some kind of next action I should take?

[kt3k]

Ok. I'll work on denoland/deno part. (I'll first try to land denoland/deno#24698 to avoid the conflicting work)