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

sftp.rename doesn't behave like OpenSSH's rename #1012

Closed
mflage opened this issue Jul 5, 2017 · 6 comments
Closed

sftp.rename doesn't behave like OpenSSH's rename #1012

mflage opened this issue Jul 5, 2017 · 6 comments

Comments

@mflage
Copy link

@mflage mflage commented Jul 5, 2017

I'm not sure if this is a bug or a feature, but when looking at the implementation of the .rename function for the sftp module in Paramiko this doesn't trigger MOVED_FROM and MOVED_TO, instead it triggers CREATE and DELETE events. Is this normal? What is it that paramiko does differently when renaming a file that causes these two events to be created instead of MOVED_FROM and MOVED_TO?

I have tested this with version 2.2.1 of Paramiko. This is the output of 'inotifywait -r -m /tmp/test' which illustrates the difference (the file "test" is uploaded and then renamed to "testfile"):

When using sftp from OpenSSH:

/tmp/test/ CREATE test
/tmp/test/ OPEN test
/tmp/test/ MODIFY test
/tmp/test/ CLOSE_WRITE,CLOSE test
/tmp/test/ MOVED_FROM test
/tmp/test/ MOVED_TO testfile

When using paramiko:

/tmp/test/ CREATE test
/tmp/test/ OPEN test
/tmp/test/ MODIFY test
/tmp/test/ CLOSE_WRITE,CLOSE test
/tmp/test/ CREATE testfile
/tmp/test/ DELETE test
@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jul 5, 2017

Excellent question, I don't know off the top of my head what exact protocol messages the SFTP modules use, but it would not surprise me if their initial implementation was slightly off-spec (or if the spec has changed since 2004-ish).

Don't have time to deep-dive right now but I can say that if we find you're right and Paramiko's not doing quite the right thing, I'd be open to changing it (probably in a backwards incompat release in case anyone's relying on quirks of the current approach.)

Thanks for the report!

@mflage
Copy link
Author

@mflage mflage commented Jul 6, 2017

Heh! I just found that the method posix_rename, which was recently added, does this correctly :)

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jul 6, 2017

Sounds like someone already ran into the "oops changing this wouldn't be backwards compat", I guess. Does that basically solve your problem for now? If so I might just mutate this into "replace rename with the behavior of posix_rename in 3.0", assuming the latter behavior is actually the default in OpenSSH.

@mflage
Copy link
Author

@mflage mflage commented Jul 7, 2017

I'm going to be running a test with the customer the following Tuesday, so I'll let you know 100% then if it works for us. But it looks promising, since the inotify events are now the expected ones.

@mflage
Copy link
Author

@mflage mflage commented Jul 12, 2017

Hi!

I can confirm that posix_rename behaves correctly (like OpenSSH's sftp implementation does). So you can probably go ahead and close this ticket if you want.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jul 13, 2017

Thanks for the update @mflage ! I'll spin off some new action items (ugh, business-speak) from this.

@bitprophet bitprophet closed this Jul 13, 2017
bitprophet added a commit that referenced this issue Jul 13, 2017
dkhapun pushed a commit to cyberx-labs/paramiko that referenced this issue Jun 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants