SMBLibrary
SMBLibrary copied to clipboard
Exposes client connection and logged in state to the public API
Currently, consumers of the client API's have to store and track the client state themselves when the client already contains this data. This pull request exposes these properties on the client interface and updates v1 & v2 of the SMB clients to expose their internal state via the interface.
Hi Kieran, Thanks for the pull request, Given that you get a clean indication during the Connect and Login phase whether those operations were successful, it's not clear to me where this information is going to be useful, could you please shed light on this? in which scenario those properties are helpful?
Hi Kieran, Thanks for the pull request, Given that you get a clean indication during the Connect and Login phase whether those operations were successful, it's not clear to me where this information is going to be useful, could you please shed light on this? in which scenario those properties are helpful?
Well this is my point, you're making the user track the state of the client outside of the client which is:
- Redundant as the state already persists within the client
- Leads to bug prone code as users may miss certain state changes, or the clients state might change without calling user code and now, the client's internal state and the user tracked state no longer align.
Take this example below, what happens if there's a socket disconnect because of invalid packets or a network drop? The user tracked state of the client is no longer valid. However, if you were to use the clients tracked state and there is a socket disconnect, the socket disconnect is caught within the client and the internal state is updated.
(This is just sudo, the method names may differ in the actual API)
private bool isConnected = false;
private bool isLoggedIn = false;
private SMB2Client _client = new SMB2Client();
public void RenderMyUI()
{
if(!isConnected)
{
// Render connection form UI.
}
else if (isConnected && !isLoggedIn)
{
// Render login form UI
}
else if (isConnected && IsLoggedIn)
{
// Render file browser
}
}
public void Connect(string hostname)
{
var state = _client.Connect(hostname);
if (state == NT_SUCCESS)
{
isConnected = true;
}
}
Its the same reason System.Net.Sockets.Socket
has the public property Connected
https://docs.microsoft.com/en-us/dotnet/api/system.net.sockets.socket.connected?view=net-6.0
@TalAloni Can I get an update on this please?
Hi Kieran, I apologize, I've taken a few responsibilities and I can't seem to find the time to give thought to this non-urgent request. I hope I'll find the time in the coming weeks.
Hi Kieran, I apologize, I've taken a few responsibilities and I can't seem to find the time to give thought to this non-urgent request. I hope I'll find the time in the coming weeks.
Ok thanks, appreciate it.
Hi Kieran, I've exposed IsConnected given that indeed this property can be changed by the server and may be useful to have available. Regarding IsLoggedIn - I'm not sure I see as much value in that given that the client is in complete control of the login which should immediately follow the Connect phase (i.e. the server cannot Logoff a user - only to disconnect the connection). I hope this helps. thank you.
@TalAloni Looks like something is wrong with the 1.4.9 release, your changes don't appear to be in that release binary or source code.
These changes were merged after the 1.4.9 release (I've remembered about them after already releasing 1.4.9).