Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug Fixes #220

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Bug Fixes #220

wants to merge 1 commit into from

Conversation

olamiral
Copy link

@olamiral olamiral commented May 2, 2020

  • URL Handler: parseUrl was not parsing URLs correctly when deleting
    files recursively

  • Resource Locator Implementation: paths and names were concatenated
    without path separator in method resolveInContext

- URL Handler: parseUrl was not parsing URLs correctly when deleting
files recursively

- Resource Locator Implementation: paths and names were concatenated
without path separator in method resolveInContext
@mbechler
Copy link
Contributor

mbechler commented May 2, 2020

Can you describe under which circumstances these issues occur?

While this may be unexpected and quite inconvenient, jcifs has always required that URLs to directories are specified with a trailing slash by the user.

@olamiral
Copy link
Author

olamiral commented May 2, 2020

Sure!

The problem occurs when I try do delete a directory recursively: suppose you have the following directories:
smb://host/share/nonEmptyDir/test/
smb://host/share/nonEmptyDir/anotherNonEmptyDir/someFile.txt

When you try to delete smb://host/share/nonEmptyDir/, jcifs returns the following error message: Invalid operation for workgroups, servers or share.

This error occurs no matter if there is a trailing slash when getting the file for nonEmptyDir/

@olamiral
Copy link
Author

olamiral commented May 2, 2020

By the way, I've noticed that the modification done in SmbResourceLocatorImpl fixed the problem related with recursive directory deletion (my unit tests break without it, and works fine with that fix).

In the other hand, the modification done in the Smb Url Handler fixes an issue when listing files:
Suppose you have the following file: smb://host/share/someDir/test1.txt

After calling SmbFile("smb://host/share/someDir", context).listFiles(), it will return a vector containing one single file with the following path: smb://host/share/test1.txt (the expected path is smb://host/share/someDir/test1.txt"). After applying the fix to the Handler class, the SmbFile.listFiles() method works as expected.

@dumh
Copy link

dumh commented Jun 18, 2022

Why isn't it merged yet?? These fixes are so needed! SmbFile.list() and .delete() don't work as expected because of this.

As far as I can see, there is a couple open issues here that are probably closely related to the bug which olamiral has fixed in the PR:
#134
#252

@mbechler
Copy link
Contributor

Please watch your tone.

Note, that as I mentioned at least some of the described issue appears to be actually "documented" behavior (even though this is admittetly stupid). Changing that however I believe has some other implications which need further investigation, making this more complex than just merging the patch.

But, I have to admit I lost track of it, at least the recursive deletion issue sounds like a legtimate one. I'll try to look into it some time soon.

@dumh
Copy link

dumh commented Jun 19, 2022

I'm sorry for the double question marks, it was only to express my utter surprise for such a bug not being fixed yet, no offence meant really.. I understand and appreciate the hard work you do for the project, thank you!

I got your point about backward compatibility. Maybe introduce a way of selecting the behaviour, keeping the old one as a legacy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants