Open Bug 1783938 Opened 2 years ago Updated 2 years ago

httpd.js refuses connections when started on 127.0.0.1

Categories

(Core :: Networking: HTTP, defect, P2)

Product:
Core
Component:
Networking: HTTP
Type:
defect

Tracking

()

People

(Reporter: whimboo, Unassigned)

References

Details

(Whiteboard: [necko-triaged])

Normally the httpd.js server is started on localhost. But depending on the DNS resolution it can mean that this is 127.0.0.1 (IPv4) or [::1] (IPv6), and this causes issues for external clients to connect to our Remote Agent port.

As such I would like to start the server specifically with 127.0.0.1 as host. But by using the following invocation any request going to this address is getting denied with a HTTP 400 response:

_start(9222, "127.0.0.1", false);

With the result of:

requests.exceptions.HTTPError: 400 Client Error: Bad Request for url: http://127.0.0.1:9222/json/version

As it looks like the problem is somewhat related to _initialize() and how the primary name is setup. When I change the given localhost to make use of the value from host it works fine, but then a couple of xpcshell tests fail.

Note that this is not a problem for the IPv6 case!

Kershaw, do you think that changing localhost to 127.0.0.1 in the IPv4 case makes sense? It would at least in par with the IPv6 case. Thanks.

Flags: needinfo?(kershaw)

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #0)

Normally the httpd.js server is started on localhost. But depending on the DNS resolution it can mean that this is 127.0.0.1 (IPv4) or [::1] (IPv6), and this causes issues for external clients to connect to our Remote Agent port.

As such I would like to start the server specifically with 127.0.0.1 as host. But by using the following invocation any request going to this address is getting denied with a HTTP 400 response:

_start(9222, "127.0.0.1", false);

With the result of:

requests.exceptions.HTTPError: 400 Client Error: Bad Request for url: http://127.0.0.1:9222/json/version

As it looks like the problem is somewhat related to _initialize() and how the primary name is setup. When I change the given localhost to make use of the value from host it works fine, but then a couple of xpcshell tests fail.

Note that this is not a problem for the IPv6 case!

Kershaw, do you think that changing localhost to 127.0.0.1 in the IPv4 case makes sense? It would at least in par with the IPv6 case. Thanks.

I think maybe it's easier to add another api in nsIHttpServer.idl and make this new api only be used by your code.
For example, we might add something like:

//
  // see nsIHttpServer.start_ipv4
  //
  start_ipv4(port) {
    this._start(port, "127.0.0.1");
  },

What do you think?

Flags: needinfo?(kershaw)

(In reply to Kershaw Chang [:kershaw] from comment #1)

I think maybe it's easier to add another api in nsIHttpServer.idl and make this new api only be used by your code.
For example, we might add something like:

//
  // see nsIHttpServer.start_ipv4
  //
  start_ipv4(port) {
    this._start(port, "127.0.0.1");
  },

What do you think?

Our code would still call this._start() directly so we do not need an extra entry point. But that also wouldn't solve the following problem:

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #0)

As it looks like the problem is somewhat related to _initialize() and how the primary name is setup. When I change the given localhost to make use of the value from host it works fine, but then a couple of xpcshell tests fail.

Having localhost hard-coded in the above line is problematic. And using the host property instead and passing in 127.0.01 causes internal server issues or the tests are wrong?

If I understand things correctly, you want an IPv4 only server that accepts requests to both 127.0.0.1 and localhost.
I think you might be able to the server normally, and then call server.identity.add("127.0.0.1", port). Does that work?

Flags: needinfo?(hskupin)

(In reply to Valentin Gosu [:valentin] (he/him) from comment #3)

If I understand things correctly, you want an IPv4 only server that accepts requests to both 127.0.0.1 and localhost.

Yes, in case of localhost resolves first to a IPv4 address we want to start the server with 127.0.0.1 but also only accept requests on that IP but not localhost or ::1.

I think you might be able to the server normally, and then call server.identity.add("127.0.0.1", port). Does that work?

When I start the server with 127.0.0.1 shouldn't it actually already become the primary identity? I wonder why I would have to add another one.

Using server.identity.add("http", "127.0.0.1", port) works in case the server is started with localhost or 127.0.01, but fails when the host is [::1]. Then I get a The URI is malformed error whether if dual stack is enabled or not.

Flags: needinfo?(hskupin)

I ran a test with:

httpserv.start_dualStack(9992);

// All of these seem to work
makeChan(`http://127.0.0.1:${httpserv.identity.primaryPort}/`);
makeChan(`http://[::1]:${httpserv.identity.primaryPort}/`);
makeChan(`http://localhost:${httpserv.identity.primaryPort}/`);

Maybe I'm not fully understanding the context here.
It's my impression that in D154151 you don't even need to try to resolve localhost - instead you can just create the local server.

So what happens if the machine doesn't support a dual stack? I assume creating the server as such would fail with an error? Sadly I cannot easily test this.

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #6)

So what happens if the machine doesn't support a dual stack? I assume creating the server as such would fail with an error? Sadly I cannot easily test this.

It's hard to know. There's this comment here about dual stack on windows. https://searchfox.org/mozilla-central/rev/6ec440e105c2b75d5cae9d34f957a2f85a106d54/netwerk/base/nsServerSocket.cpp#373
But since we end up using 127.0.0.1 do we really need a dual stack IP? I'm wondering what are the specific requirements for this thing?
Based on D154151 I would think just using IPv4 is enough?

Flags: needinfo?(hskupin)

(In reply to Valentin Gosu [:valentin] (he/him) from comment #7)

It's hard to know. There's this comment here about dual stack on windows. https://searchfox.org/mozilla-central/rev/6ec440e105c2b75d5cae9d34f957a2f85a106d54/netwerk/base/nsServerSocket.cpp#373

Reading this comment and seeing that IPV6Only gets disabled, I would assume it then would also work on Windows?

But since we end up using 127.0.0.1 do we really need a dual stack IP? I'm wondering what are the specific requirements for this thing?
Based on D154151 I would think just using IPv4 is enough?

There have been requests in the past from people running IPv6 only in their testing environment. Under such conditions the above proposal would be broken. If we resolve localhost on our side the first resolved IP address would be [::1] then. Using this IP will make it work immediately, but on normal systems with dual-stacks it will resolve to 127.0.0.1.

Do you have an idea why [::1] works as host but 127.0.0.1 not when not adding additional identities?

Flags: needinfo?(hskupin)

There have been requests in the past from people running IPv6 only in their testing environment.

Even if you're running an IPv6 only network, I think it's likely that IPv4 localhost will still work.
Since bug 1220810, we don't even use the system resolver to resolve localhost, so I think this (local Server socket) should work the same regardless of the host's support for IPv4, IPv6. I'm happy to investigate if there's any specific report of something not working.

I want to add that requiring clients of httpd.js to check for all the different conditions and correctly start the server is a bit complex. It would be great if 127.0.0.1 could just be used.

Otherwise (without a fix for the above) I could see the following steps:

  1. Have a flag if IPv6 is available based on if [::1] is a resolved IP for localhost
  2. If IPv6 is available start the server with [::1] (and enable dualstack)
  3. Otherwise start with localhost and add 127.0.0.1 as additional identity that also has to be used on our side to report to WebDriver clients.

We could also have a preference that actually allows clients to define if IPv6 should be used. If not set (as per default) IPv4 only would be used.

Please note that external clients have their own DNS resolver and as of Node.js 17 this has been changed from IPv4 per default to IPV6. If we send localhost to them the resulting behavior depends on the Node.js version or maybe some other factors. As such we really want to report the IP address our server runs on and not just localhost.

As discussed with Julian on Element the workaround for now is the following:

  1. Resolve localhost via DNSService.asyncResolve
  2. If the first entry is 127.0.0.1 use localhost as host when starting the server and adding a new identity for 127.0.0.1
  3. If first entry is [::1] start the server with that IP address

Internally we will use the resolved IP from step 1) as reference that is also used to announce the address to clients.

I'm going to keep this bug open for a real fix.

Severity: -- → S3
Priority: -- → P2
Whiteboard: [necko-triaged]
You need to log in before you can comment on or make changes to this bug.