ninenines / ranch

Socket acceptor pool for TCP protocols.
ISC License
1.19k stars 336 forks source link

Add option to set permissions of local sockets #321

Closed juhlig closed 3 years ago

juhlig commented 3 years ago

320 Socket file permissions can be set with the sockfile_mode transport option. No docs yet, will do them when the code has been approved.

juhlig commented 3 years ago

The MacOS agent seems to be down, so the tests don't complete...

I've been thinking about the sockfile_mode option, it doesn't seem right to place it directly in the transport options. It is more of a socket option really, and might not make sense generally (eg with custom transports), so I would change that to be passed in the socket options, to be checked and filtered out in ranch_tcp/ranch_ssl? What do you think?

essen commented 3 years ago

Well ideally Ranch wouldn't have to do it.

Do we have another socket option that is exclusive to Ranch and not passed to gen_tcp or ssl?

juhlig commented 3 years ago

Well ideally Ranch wouldn't have to do it.

Yeah, but there is no way to tell gen_tcp/inet and ssl anything about how to create the socket file. I guess the same thinking applies there...

Do we have another socket option that is exclusive to Ranch and not passed to gen_tcp or ssl?

Nope.

Another way would be to allow more transport options than the ones we currently have and validate via ranch:validate_transport_opts/1 and ranch:validate_transport_opt/3. Currently, when an unknown transport option is found in the map, we return {error, {bad_option, Key}} immediately. Instead, we could try Transport:validate_transport_opt(...), as it may be an option that makes sense and is thus allowed for the specific Transport. Not sure how you would correctly typespec this.

essen commented 3 years ago

How about we simply add an optional callback to be called after a successful listen which receives the socket as only argument? Then the user can do whatever they want to the socket.

juhlig commented 3 years ago

Good idea. This way, we also offload a lot of future wishes to the users, too, like changing sockfile ownership and whatnot :)

juhlig commented 3 years ago

@essen I did a quick draft, see here, what do you think?

The transport option is called post_listen (suggestions for a better name welcome) which takes a 1-ary fun (could be extended to also accept {Mod, Fun}?), which may return either ok, {ok, NewSocket} (for returning a new/transformed socket, may be useful?), or {error, Reason}.

essen commented 3 years ago

I'll get back to you on Friday.

juhlig commented 3 years ago

I'll get back to you on Friday.

Sure :)

In the meantime, I opened an alternative PR to this one, #322. I dropped the feature of returning a new socket from the callback, I can't think of a real use case for that. It's no big problem of adding it back in the future if the need arises.

juhlig commented 3 years ago

I'll close this in favor of #322.