Fix link and socket detection in search methods

[djdv]

Since we don't use the result and only want to know if the target exists, we can do even less processing of the target by using Lstat.
That is, don't resolve links, don't call file relevant stat methods on non-files like sockets, etc. Just check if they exist.
This specifically resolves an issue with os.Stat returning an error if the path is a Unix domain socket, on Windows (despite the path being valid and usable).

For example, this: golang/go#33357 (comment)
will return CreateFile server.sock: The file cannot be accessed by the system.
While changing to Lstat returns information on the target without error.
I'm experiencing this same issue with my own Go program which is creating and using valid Unix sockets, but they're not being found by xdg.SearchRuntimeFile, xdg.SearchConfigFile, etc.

This specific issue with Stat may need to be fixed upstream, but using Lstat here may still make sense for symlinks, and other types.
*(Unless this goes against some xdg expectation, I'm not sure)

[codecov]

Codecov Report

Merging #14 (dcee951) into master (1f7450a) will decrease coverage by 5.85%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #14      +/-   ##
==========================================
- Coverage   92.63%   86.77%   -5.86%     
==========================================
  Files           6        5       -1     
  Lines         190      121      -69     
==========================================
- Hits          176      105      -71     
- Misses         13       15       +2     
  Partials        1        1              

Impacted Files	Coverage Δ
utils.go	71.92% <ø> (-4.35%)	⬇️
exist.go	100.00% <100.00%> (ø)
paths_darwin.go
paths_windows.go

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f7450a...dcee951. Read the comment docs.

[adrg]

Hi @djdv. Thank you for the PR. So there are no issues with finding the file when using os.Lstat instead of os.Stat.
Are you able to read/write from/to the socket after it is found?

Is there any way to isolate this scenario? Are the os.ModeSocket bits set when retrieving the info using os.Lstat?
I think it would be good to use os.Stat most of the time so that broken symbolic links are skipped when searching files (e.g. when using xdg.SearchConfigFile, which searches multiple paths).

[djdv]

So there are no issues with finding the file when using os.Lstat instead of os.Stat.

It seems so. This continues to work fine for files, but now also detects the socket I'm searching for. (this probably already worked fine on *nix, but not on Windows because of golang/go#33357).

Are you able to read/write from/to the socket after it is found?

I can. I put an example of how I'm getting and using the path below, but it's kind of convoluted.
In short though, yeah. Everything seems to work as expected. Where before I wasn't sure why it was not being detected.

Are the os.ModeSocket bits set when retrieving the info using os.Lstat?

It seems like no, but this may be a bug in Go (same issue linked above), I'm not sure if this will change or not.

	fmt.Printf("%d\n", fi.Mode()&os.ModeSocket)

is returning 0 when the fi comes from these Unix sockets. It seems like it should though.

I think it would be good to use os.Stat most of the time so that broken symbolic links are skipped when searching files (e.g. when using xdg.SearchConfigFile, which searches multiple paths

I was wondering about that specifically. It seems like as the application developer you would still want to get a path to broken links (so that you can detect this and warn, or fix the path)
That said, I'm not sure what's conventional for xdg standards. Skipping broken files may make more sense.

---

Current usage: https://www.youtube.com/watch?v=Bd5wSKKf4ac
Before the socket is created, I check if the process is running as a regular user, or as a system service.
If it's a system service, I make a directory manually with more open permissions, as high in the hierarchy as I can.
Otherwise I use the directory created by RuntimeFile, as-is.
(For now, the actual directory creation happens when the service starts, not here)

...
	if service.Interactive() { // use the most user-specific path
		if servicePath, err = xdg.RuntimeFile(serviceName); err != nil {
			return nil, err
		}
	} else { // For system services, use the least user-specific path
		// NOTE: xdg standard says this list should always have a fallback
		// (so this list should never contain less than 1 element - regardless of platform)
		leastLocalDir := xdg.ConfigDirs[len(xdg.ConfigDirs)-1]
		servicePath = filepath.Join(leastLocalDir, serviceName)
	}

	return multiaddr.NewMultiaddr("/unix/" + servicePath)
}

Client side, we'll come back to this same function, and use whichever path has the socket in it.
Preferring the runtime one if one's found, or falling back to the config locations.

func localServiceMaddr() (multiaddr.Multiaddr, error) {
	ourName := filepath.Base(os.Args[0])
	ourName = strings.TrimSuffix(ourName, filepath.Ext(ourName))
	serviceName := filepath.Join(ourName, serviceTarget)

	// use existing if found
	servicePath, err := xdg.SearchRuntimeFile(serviceName)
	if err == nil {
		return multiaddr.NewMultiaddr("/unix/" + servicePath)
                // ^ this returns the path we expect if the server is being run by a regular user
	}
	if servicePath, err = xdg.SearchConfigFile(serviceName); err == nil {
		return multiaddr.NewMultiaddr("/unix/" + servicePath)
                // ^ this returns the path we expect if the server is running globally on the system
	}
...

With Stat neither will find it, with Lstat both seem to find it in any combination I can run this process in.

[adrg]

Wow, thank you for the detailed explanation.
It may be possible to use os.Stat and if that fails, use os.Lstat as a fallback. If os.Stat fails but os.Lstat succeeds, then I can check if the file is symbolic link. If it is, it means that it is a broken link. Otherwise, it must be a socket, a named pipe, a device, things like that. That is if the symbolic link bits are set correctly and there's no bug like the one you referenced. I'll have to give this a bit of thought and make some tests. I could also limit this behavior to Windows only.

[adrg]

I made a quick test on Windows using the C server from golang/go#33357. With the server started, I called os.Stat from Go on the socket file and it works fine for me. Yes, the socket bits are not set on the file info, but there is no access error.

Are you sure you have the right permissions set on the fs directory and on the filesystem.service file (the ones from the video link you attached)? Maybe try enabling all permissions for Owners, Administrators and Users both on the created file and on the parent directory in order to test it.

[djdv]

Wow, thank you for the detailed explanation.

:^)

It may be possible to use os.Stat and if that fails, use os.Lstat as a fallback...

Seems like a good idea to me. 👍 Any special existence checking code will still remain in exists() even if it has to be broken out into build constrained files for edge cases.

With the server started, I called os.Stat from Go on the socket file and it works fine for me. Yes, the socket bits are not set on the file info, but there is no access error.

I'm wondering if it has something to do with versions. I'm using Go 1.16, on Windows 10 (19042.844).

I tried using more relaxed permissions, but couldn't get Stat to return an info object, only Lstat.
Windows syscalls like GetFileAttributes, and tools like MSYS2's stat.exe are returning results for these paths as well.
(The native call being more accurate, but neither seem to get access errors in any combination of permissions I've tried)

I tried various tools against these paths: https://www.youtube.com/watch?v=J3hjW65Acns
But get similar behavior when using the C server, and/or different paths like %TMP%, or home which should have lax permissions also.

I'll have to see if a clean VM has different results than my current host environment.

[adrg]

I updated Go to 1.16 and installed all the Windows updates so that we have matching environments and still cannot reproduce the issue using the C server from that issue. I want to be able to reproduce it so that I can add test cases for any changes I introduce. Would it be possible for you to make a video showcasing how you reproduce the issue using the C server and that Go sample from golang/go#33357? You could also link the video to your comment on that issue.

[djdv]

@adrg
Hey again, sorry for the delayed response.

I updated Go to 1.16 and installed all the Windows updates so that we have matching environments and still cannot reproduce the issue using the C server from that issue.

Strange. I tried this in a relatively stock VM as well as my host but got the same problem.

Would it be possible for you to make a video showcasing how you reproduce the issue using the C server and that Go sample from golang/go#33357? You could also link the video to your comment on that issue.

For sure :^)
I put one up here: https://www.youtube.com/watch?v=XaKrxpxrbtY
and edited my comment to include it here: golang/go#33357 (comment)

I noticed a few other projects are implementing similar workarounds by using Lstat, and also noticed this
microsoft/Windows-Containers#97 (comment)

Going to implement the fallback idea you mentioned, and see about making a test case for this too.
But will wait a little longer to see if things are going to change in either Windows or Go itself since it might influence this.
(will probably push something by the end of the week if there's no news though)

[djdv]

I removed the previous change (Stat->Lstat) and added a test which creates a Unix socket and looks for it.
Seems like it's passing except on Windows. However the CI isn't even letting the socket be created in the first place.

Locally I get this:

> go test -run TestSocketFile
--- FAIL: TestSocketFiles (0.00s)
    xdg_test.go:140: C:\Users\Dominic Della Valle\AppData\Local\app.socket
    xdg_test.go:144:
                Error Trace:    xdg_test.go:144
                Error:          Received unexpected error:
                                could not locate `app.socket` in any of the following paths: C:\Users\Dominic Della Valle\AppData\Local
                Test:           TestSocketFiles
    xdg_test.go:145:
                Error Trace:    xdg_test.go:145
                Error:          Not equal:
                                expected: "C:\\Users\\Dominic Della Valle\\AppData\\Local\\app.socket"
                                actual  : ""

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -C:\Users\Dominic Della Valle\AppData\Local\app.socket
                                +
                Test:           TestSocketFiles
    xdg_test.go:140: C:\Users\Dominic Della Valle\AppData\Local\appname\app.socket
    xdg_test.go:144:
                Error Trace:    xdg_test.go:144
                Error:          Received unexpected error:
                                could not locate `app.socket` in any of the following paths: C:\Users\Dominic Della Valle\AppData\Local\appname
                Test:           TestSocketFiles
    xdg_test.go:145:
                Error Trace:    xdg_test.go:145
                Error:          Not equal:
                                expected: "C:\\Users\\Dominic Della Valle\\AppData\\Local\\appname\\app.socket"
                                actual  : ""

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -C:\Users\Dominic Della Valle\AppData\Local\appname\app.socket
                                +
                Test:           TestSocketFiles
FAIL
exit status 1
FAIL    github.com/adrg/xdg     0.038s

The underlying error is being lost inside of exists() so it's not printed here.
The same test passes locally when using Lstat instead.

I think we're blocked on upstream making changes to Stat to fix socket paths.

As for broken symlinks, I can implement the fallback check if you'd like.
That is, if Stat fails, we can check if the error is a path error, call Lstat on the same path, check if it's a symlink, and if it is, we can return true from exists.
Or whatever else seems appropriate.

Likewise let me know if there's any changes that should be made for the test itself. I kind of copy pasted it quickly from the one above. lol

[adrg]

Hi @djdv. Sorry for the late response.

Yeah, the workaround seems like the only option for now, although it's a bit of a challenge to create test cases for the changes.
It might be possible to use syscall.Socket or run the C server provided in golang/go#33357 in the Github Actions workflow in order to create the file.

There's also the issue that os.Stat does not work for symlinks to directories on Windows, described in microsoft/Windows-Containers#97.

Not sure what's the best course of action. Ideally, the problem would be solved by a Windows update or by the Go team. But we cannot be sure that it will happen soon. In any case, if we were to implement the workaround, I think it is best to limit that behavior to Windows. Another option (not ideal) would be to use os.Lstat all the time in combination with os.Readlink for symlinks.

[djdv]

I changed net.Listen to use the underlying syscalls, and it seems to work fine locally, but it's the same issue on the Windows CI.
It seems like a network service is not available on the worker.
Locally I'm just running:

go test -run TestSocketFile
PASS
ok      github.com/adrg/xdg     0.034s

and it creates, detects, and cleans up the sockets.

Edit: this looks relevant
https://travis-ci.community/t/socket-the-requested-service-provider-could-not-be-loaded-or-initialized/1127

---

I also split exists out into its own file so that it only calls Lstat on Windows.
But this can be reverted later if os.Stat is changed upstream.

[adrg]

Thanks @djdv. I think I'm going to go with os.Lstat + os.Readlink on Windows. The other workaround is a bit inefficient in some circumstances because it calls os.Stat first and if it fails, it falls back to os.Lstat and os.Readlink is still necessary for symbolic links. I'll make some tests to see how this behaves in multiple scenarios and get back to you. I'm hoping to have a fix for this by the end of the week.

[adrg]

@djdv Can you test PR #16 in order to check that it solves the problems you were having with the Unix socket?
Maybe also run the test cases on your system if possible.

[djdv]

Superseded by: #16