ssh: add ability to enable compression (for SCP/SFTP) #1735

vszakats · 2017-08-05T10:10:19Z

Add the ability to enable compression for SCP/SFTP.

The required low-level logic was already available as part of libssh2 (via LIBSSH2_FLAG_COMPRESS libssh2_session_flag() option.)

This patch adds the new libcurl option CURLOPT_SSH_COMPRESSION (boolean) and the new curl command-line option --compressed-ssh to request this libssh2 feature. To have compression enabled, it is required that the SSH server supports a (zlib) compatible compression method and that libssh2 was built with zlib support enabled.

Ref: #1732

Use case: When transferring well-compressible (f.e. plaintext) data which is either large and/or the bandwidth is relatively low. For small transfers and/or fast connections, it may even result in slightly worse performance. See also: https://askubuntu.com/questions/6770/does-ssh-use-any-compression

mention-bot · 2017-08-05T10:10:21Z

@vszakats, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Lekensteyn, @bagder and @yangtse to be potential reviewers.

vszakats · 2017-08-05T13:03:04Z

I've created the patch assuming it would make it only into 7.56.0. That is to say I'd be quite glad if it could be included in 7.55.0 next week. Given, of course that all eyes find it alright.

bagder · 2017-08-05T14:20:40Z

We only merge features when the feature window is open. That's 4 weeks following a release, so too late for 7.55.0

vszakats · 2017-08-05T14:22:15Z

Ok, it's clearly too late for that, thanks for clarifying!

* contrib/hbcurl/easy.c * contrib/hbcurl/hbcurl.ch + add tentative support for CURLOPT_SSH_COMPRESSION when built against upper than libcurl 7.55.0 curl/curl#1735

…skip] curl/curl#1735

vszakats · 2017-08-09T21:47:11Z

My real world tests were conclusively positive. Here are test binaries (for Windows) with this patch applied including a libssh2 library with zlib enabled: https://bintray.com/vszakats/generic/curl-test/7.55.1

(NOTE: these binaries are not for production use or distribution, their sole purpose is testing this specific feature on that platform.)

vszakats · 2017-08-15T11:39:25Z

Any suggestions or anything missing to have this approved?

bagder

Looks great!

jay · 2017-08-16T16:33:04Z

src/tool_operate.c

