-
Notifications
You must be signed in to change notification settings - Fork 385
make SFTP extensions configurable #315
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
Conversation
c66a36f
to
c9868fd
Compare
@drakkan Thanks for the submission and I think it is a good idea. I'll be happy to merge it though there is one thing... Could you add a test or 2 covering the new functions? |
@eikenb, sorry for the delay. I added some basic test cases, please let me know if they are enough. |
@drakkan You don't have to apologize to me for delays... I'm more guilty than anyone on that front around here. @puellanivis Thanks for reviewing that. Very much appreciated. |
@drakkan Thanks for the tests and I just had one more thought (sorry)... I was looking it over again and wondered if it might not be better if we kept SSHExtensionPair private and made SetSFTPExtensions take a list of the names they wanted. Changing the signature to...
And calling it...
Seems like this would both keep the API as is and seems like it be easier to call. What do you think? |
I considered that as well, I was not sure how important the “Data” field of the extension pair was. If it’s just ”always the value 1” then it makes sense. |
@eikenb @puellanivis please let me know if there are any remaining fixes/improvements, thanks! |
It may not always be "1", but it will always be fixed... at least for Openssh extensions. The spec is very vague about what it is used for, only specifying that both fields (name, data) must be present though the data one may be blank. Openssh uses the data field as an incremental version number, and the later versions supersede the former. So it might be something other than "1", but it will still be a fixed setting. In other words, I don't have any plans on supporting multiple versions of a specific extension protocol. |
update pkg/sftp to a git revision that includes the needed patch pkg/sftp#315
This patch allow to configure the supported extensions