pipe: add back error handling to connect / bind (libuv#4202)

This was incorrectly dropped by libuv#4030, where previously connecting to "" might fail eventually, now instead it would return EINVAL and then fail to initialize the struct or call the callback.
vtjnash · Nov 14, 2023 · 8bb7e11 · 8bb7e11
1 parent 633629e
commit 8bb7e11
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 13 deletions.
diff --git a/src/unix/pipe.c b/src/unix/pipe.c
@@ -206,7 +206,22 @@ void uv_pipe_connect(uv_connect_t* req,
                     uv_pipe_t* handle,
                     const char* name,
                     uv_connect_cb cb) {
-  uv_pipe_connect2(req, handle, name, strlen(name), 0, cb);
+  int err;
+
+  err = uv_pipe_connect2(req, handle, name, strlen(name), 0, cb);
+
+  if (err) {
+    handle->delayed_error = err;
+    handle->connect_req = req;
+
+    uv__req_init(handle->loop, req, UV_CONNECT);
+    req->handle = (uv_stream_t*) handle;
+    req->cb = cb;
+    uv__queue_init(&req->queue);
+
+    /* Force callback to run on next tick in case of error. */
+    uv__io_feed(handle->loop, &handle->io_watcher);
+  }
 }
 
 
@@ -286,7 +301,7 @@ int uv_pipe_connect2(uv_connect_t* req,
   handle->connect_req = req;
 
   uv__req_init(handle->loop, req, UV_CONNECT);
-  req->handle = (uv_stream_t*)handle;
+  req->handle = (uv_stream_t*) handle;
   req->cb = cb;
   uv__queue_init(&req->queue);
 

diff --git a/src/win/pipe.c b/src/win/pipe.c
@@ -809,7 +809,19 @@ void uv_pipe_connect(uv_connect_t* req,
                     uv_pipe_t* handle,
                     const char* name,
                     uv_connect_cb cb) {
-  uv_pipe_connect2(req, handle, name, strlen(name), 0, cb);
+  uv_loop_t* loop;
+  int err;
+
+  err = uv_pipe_connect2(req, handle, name, strlen(name), 0, cb);
+
+  if (err) {
+    loop = handle->loop;
+    /* Make this req pending reporting an error. */
+    SET_REQ_ERROR(req, err);
+    uv__insert_pending_req(loop, (uv_req_t*) req);
+    handle->reqs_pending++;
+    REGISTER_HANDLE_REQ(loop, handle, req);
+  }
 }
 
 
@@ -819,11 +831,20 @@ int uv_pipe_connect2(uv_connect_t* req,
                      size_t namelen,
                      unsigned int flags,
                      uv_connect_cb cb) {
-  uv_loop_t* loop = handle->loop;
-  int err, nameSize;
+  uv_loop_t* loop;
+  int err;
+  size_t nameSize;
   HANDLE pipeHandle = INVALID_HANDLE_VALUE;
   DWORD duplex_flags;
 
+  loop = handle->loop;
+  UV_REQ_INIT(loop, req, UV_CONNECT);
+  req->handle = (uv_stream_t*) handle;
+  req->cb = cb;
+  req->u.connect.pipeHandle = INVALID_HANDLE_VALUE;
+  req->u.connect.duplex_flags = 0;
+  req->u.connect.name = NULL;
+
   if (flags & ~UV_PIPE_NO_TRUNCATE) {
     return UV_EINVAL;
   }
@@ -844,13 +865,6 @@ int uv_pipe_connect2(uv_connect_t* req,
     return UV_ENAMETOOLONG;
   }
 
-  UV_REQ_INIT(loop, req, UV_CONNECT);
-  req->handle = (uv_stream_t*) handle;
-  req->cb = cb;
-  req->u.connect.pipeHandle = INVALID_HANDLE_VALUE;
-  req->u.connect.duplex_flags = 0;
-  req->u.connect.name = NULL;
-
   if (handle->flags & UV_HANDLE_PIPESERVER) {
     err = ERROR_INVALID_PARAMETER;
     goto error;

diff --git a/test/test-pipe-bind-error.c b/test/test-pipe-bind-error.c
@@ -34,6 +34,7 @@
 
 
 static int close_cb_called = 0;
+static int connect_cb_called = 0;
 
 
 static void close_cb(uv_handle_t* handle) {
@@ -180,6 +181,14 @@ TEST_IMPL(pipe_bind_or_listen_error_after_close) {
   return 0;
 }
 
+
+static void connect_overlong_cb(uv_connect_t* connect_req, int status) {
+  ASSERT_EQ(status, UV_EINVAL);
+  connect_cb_called++;
+  uv_close((uv_handle_t*) connect_req->handle, close_cb);
+}
+
+
 TEST_IMPL(pipe_overlong_path) {
   char path[512];
   uv_pipe_t pipe;
@@ -196,9 +205,17 @@ TEST_IMPL(pipe_overlong_path) {
                              sizeof(path),
                              UV_PIPE_NO_TRUNCATE,
                              (uv_connect_cb) abort));
-  uv_close((uv_handle_t*) &pipe, NULL);
   ASSERT_OK(uv_run(uv_default_loop(), UV_RUN_DEFAULT));
 
+  ASSERT_EQ(UV_EINVAL, uv_pipe_bind(&pipe, ""));
+  uv_pipe_connect(&req,
+                  &pipe,
+                  "",
+                  (uv_connect_cb) connect_overlong_cb);
+  ASSERT_OK(uv_run(uv_default_loop(), UV_RUN_DEFAULT));
+  ASSERT_EQ(1, connect_cb_called);
+  ASSERT_EQ(1, close_cb_called);
+
   MAKE_VALGRIND_HAPPY(uv_default_loop());
   return 0;