Subject: [PATCH v2] fsmonitor: handle differences between Windows named pipe functions

From: "Eric DeCosta via GitGitGadget" <[email-masked]>
Date: Mon, 10 Apr 2023 19:46:02 +0000

From: Eric DeCosta <[email-masked]>

The two functions involved with creating and checking for the existance of
the local fsmonitor named pipe, CratedNamedPipeW and WaitNamedPipeW appear
to handle names with leading slashes or backslashes a bit differently.

If a repo resolves to a remote file system with the UNC path of
//some-server/some-dir/some-repo, CreateNamedPipeW accepts this name and
creates this named pipe: \\.\pipe\some-server\some-dir\some-repo

However, when the same UNC path is passed to WaitNamedPipeW, it fails with
ERROR_FILE_NOT_FOUND.

Skipping the leading slash for UNC paths works for both CreateNamedPipeW and
WaitNamedPipeW. Resulting in a named pipe with the same name as above that
WaitNamedPipeW is able to correctly find.

Signed-off-by: Eric DeCosta <[email-masked]>
---
    fsmonitor: handle differences between Windows named pipe functions
    
    The two functions involved with creating and checking for the existance
    of the local fsmonitor named pipe, CratedNamedPipeW and WaitNamedPipeW
    appear to handle names with leading slashes or backslashes a bit
    differently.
    
    If a repo resolves to a remote file system with the UNC path of
    //some-server/some-dir/some-repo, CreateNamedPipeW accepts this name and
    creates this named pipe: .\pipe\some-server\some-dir\some-repo
    
    However, when the same UNC path is passed to WaitNamedPipeW, it fails
    with ERROR_FILE_NOT_FOUND.
    
    Skipping the leading slash for UNC paths works for both CreateNamedPipeW
    and WaitNamedPipeW. Resulting in a named pipe with the same name as
    above that WaitNamedPipeW is able to correctly find.
    
    v1 differs from v0:
    
     * Abandon hashing repo path in favor of simpler solution where the
       leading slash is simply dropped for UNC paths

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1503%2Fedecosta-mw%2Ffsmonitor_windows-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1503/edecosta-mw/fsmonitor_windows-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1503

