Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: use filepath package when connecting to UNIX sockets #1179

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stapelberg
Copy link

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

@stapelberg
Copy link
Author

BTW, I verified that the github.com/jackc/pgx package behaves correctly on Windows when connecting to UNIX domain sockets. I think it would be good to fix this issue in lib/pq as well :)

Copy link
Contributor

@johto johto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I can push this next week unless someone beats me to it.

@cbandy
Copy link
Contributor

cbandy commented Nov 16, 2024

👍🏻 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
@stapelberg
Copy link
Author

Thanks for the quick review!

🌱 Should this code consider abstract sockets, too?

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.

@stapelberg
Copy link
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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants