search

CloudSoda / go-smb2

Client implementation of the SMB 2 & 3 protocols
BSD 2-Clause "Simplified" License
13 stars 5 forks source link

DFS support #21

Closed tooptoop4 closed 3 weeks ago

tooptoop4 commented 9 months ago

Fixes https://github.com/CloudSoda/go-smb2/issues/20

see https://github.com/hirochachacha/go-smb2/pull/79

credit to @ashutosh-bhole-druva https://github.com/hirochachacha/go-smb2/pull/79

tooptoop4 commented 9 months ago

@arashpayan can u merge?

b-turchyn commented 9 months ago

Thanks for providing the missing file.

A few points:

  1. I've hit some defects in the code from my original testing, namely that Share doesn't have mapWriterLock and dfsTargetList populated when mounting a share. I've got this fixed locally and successfully traversed one DFS link. Which leads into...
  2. Something I've noticed while testing this is if the DFS link is mid-way through the network share, this implementation won't detect it. Rather, it's the responsibility of the user of this new functionality to iterate through each step of the path, see if there's a link present, and traverse it to go to the next server. i.e. if the user wants to mount \\example.com\foo\bar\baz and bar is the DFS link, this current implementation is clunky. I think there's opportunities to make this nicer. Some sort of helper to be able to access the "true" location across multiple directory traversals would be a welcome addition.

I don't want to downplay the value that this will provide. It's still better than no DFS support, and that's great. But usage is based around the assumptions that:

  1. The developer knows which specific shares are the DFS links
  2. The developer is willing to be responsible for handle the jumping between servers to get to the final target

Maybe the original developer anticipated some of this. Not sure! It's more likely that I'm stoopid and am missing something.

Others are more than welcome to get into this and kick the tires. I have no affiliation to this project; I'm just interested in this functionality for my own needs, and my Golang and SMB knowledge are weak enough that I wouldn't consider my approval to carry any weight. All I'll really be able to do is give an "it works for my use case" thumbs-up.

arashpayan commented 9 months ago

I intend to review it and bring it in. I'll be working on/with this package next week, so I'll have hours allocated to review this then. In the meantime, I'd appreciate any feedback from others similar to what @b-turchyn provided. I'm a complete novice when it comes to DFS so it'll take me some time to get up to speed, and there's a good chance I'll still overlook something.

arashpayan commented 5 months ago

Don't worry. I haven't forgotten about this PR. I've recently been working on adding support for getting and setting security descriptors. Once I bring those changes into main I can focus on DFS.

cyberb commented 3 months ago

sorry is it possible to merge this?

tooptoop4 commented 3 weeks ago

stale