Bugfix: use filepath package when connecting to UNIX sockets#1179
Merged
johto merged 1 commit intolib:masterfrom Nov 26, 2024
Merged
Bugfix: use filepath package when connecting to UNIX sockets#1179johto merged 1 commit intolib:masterfrom
johto merged 1 commit intolib:masterfrom
Conversation
Contributor
Author
|
BTW, I verified that the |
johto
approved these changes
Nov 16, 2024
Contributor
johto
left a comment
There was a problem hiding this comment.
LGTM. I can push this next week unless someone beats me to it.
Contributor
|
👍🏻 filepath 🌱 Should this code consider abstract sockets, too? |
The code before this commit is non-portable: it only works on UNIX systems where path.Join() is close enough to filepath.Join(). However, path.Join() is the wrong choice: UNIX domain sockets are identified via their file system name, so — conceptually and in practice — we need to use the filepath package instead. This commit fixes connecting to a PostgreSQL database that is listening on a UNIX domain socket on Windows. If you are surprised to see UNIX domain sockets on Windows, they were introduced with Windows 10, i.e. are widely available now: https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/ filepath.IsAbs() is equivalent to strings.HasPrefix(s, "/") on UNIX, but also works on Windows, where absolute paths start with e.g. C:\ Similarly, filepath.Join() uses the correct path separator. Also recognize UNIX domain sockets in the abstract name space (represented by a leading @ character) while here. related to zombiezen/postgrestest#3
7821951 to
b7ffbd3
Compare
Contributor
Author
|
Thanks for the quick review!
That’s a good point. I wasn’t sure if PostgreSQL actually supported abstract sockets, but yes, indeed it does: https://www.postgresql.org/docs/current/runtime-config-connection.html#GUC-UNIX-SOCKET-DIRECTORIES I updated the Pull Request accordingly and verified that both listening and connecting to abstract UNIX domain sockets works on Linux. |
Contributor
Author
|
@johto Is pushing this still on your radar? :) Just want to make sure this doesn’t get lost, as it’s a straight-forward fix. Thank you! |
johto
pushed a commit
that referenced
this pull request
Nov 26, 2024
The code before this commit is non-portable: it only works on UNIX systems where path.Join() is close enough to filepath.Join(). However, path.Join() is the wrong choice: UNIX domain sockets are identified via their file system name, so — conceptually and in practice — we need to use the filepath package instead. This commit fixes connecting to a PostgreSQL database that is listening on a UNIX domain socket on Windows. If you are surprised to see UNIX domain sockets on Windows, they were introduced with Windows 10, i.e. are widely available now: https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/ filepath.IsAbs() is equivalent to strings.HasPrefix(s, "/") on UNIX, but also works on Windows, where absolute paths start with e.g. C:\ Similarly, filepath.Join() uses the correct path separator. Also recognize UNIX domain sockets in the abstract name space (represented by a leading @ character) while here. related to zombiezen/postgrestest#3 GitHub: #1179
Contributor
|
Pushed. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The code before this commit is non-portable: it only works on UNIX systems where path.Join() is close enough to filepath.Join(). However, path.Join() is the wrong choice: UNIX domain sockets are identified via their file system name, so — conceptually and in practice — we need to use the filepath package instead.
This commit fixes connecting to a PostgreSQL database that is listening on a UNIX domain socket on Windows. If you are surprised to see UNIX domain sockets on Windows, they were introduced with Windows 10, i.e. are widely available now: https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
filepath.IsAbs() is equivalent to strings.HasPrefix(s, "/") on UNIX, but also works on Windows, where absolute paths start with e.g. C:\ Similarly, filepath.Join() uses the correct path separator.
related to zombiezen/postgrestest#3