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

Add Unix domain socket support. #25

Merged
merged 1 commit into from
May 24, 2021

Conversation

sysvinit
Copy link
Contributor

Details in the commit message.

This adds an optional boolean-valued configuration file option
"unixsocket". When specified, it changes the meaning of "listenport" to
indicate the path to a Unix domain socket on which prosody-filer should listen,
instead of a TCP port.

This is useful for multi-user shared hosting environments, where listening on
loopback TCP sockets may allow other, unrelated users to connect to the filer.
@cbix
Copy link
Contributor

cbix commented Dec 23, 2020

You might want to checkout my PR from a while ago (now conflicting, but I'll try to rebase it soon). The goal was also to allow unix sockets, but is based on systemd (which, according to your username, you probably don't use? 😜).

@ThomasLeister ThomasLeister self-assigned this Dec 23, 2020
@ThomasLeister ThomasLeister added the enhancement New feature or request label Dec 23, 2020
@sysvinit
Copy link
Contributor Author

The box where I'm running prosody-filer is just a stock Debian box, so it does indeed have systemd ;). My only comment on #22 would be that it requires systemd-specific functionality in order to enable listening on Unix sockets, while adding an explicit option makes this a little more portable (both to non-systemd Linux machines, but also on BSD's etc).

@ThomasLeister
Copy link
Owner

@sysvinit and @cbix nice idea to enable socket support! Indeed I like the "general approach" here more. Is @sysvinit 's approach also compatible to systemd things? I haven't looked into the details, yet and I'm not very familiar with that topic.

@sysvinit
Copy link
Contributor Author

systemd's socket activation functionality is very similar to the traditional Unix inetd(8) -- the supervisor opens and listens on a socket, and then only when connections arrive on that socket is a process started to handle that connection. That process can then tell if it's been "socket activated" by checking for the presence of an environment variable set by systemd, which indicates which file descriptor numbers correspond to the listening socket(s) which systemd had opened previously. This means the service isn't started until it receives a request, which might be an advantage in some circumstances.

My changes are compatible with systemd inasmuch as they don't do anything which conflicts with systemd; they just don't use any systemd-specific functionality. (As it happens I have more than one instance of prosody-filer running on my machine which handles XMPP uploads, which I manage using the systemd user unit below:

# prosody-filer@.service
[Unit]
Description=prosody external upload handler

[Service]
Type=simple
ExecStartPre=/bin/rm -f /srv/sites/molly/run/prosody-filer-%I.sock
ExecStart=/home/molly/bin/prosody-filer -config /home/molly/conf/prosody-filer-%I.toml

[Install]
WantedBy=default.target

)

@cbix
Copy link
Contributor

cbix commented Dec 23, 2020

@sysvinit I like your approach! And yes, it all works side by side, I'll just have to make some changes to my PR later.

Anyway it's a good idea to leave the choice to the user, I don't know if packagers are usually opinionated about systemd vs. "manually configured" network services, in case this project aims to eventually make it into distro repos.

Also, happy holidays! 🎅

@ThomasLeister ThomasLeister merged commit 2638338 into ThomasLeister:develop May 24, 2021
@sysvinit sysvinit deleted the unix-sockets branch May 24, 2021 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants