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

[ftp] Don't append slash to NLST in exists() #1438

Merged

Conversation

th3hamm0r
Copy link
Contributor

We're using multiple different thirdparty ftp servers and we've now stumbled over a case, where the .exists(name) call did not work correctly.

In this case, the location looked like ftp://username:[email protected]:21/, so it CWDs to the root. In the root there were multiple files like 20240724.txt, 20240723.txt,...

When I used storage.exists("20240724.txt") it returned False (note, allow_overwrite was False). The problem with this setup is, that this exists-call resulted in the call of self._connection.nlst("/") (which is also not correct, if a different path/CWD is used), which returned a list with items like ["/20240724.txt", "/20240723.txt"], so all items have a "/" prepended, therefore the actual name "20240724.txt" cannot be found. This is not the case, when self._connection.nlst("") is used.

I've tested my change (where no "/" gets added) with other setups (e.g. other CWD and also with the changes of #1433 with no CWD at all), and in all cases my fix works without issues. Therefore I'm hoping we can merge this.

With this change the code is also consistent with similar usages, like .size(), where no slash gets added either.

@jschneier
Copy link
Owner

Can you change the message to describe the fix rather than the change.

@jschneier jschneier merged commit 9ea35e1 into jschneier:master Jul 27, 2024
19 checks passed
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.

2 participants