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

Various fixes #29

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Various fixes #29

wants to merge 11 commits into from

Conversation

adiroiban
Copy link

I gave this code a try and here is a branch with quick and dirty fixes.

channel.recv() method is (ab)used in the code and already receive data is not buffered to be used in next phase/step of SCP protocol.

In the same time, when remote scp command fails, the error is not converted into a SCPException.

I have just sent to code, in case someone find it useful.... it does not contain any tests.

Many thanks for the work on this module!

Cheers

@jbardin
Copy link
Owner

jbardin commented May 19, 2014

Thanks. These look good, and I'll go over them as soon as I can.

@adiroiban
Copy link
Author

No hurry. For now, I am just trying scp.py ... in case I will stick with it I will need to write some tests as the patch is just a dirty hack.

@jbardin
Copy link
Owner

jbardin commented Mar 2, 2015

Hi, sorry this got lost in the py3k fray.

Are these patches something you're still using?
If so, would you mind trying to rebase them, against master, and seeing if you can get a failing->passing testcase for the changes?

If not, I'll close this work on these possible issues in a new patch.

Thanks!

@adiroiban
Copy link
Author

Hi. I am still using the changes as part of automated functional test suite.

I will try to find time to update the code and add tests.

Thanks!

@adiroiban
Copy link
Author

Hi.. i did an intial merge with master but my branch is broken.

I have created this patch while working on a custom SCP implementation so many of the errors are hard to reproduce using the OpenSSH server as they are only triggered by custom SSH servers (ex one which does not allow exec subsystem) or which don't behave as the original scp command.
... so I found it hard to write automated test using the current code. It will need a custom SSH server with a custom server side SCP implementation.

While trying to understand how I should set up the test environment I tried to improve the dev documenation...


Can you please make a new release and maybe also keep a record of public changes in a release-notes / changelog file? ... and create a tag for each release :)


btw I run latest scp.py on Linux/Windows/OSX and all looks good. So I think that you have already included in master the important bits of unicode path handling and empty files

Thanks!

@jbardin
Copy link
Owner

jbardin commented Mar 4, 2015

Ah yes, that explains why I couldn't replicate the problems you had listed, this was created by using openssh's scp as a reference.

Does your ssh/scp implementation interoperate correctly with openssh's?

For testing, I would use a fake scp server that sends back pre-scripted responses which are known to break the current implementation.

Thanks!

@adiroiban
Copy link
Author

I did not had time to update the tests..

I have pushed a new commit in which instead for failing if command does not end with \n it continue to read more :) ... the current fix is still naive

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

Successfully merging this pull request may close these issues.

3 participants