Range-diff vs v1:

 1:  c5307928cd7 ! 1:  b41f9bd2e96 fsmonitor: handle differences between Windows named pipe functions
     @@ Metadata
       ## Commit message ##
          fsmonitor: handle differences between Windows named pipe functions
      
     -    CreateNamedPipeW is perfectly happy accepting pipe names with seemingly
     -    embedded escape charcters (e.g. \b), WaitNamedPipeW is not and incorrectly
     -    returns ERROR_FILE_NOT_FOUND when clearly a named pipe, succesfully created
     -    with CreateNamedPipeW, exists.
     +    The two functions involved with creating and checking for the existance of
     +    the local fsmonitor named pipe, CratedNamedPipeW and WaitNamedPipeW appear
     +    to handle names with leading slashes or backslashes a bit differently.
      
     -    For example, this network path is problemmatic:
     -    \\batfs-sb29-cifs\vmgr\sbs29\my_git_repo
     +    If a repo resolves to a remote file system with the UNC path of
     +    //some-server/some-dir/some-repo, CreateNamedPipeW accepts this name and
     +    creates this named pipe: \\.\pipe\some-server\some-dir\some-repo
      
     -    In order to work around this issue, rather than using the path to the
     -    worktree directly as the name of the pipe, instead use the hash of the
     -    worktree path.
     +    However, when the same UNC path is passed to WaitNamedPipeW, it fails with
     +    ERROR_FILE_NOT_FOUND.
     +
     +    Skipping the leading slash for UNC paths works for both CreateNamedPipeW and
     +    WaitNamedPipeW. Resulting in a named pipe with the same name as above that
     +    WaitNamedPipeW is able to correctly find.
      
          Signed-off-by: Eric DeCosta <[email-masked]>
      
       ## compat/simple-ipc/ipc-win32.c ##
     -@@
     - #include "cache.h"
     -+#include "hex.h"
     - #include "simple-ipc.h"
     - #include "strbuf.h"
     - #include "pkt-line.h"
      @@
       static int initialize_pipe_name(const char *path, wchar_t *wpath, size_t alloc)
       {
       	int off = 0;
     --	struct strbuf realpath = STRBUF_INIT;
     --
     --	if (!strbuf_realpath(&realpath, path, 0))
     --		return -1;
     -+	int ret = 0;
     -+	git_SHA_CTX sha1ctx;
     -+	struct strbuf real_path = STRBUF_INIT;
     -+	struct strbuf pipe_name = STRBUF_INIT;
     -+	unsigned char hash[GIT_MAX_RAWSZ];
     ++	int real_off = 0;
     + 	struct strbuf realpath = STRBUF_INIT;
       
     --	off = swprintf(wpath, alloc, L"\\\\.\\pipe\\");
     --	if (xutftowcs(wpath + off, realpath.buf, alloc - off) < 0)
     -+	if (!strbuf_realpath(&real_path, path, 0))
     + 	if (!strbuf_realpath(&realpath, path, 0))
       		return -1;
       
     --	/* Handle drive prefix */
     --	if (wpath[off] && wpath[off + 1] == L':') {
     --		wpath[off + 1] = L'_';
     --		off += 2;
     --	}
     -+	git_SHA1_Init(&sha1ctx);
     -+	git_SHA1_Update(&sha1ctx, real_path.buf, real_path.len);
     -+	git_SHA1_Final(hash, &sha1ctx);
     -+	strbuf_release(&real_path);
     - 
     --	for (; wpath[off]; off++)
     --		if (wpath[off] == L'/')
     --			wpath[off] = L'\\';
     -+	strbuf_addf(&pipe_name, "git-fsmonitor-%s", hash_to_hex(hash));
     -+	off = swprintf(wpath, alloc, L"\\\\.\\pipe\\");
     -+	if (xutftowcs(wpath + off, pipe_name.buf, alloc - off) < 0)
     -+		ret = -1;
     - 
     --	strbuf_release(&realpath);
     --	return 0;
     -+	strbuf_release(&pipe_name);
     -+	return ret;
     - }
     ++	/* UNC Path, skip leading slash */
     ++	if (realpath.buf[0] == '/' && realpath.buf[1] == '/')
     ++		real_off = 1;
     ++
     + 	off = swprintf(wpath, alloc, L"\\\\.\\pipe\\");
     +-	if (xutftowcs(wpath + off, realpath.buf, alloc - off) < 0)
     ++	if (xutftowcs(wpath + off, realpath.buf + real_off, alloc - off) < 0)
     + 		return -1;
       
     - static enum ipc_active_state get_active_state(wchar_t *pipe_path)
     + 	/* Handle drive prefix */

 compat/simple-ipc/ipc-win32.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/compat/simple-ipc/ipc-win32.c b/compat/simple-ipc/ipc-win32.c
index 997f6144344..632b12e1ab5 100644
--- a/compat/simple-ipc/ipc-win32.c
+++ b/compat/simple-ipc/ipc-win32.c
@@ -19,13 +19,18 @@
 static int initialize_pipe_name(const char *path, wchar_t *wpath, size_t alloc)
 {
 	int off = 0;
+	int real_off = 0;
 	struct strbuf realpath = STRBUF_INIT;
 
 	if (!strbuf_realpath(&realpath, path, 0))
 		return -1;
 
+	/* UNC Path, skip leading slash */
+	if (realpath.buf[0] == '/' && realpath.buf[1] == '/')
+		real_off = 1;
+
 	off = swprintf(wpath, alloc, L"\\\\.\\pipe\\");
-	if (xutftowcs(wpath + off, realpath.buf, alloc - off) < 0)
+	if (xutftowcs(wpath + off, realpath.buf + real_off, alloc - off) < 0)
 		return -1;
 
 	/* Handle drive prefix */

base-commit: f285f68a132109c234d93490671c00218066ace9
-- 
gitgitgadget