@@ -1091,6 +1091,10 @@ static CURLcode operate_do(struct GlobalConfig *global,
             to fail if we are not talking to who we think we should */
          my_setopt_str(curl, CURLOPT_SSH_HOST_PUBLIC_KEY_MD5,
                        config->hostpubmd5);
+
+          /* new in libcurl 7.56.0 */
+          my_setopt_str(curl, CURLOPT_SSH_COMPRESSION,


this should be my_setopt and you have to pass it a long like if(config->ssh_compression) my_setopt(curl, CURLOPT_SSH_COMPRESSION, 1L). Also the example should use 1L

Fixed both.

jay · 2017-08-16T16:42:32Z

tests/data/test642

+SFTP retrieval
+ </name>
+ <command>
+--key curl_client_key --pubkey curl_client_key.pub -u %USER: --compressed-ssh sftp://%HOSTIP:%SSHPORT%PWD/log/file642.txt --insecure


the test doesn't do anything to see if the compression is enabled, is that right?

That's right. It tests if download is still (transparently) working.

I've found no obvious way to verify if compression is indeed used in a libcurl transfer. It also requires support from the server-side, not sure if the test server supports it. For local, manual tests, I was timing the transfers and watching the process with a network monitor.

The required low-level logic was already available as part of `libssh2` (via `LIBSSH2_FLAG_COMPRESS` `libssh2_session_flag()`[1] option.) This patch adds the new `libcurl` option `CURLOPT_SSH_COMPRESSION` (boolean) and the new `curl` command-line option `--compressed-ssh` to request this `libssh2` feature. To have compression enabled, it is required that the SSH server supports a (zlib) compatible compression method and that `libssh2` was built with `zlib` support enabled. [1] https://www.libssh2.org/libssh2_session_flag.html Ref: #1732

vszakats · 2017-08-16T17:42:39Z

Squashed and rebased on current master.

vszakats · 2017-08-16T21:19:42Z

Slightly off, but the Travis CI queue may be eased a bit by enabling both 'Auto cancel' options on https://travis-ci.org/curl/curl/settings. This would drop any pending commits from the queue if a newer commit lands.

There is a similar option on AppVeyor CI, called 'Rolling builds', which could be enabled for a similar effect, but in this case, by default it will even cancel running builds on the same branch on any new commit (this can be disabled to match Travis CI.)

With these set, there would be less strain on the CI servers and the queue could finish faster.

What do you think about it?

coveralls · 2017-08-16T21:51:42Z

Coverage increased (+0.01%) to 73.158% when pulling fa7b3a1 on vszakats:ssh_compress into 870d849 on curl:master.

jay · 2017-08-17T07:39:19Z

Thanks. I landed this with a slight change so that the error message will be shown even if libssh2_session_flag isn't available. Also note infof should end in a linefeed

  if(data->set.ssh_compression) {
#if LIBSSH2_VERSION_NUM >= 0x010208
    if(libssh2_session_flag(ssh->ssh_session, LIBSSH2_FLAG_COMPRESS, 1) < 0)
#endif
      infof(data, "Failed to enable compression for ssh session\n");
  }

vszakats · 2017-08-17T07:43:23Z

Good idea, thank you @jay! (Noted the infof() issue – what happened is that I copied that line from the preceding failf() which doesn't need the \n)

vszakats mentioned this pull request Aug 5, 2017

Support for compression in SCP/SFTP via libssh2 #1732

Closed

vszakats added the SCP/SFTP label Aug 5, 2017

vszakats changed the title ~~add ability to enable SSH compression~~ add ability to enable SSH compression (for SCP/SFTP) Aug 5, 2017

vszakats changed the title ~~add ability to enable SSH compression (for SCP/SFTP)~~ ssh: add ability to enable compression (for SCP/SFTP) Aug 5, 2017

vszakats requested a review from jay August 5, 2017 10:29

curl deleted a comment from coveralls Aug 5, 2017

vszakats added a commit to curl/curl-for-win that referenced this pull request Aug 9, 2017

add patch to test branches for tentative SSH compression feature [ci …

f331e9c

…skip] curl/curl#1735

vszakats removed the request for review from jay August 9, 2017 11:31

bagder approved these changes Aug 15, 2017

View reviewed changes

jay reviewed Aug 16, 2017

View reviewed changes

curl deleted a comment from coveralls Aug 16, 2017

jay closed this in b7b4dc0 Aug 17, 2017

vszakats deleted the ssh_compress branch August 17, 2017 07:43

lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ssh: add ability to enable compression (for SCP/SFTP) #1735

ssh: add ability to enable compression (for SCP/SFTP) #1735

vszakats commented Aug 5, 2017 •

edited

mention-bot commented Aug 5, 2017

vszakats commented Aug 5, 2017

bagder commented Aug 5, 2017 •

edited by vszakats

vszakats commented Aug 5, 2017

vszakats commented Aug 9, 2017 •

edited

vszakats commented Aug 15, 2017

bagder left a comment

jay Aug 16, 2017

vszakats Aug 16, 2017

vszakats Aug 16, 2017 •

edited

jay Aug 16, 2017

vszakats Aug 16, 2017

vszakats Aug 16, 2017 •

edited

vszakats commented Aug 16, 2017 •

edited

vszakats commented Aug 16, 2017 •

edited

coveralls commented Aug 16, 2017

jay commented Aug 17, 2017 •

edited

vszakats commented Aug 17, 2017

ssh: add ability to enable compression (for SCP/SFTP) #1735

ssh: add ability to enable compression (for SCP/SFTP) #1735

Conversation

vszakats commented Aug 5, 2017 • edited

mention-bot commented Aug 5, 2017

vszakats commented Aug 5, 2017

bagder commented Aug 5, 2017 • edited by vszakats

vszakats commented Aug 5, 2017

vszakats commented Aug 9, 2017 • edited

vszakats commented Aug 15, 2017

bagder left a comment

Choose a reason for hiding this comment

jay Aug 16, 2017

Choose a reason for hiding this comment

vszakats Aug 16, 2017

Choose a reason for hiding this comment

vszakats Aug 16, 2017 • edited

Choose a reason for hiding this comment

jay Aug 16, 2017

Choose a reason for hiding this comment

vszakats Aug 16, 2017

Choose a reason for hiding this comment

vszakats Aug 16, 2017 • edited

Choose a reason for hiding this comment

vszakats commented Aug 16, 2017 • edited

vszakats commented Aug 16, 2017 • edited

coveralls commented Aug 16, 2017

jay commented Aug 17, 2017 • edited

vszakats commented Aug 17, 2017

vszakats commented Aug 5, 2017 •

edited

bagder commented Aug 5, 2017 •

edited by vszakats

vszakats commented Aug 9, 2017 •

edited

vszakats Aug 16, 2017 •

edited

vszakats Aug 16, 2017 •

edited

vszakats commented Aug 16, 2017 •

edited

vszakats commented Aug 16, 2017 •

edited

jay commented Aug 17, 2017 •

edited