conn: do not chdir for conn.sock #581
Conversation
Instead, we create the socket in a temporary directory - TMPDIR if it is not too long, or else /tmp. The directory is made via `mkdtemp` with the template `urbit-XXXXXX`. The socket is located there (getting around the length limit) and `$pier/.urb/conn.sock` is symlinked to it. Replaces calls to uv_strerror with strerror for system calls; the former does not print system call errors properly.
The pier socket link is predictable; no information is conveyed by printing it.
|
I'm actually indifferent as to whether this or #580 is taken. I think this is a slightly better solution, since it allows clients to always be able to specify the full absolute path to the socket when connecting to it. But either way is fine. |
|
@pkova Are you the right person to review this? |
Now uv_close is never needed as part of failure cleanup. N.B. this can be moved before the close to keep the cleanup logic stacked.
Also handles 0-length TMPDIR.
|
Oh, I guess now there's also In that case the temporary directory should probably be stored on |
|
@mopfel-winrux how do you want to handle If not, then the next best thing is probably to reopen #580 and also apply that to |
| } | ||
| memcpy(tad_c + len_w + sizeof("/urbit-XXXXXX") - 1, "/conn.sock", |
There was a problem hiding this comment.
The string "/urbit-XXXXXX" gets reused a lot, we should make it a constant or macro
|
@mrdomino I'm sorry I completely missed this. We should also do this for |
|
Yeah. How does lick work? Does it make one socket or many?
I guess it's going to be necessary to track and manage the temp dir from some location that is neither conn.c nor lick.c if we go this route.
I also gave a little thought to putting the socket directory under /run/urbit/sampel-palnet rather than /tmp/urbit-XXXXXX. I'm leaning towards /tmp at the moment, but could be persuaded either way.
…On Saturday, March 2nd, 2024 at 17:56, mopfel-winrux ***@***.***> wrote:
***@***.***(https://github.com/mrdomino) I'm sorry I completely missed this. We should also do this for lick.c
—
Reply to this email directly, [view it on GitHub](#581 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAAFT2ZB4P35LA3JYJEJTGLYWJ7NRAVCNFSM6AAAAABBNZHUO6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZUHE3TOOJSGQ).
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
It makes multiple sockets. One per |
|
Ok, so supposing we want to keep sockets symlinked in the pier directory, maybe we can just do a symlink from dev under the temp directory to dev under the pier directory?
…On Monday, March 11th, 2024 at 06:32, mopfel-winrux ***@***.***> wrote:
It makes multiple sockets. One per [%spin name=path] card it is given. It puts them in `.urb/dev/ folder under the name path. we could use that directory
—
Reply to this email directly, [view it on GitHub](#581 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAAFT2Y4ROJDESATN7D2HJ3YXW57LAVCNFSM6AAAAABBNZHUO6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBYGQ2TCNJSGQ).
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Instead, we create the socket in a temporary directory -
TMPDIRif it is not too long, or else/tmp. The directory is made viamkdtempwith the templateurbit-XXXXXX. The socket is located there (getting around the length limit) and$pier/.urb/conn.sockis symlinked to it.This fixes an issue where if urbit is started with
sudo -u pieruserfrom a directory that the pier user doesn't have access to (e.g. a different user's home directory that is chmod 0750), the process would die trying tochdirback to the old directory.It also gives clients more flexibility; previously, they would have had to
chdirto the pier directory to connect to the socket without risking overrunning the length limit. They can still do that (and use".urb/conn.sock"as theirsun_path), but they now also have the option of usingreadlinkand connecting to the full absolute path of the socket without changing directories.Replaces calls to
uv_strerrorwithstrerrorfor system calls; the former does not print system call errors properly.Supersedes #580.