Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nrepl wrongly prefers ip4 on dual stack systems #302

Open
birdspider opened this issue Oct 2, 2023 · 4 comments
Open

nrepl wrongly prefers ip4 on dual stack systems #302

birdspider opened this issue Oct 2, 2023 · 4 comments
Labels

Comments

@birdspider
Copy link

birdspider commented Oct 2, 2023

Starting nrepl with default params (no -b) launches it on 127.0.0.1. It happily reports that it started/bound to localhost:99999 . Other software (calva) connect via printed message and fails because ::1:99999 (ipv6) is not listened on.

(this started as BetterThanTomorrow/calva#2310)

(BetterThanTomorrow/calva#2310 (comment) is why I think this started occurring after Aug 2023)


since nrepl binds to a port, it should print the address instead of the host

Expected behavior

banner text to print connectable address

nREPL server started on port 36009 on host localhost - nrepl://127.0.0.1:36009

or

nREPL server started on port 36009 on host localhost - nrepl://::1:36009

Actual behavior

banner text to port not listed on

nREPL server started on port 36009 on host localhost - nrepl://localhost:36009

Steps to reproduce the problem

  1. have a dualstack system (ip4 + ip6)
  2. start nrepl (without -b)
  3. nrepl starts (hardbinds to 127.0.0.1)
  4. nrepl prints nrepl://localhost:36009
  5. calva/some software wants to connect - resolves localhost - ::1
  6. and finally nREPL connection failed: Error: connect ECONNREFUSED ::1:36009

Environment & Version information

Clojure version

1.11.1

Java version

openjdk version "21" 2023-09-19
OpenJDK Runtime Environment (build 21+35)
OpenJDK 64-Bit Server VM (build 21+35, mixed mode, sharing)

Operating system

arch linux


quickfix:

diff --git a/src/clojure/nrepl/socket.clj b/src/clojure/nrepl/socket.clj
--- src/clojure/nrepl/socket.clj
+++ src/clojure/nrepl/socket.clj
@@ -207,9 +207,9 @@
           (URI. (str transport-scheme
                      (when (instance? SSLServerSocket sock)
                        "s"))
                 nil ;; userInfo
-                (-> sock .getInetAddress .getHostName)
+                (-> sock .getInetAddress .getHostAddress)
                 (.getLocalPort sock)
                 nil       ;; path
                 nil       ;; query
                 nil)))))) ;; fragment
@bbatsov bbatsov added the Bug label Nov 9, 2023
@bbatsov
Copy link
Contributor

bbatsov commented Jan 14, 2024

nrepl starts (hardbinds to 127.0.0.1)

I recall we did this for a reason back in the day (something to do with dual stack systems), but I no longer remember the reason. 😄 I'll have to dig through the old conversations about the default host.

nrepl prints nrepl://localhost:36009

From what I get the issue for you is what we're printing, right?

@birdspider
Copy link
Author

From what I get the issue for you is what we're printing, right?

yes

however, make that low priority - currently it works again and since there is so much involved (jvm, nodejs, linux, linux-settings, actually having a ip-dualstack) I'm not sure why and I'd have to take a look.

@cichli
Copy link
Member

cichli commented May 10, 2024

Would it make sense to print whatever :port and :bind were passed to start-server rather than relying on the socket's InetAddress? Then we avoid the question of whether to call getHostName or getHostAddress.

The default was changed from localhost to 127.0.0.1 in c6b1328 FWIW (shortly after changing it from ::).

@bbatsov
Copy link
Contributor

bbatsov commented May 11, 2024

@cichli I think that's a good suggestion. PR welcome!

cichli added a commit to cichli/nrepl that referenced this issue May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants