Skip to content

Commit 919f1ba

Browse files
committed
fix(logseq-cli): flush OAuth callback response before settling login
The rewritten OCaml CLI resolves effects synchronously: cli/lib/platform/node/cli_effect.ml wakeup invokes each bound callback inline. login_callback_server's on_request settled the login task with finish (Ok result) before write_head/end_, so on the synchronous result paths (state mismatch, missing code, oauth error) the whole login continuation ran up to cli/bin/main.ml's exit on the same stack and the process terminated before the response flushed. logseq-cli-login-callback then failed with "curl: (52) Empty reply from server" despite the patch already binding 127.0.0.1. The success path was spared only because the authorization-code exchange is async and deferred the exit. Write the response first and call finish from the end_ completion callback, so the login task settles after the response is handed to the socket. The dual-family loopback bind (127.0.0.1 and ::1) stays: upstream still binds only the first dns.lookup result. Patch rationale consolidated into the preamble per the no-inline-comment patch convention. Validated against logseqRev 1d1ca70d: git apply --check and patch -p1 --dry-run apply cleanly; nix build .#checks.x86_64-linux.logseq-cli-login-callback and .#checks.x86_64-linux.logseq-cli-help both pass.
1 parent 71b38c2 commit 919f1ba

1 file changed

Lines changed: 35 additions & 29 deletions

File tree

patches/logseq-cli-auth-bind-loopback-address-families.patch

Lines changed: 35 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,50 @@
1-
Bind the CLI OAuth callback server to both loopback address families.
1+
Serve the CLI OAuth login callback reliably on loopback.
22

3-
logseq-cli's Node platform binds the OAuth callback with
4-
Http_server.listen server port "localhost" (cli/lib/platform/node/cli_platform.ml).
5-
Node resolves the host with dns.lookup and binds only the first returned
6-
address, which is often ::1 on systems whose hosts file lists the IPv6 loopback
7-
first. Browsers resolve localhost independently, and browser IPv6 policy can make
8-
the callback connect to the other loopback family. When that happens, the OAuth
9-
callback never reaches the CLI, which keeps waiting until the login timeout.
3+
logseq-cli's Node platform login_callback_server has two defects that keep the
4+
browser OAuth redirect from reaching the CLI
5+
(cli/lib/platform/node/cli_platform.ml). First, it binds with
6+
Http_server.listen server port "localhost". Node resolves the host with
7+
dns.lookup and binds only the first returned address, which is often ::1 on
8+
systems whose hosts file lists the IPv6 loopback first. Browsers resolve
9+
localhost independently, and browser IPv6 policy can make the callback connect
10+
to the other loopback family, so the callback never reaches the CLI and login
11+
waits until the login timeout. Second, on_request settles the login task with
12+
finish (Ok result) before it writes the HTTP response. cli_effect.ml runs effect
13+
continuations synchronously (wakeup invokes each bound callback inline), so a
14+
result that resolves without awaiting the network (state mismatch, missing code,
15+
oauth error) runs the whole login continuation on the same stack up to
16+
cli/bin/main.ml, whose completion handler calls exit on a nonzero code. The
17+
process terminates before write_head/end_ flush, so the browser receives an
18+
empty reply (curl reports "Empty reply from server"). The success path survives
19+
only because the authorization-code exchange is async and defers the exit.
1020

1121
Fix: keep the "localhost" redirect URI registered with the Cognito app client
1222
(auth_state.ml callback_host), but make login_callback_server bind explicit
1323
127.0.0.1 and ::1 listeners that share one request handler and resolver. The
1424
listeners stay loopback-only (no wildcard :: or omitted host), the browser may
1525
resolve localhost to either family, and an EAFNOSUPPORT/EADDRNOTAVAIL on one
16-
family is tolerated as long as the other binds.
26+
family is tolerated as long as the other binds. Then write the callback response
27+
first and call finish from the end_ completion callback, so the login task
28+
settles (and the process may exit) only after the response has been handed to
29+
the socket.
1730

18-
Removal condition: drop this patch when upstream binds explicit loopback literals
19-
for both families, or switches the Cognito app client to IP-literal redirect URIs,
31+
Removal condition: drop this patch when upstream binds explicit loopback
32+
literals for both families (or switches the Cognito app client to IP-literal
33+
redirect URIs) and writes the callback response before settling the login task,
2034
in cli/lib/platform/node/cli_platform.ml. A failed strict apply in the nightly
2135
build or the CLI build is the drift signal.
2236

2337
diff --git a/cli/lib/platform/node/cli_platform.ml b/cli/lib/platform/node/cli_platform.ml
24-
index 89478a6..2a66b95 100644
38+
index 89478a6..8292fbd 100644
2539
--- a/cli/lib/platform/node/cli_platform.ml
2640
+++ b/cli/lib/platform/node/cli_platform.ml
27-
@@ -61,11 +61,33 @@ let js_error_message exn =
41+
@@ -61,11 +61,26 @@ let js_error_message exn =
2842
| Some message when String.trim message <> "" -> message
2943
| _ -> "JavaScript error"
3044

3145
+external error_code : Js.Exn.t -> string option = "code"
3246
+[@@mel.get] [@@mel.return { undefined_to_opt }]
3347
+
34-
+(* Bind the OAuth callback on explicit loopback literals for both address
35-
+ families. Node's listen(port, "localhost") resolves "localhost" via
36-
+ dns.lookup and binds only the first returned address (often ::1 on
37-
+ IPv6-first hosts); a browser that resolves localhost to the other family
38-
+ then never reaches the callback and login hangs until timeout. Keep the
39-
+ redirect URI host as "localhost" (the Cognito app client registers that
40-
+ exact URL) but listen on 127.0.0.1 and ::1. *)
4148
+let loopback_bind_hosts host =
4249
+ if String.equal host "localhost" then [ "127.0.0.1"; "::1" ] else [ host ]
4350
+
@@ -59,7 +66,7 @@ index 89478a6..2a66b95 100644
5966
let clear_timer () =
6067
match !timeout_ref with
6168
| None -> ()
62-
@@ -73,18 +95,16 @@ let login_callback_server ~host ~port ~timeout_span ~on_listen ~handle_request =
69+
@@ -73,18 +88,16 @@ let login_callback_server ~host ~port ~timeout_span ~on_listen ~handle_request =
6370
timeout_ref := None;
6471
Timer.clear_timeout timeout_handle
6572
in
@@ -83,18 +90,22 @@ index 89478a6..2a66b95 100644
8390
Cli_effect.wakeup resolver result)
8491
in
8592
let on_request request response =
86-
@@ -100,9 +120,7 @@ let login_callback_server ~host ~port ~timeout_span ~on_listen ~handle_request =
93+
@@ -96,13 +109,10 @@ let login_callback_server ~host ~port ~timeout_span ~on_listen ~handle_request =
94+
let response_body, result =
95+
handle_request { target = Http_server.target request }
96+
in
97+
- finish (Ok result);
8798
Http_server.write_head response response_body.status
8899
(Http_server.text_headers ());
89100
Http_server.end_ response response_body.body (fun[@u] () ->
90101
- match !server_ref with
91102
- | None -> ()
92103
- | Some server -> Http_server.close_ignore server)
93-
+ List.iter Http_server.close_ignore !servers)
104+
+ finish (Ok result))
94105
in
95106
let on_listening () =
96107
let timeout = timeout_ms timeout_span in
97-
@@ -120,20 +138,50 @@ let login_callback_server ~host ~port ~timeout_span ~on_listen ~handle_request =
108+
@@ -120,20 +130,45 @@ let login_callback_server ~host ~port ~timeout_span ~on_listen ~handle_request =
98109
(fun exn ->
99110
finish (Error (Login_callback_server_aborted (Printexc.to_string exn))))
100111
in
@@ -105,16 +116,11 @@ index 89478a6..2a66b95 100644
105116
- in
106117
- server_ref := Some server;
107118
- Http_server.on_error server (fun[@u] error ->
108-
+ (* Arm the timeout and open the browser once, on the first family that binds. *)
109119
+ let first_bind () =
110120
+ if not !listened then (
111121
+ listened := true;
112122
+ on_listening ())
113123
+ in
114-
+ (* If every listen attempt settles with nothing bound, the callback cannot be
115-
+ served on any loopback family: fail like the single-server path did. An
116-
+ EAFNOSUPPORT/EADDRNOTAVAIL on one family is tolerated as long as the other
117-
+ binds. *)
118124
+ let note_attempt_settled () =
119125
+ if !pending = 0 && !bound = 0 && not !settled then
120126
+ finish

0 commit comments

Comments
 (0)