Expose socket info in new_session

[wiktor-k]

This is not yet ready and may need a couple of tweaks... so... I'm handing it over to @jcspencer :)

[jcspencer]

Hi @wiktor-k! Thanks for opening this one.

After having a play around with this, I think we have two options here:

1. Keep this PR as-is, and have users match on socket.s.downcast_ref::<Type>() to get a "concrete" listener socket.
   • PR WIP: Add ClientInfo type to ListeningSocket #61 builds on this idea by also introducing a ClientInfo type parameter to the ListeningSocket trait, and letting users deduce type information using the Any trait. (Though I'm not set on that idea, as it has it's own problems)
   • This is probably the easiest option, and keeps the extensibility for users to bring their own ListeningSocket implementation.
2. Introduce an enum with Unix(&T), Tcp(&T), and NamedPipe(&T) to pass a typed variant of the socket to new_session
   • This is probably cleaner from a typing perspective (no need to try a bunch of downcast_refs), but does lock the consumer into the ListeningSocket impls in this library.

Personally I think the first option is probably easier for the sake of extensibility, but does lead users to have to do some type wrangling. What do you think?

[wiktor-k]

Okay, phew, check out the latest iteration of changes here. I hope I found an "option 3" :)

So, under this new scheme there's no Agent (only listen remained, as a free-standing function) and the new_session has been moved to the SessionFactory trait (happy to hear ideas for a better name, since my 15-years of Java experience is showing 😅 ).

The client gives us the ListeningSocket and a SessionFactory which produces Sessions for that type of socket. This way the implementation receives concrete objects, as shown in the key_storage example (the Listener there is a UnixListener on Unix and NamedPipeListener for Windows, it's a bit confusing). This way there's no need for dynamic casts and no need for enums.

The only downside is that if the client wants to support all types of sockets (tcp, unix, named pipes) they need to provide 3 implementations of the SocketFactory but there's a blanket implementation for Default, so as, you can see in the README not much changes. (I'm also thinking about adding blanked implementation for types implementing Clone as it'd simplify key_storage example).

Happy to hear your input!

Thanks for providing me with the motivation to do all these adjustments, code examples and comments - they really are invaluable 🙇 . Sorry for the delays but I'm procrastinating a bit while I should be looking for a Rust job 😃

[wiktor-k]

Okay folks, I've refactored it a bit and just renamed SessionFactory back into Agent and it looks a bit better now 😅

I've also rebased this on top of the OpenPGP Card example and added a new fresh example which accesses and displays the underlying socket info (with peer info in case of a unix socket).

I've changed the blanked implementation from Default to Clone since based on the examples it seems Sessions are most frequently cloned from each other. It doesn't have to be like that and the new example illustrates how to do it manually.

If you don't mind I'd be happy if you too a look @jcspencer and @baloo 🙏

[jcspencer]

Hi @wiktor-k, sorry for the delay in reviewing this! This looks fantastic, and the way you've structured it should (hopefully) minimize the changes required to existing agent implementations.

Looks good to me!

[wiktor-k]

No worries, glad you were able to take a look! Frankly, I didn't want to proceed without your blessing but I've been writing a couple of examples and it all becomes harder and harder to keep in sync 😅

(I'll rebase, clean and merge early next week)