From c545e10fa75c04b4ded093573a0c15077937c51b Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 11 Nov 2025 09:42:16 +0100 Subject: [PATCH] sftp: fix range downloads in both SSH backends When asking for the last N bytes of a file, and that size was larger than the file size, it would miss the first byte due to a logic error. The fixed range parser is now made a common function in the file now renamed to vssh.c (from curl_path.c) - used by both backends. Unit test 2605 verifies the parser. Reported-by: Stanislav Fort (Aisle Research) Closes #19460 --- lib/Makefile.inc | 4 +- lib/vssh/libssh.c | 45 ++---------- lib/vssh/libssh2.c | 43 ++---------- lib/vssh/ssh.h | 2 +- lib/vssh/{curl_path.c => vssh.c} | 49 +++++++++++++- lib/vssh/{curl_path.h => vssh.h} | 4 ++ tests/data/Makefile.am | 2 +- tests/data/test2605 | 20 ++++++ tests/data/test637 | 2 +- tests/unit/Makefile.inc | 2 +- tests/unit/unit2604.c | 2 +- tests/unit/unit2605.c | 113 +++++++++++++++++++++++++++++++ 12 files changed, 204 insertions(+), 84 deletions(-) rename lib/vssh/{curl_path.c => vssh.c} (80%) rename lib/vssh/{curl_path.h => vssh.h} (88%) create mode 100644 tests/data/test2605 create mode 100644 tests/unit/unit2605.c diff --git a/lib/Makefile.inc b/lib/Makefile.inc index 8b506febcf..4bdf293f89 100644 --- a/lib/Makefile.inc +++ b/lib/Makefile.inc @@ -133,10 +133,10 @@ LIB_VQUIC_HFILES = \ LIB_VSSH_CFILES = \ vssh/libssh.c \ vssh/libssh2.c \ - vssh/curl_path.c + vssh/vssh.c LIB_VSSH_HFILES = \ - vssh/curl_path.h \ + vssh/vssh.h \ vssh/ssh.h LIB_CFILES = \ diff --git a/lib/vssh/libssh.c b/lib/vssh/libssh.c index f5de9f3e84..781c5dafc3 100644 --- a/lib/vssh/libssh.c +++ b/lib/vssh/libssh.c @@ -64,7 +64,7 @@ #include "../multiif.h" #include "../select.h" #include "../curlx/warnless.h" -#include "curl_path.h" +#include "vssh.h" #ifdef HAVE_UNISTD_H #include @@ -1329,44 +1329,11 @@ static int myssh_in_SFTP_DOWNLOAD_STAT(struct Curl_easy *data, return myssh_to_ERROR(data, sshc, CURLE_BAD_DOWNLOAD_RESUME); } if(data->state.use_range) { - curl_off_t from, to; - const char *p = data->state.range; - int from_t, to_t; - - from_t = curlx_str_number(&p, &from, CURL_OFF_T_MAX); - if(from_t == STRE_OVERFLOW) - return myssh_to_ERROR(data, sshc, CURLE_RANGE_ERROR); - - curlx_str_passblanks(&p); - (void)curlx_str_single(&p, '-'); - - to_t = curlx_str_numblanks(&p, &to); - if(to_t == STRE_OVERFLOW) - return myssh_to_ERROR(data, sshc, CURLE_RANGE_ERROR); - - if((to_t == STRE_NO_NUM) || (to >= size)) { - to = size - 1; - } - - if(from_t == STRE_NO_NUM) { - /* from is relative to end of file */ - from = size - to; - to = size - 1; - } - if(from > size) { - failf(data, "Offset (%" FMT_OFF_T ") was beyond file size (%" - FMT_OFF_T ")", from, size); - return myssh_to_ERROR(data, sshc, CURLE_BAD_DOWNLOAD_RESUME); - } - if(from > to) { - from = to; - size = 0; - } - else { - if((to - from) == CURL_OFF_T_MAX) - return myssh_to_ERROR(data, sshc, CURLE_RANGE_ERROR); - size = to - from + 1; - } + curl_off_t from; + CURLcode result = Curl_ssh_range(data, data->state.range, size, + &from, &size); + if(result) + return myssh_to_ERROR(data, sshc, result); rc = sftp_seek64(sshc->sftp_file, from); if(rc) diff --git a/lib/vssh/libssh2.c b/lib/vssh/libssh2.c index ee82396753..e83bc682b9 100644 --- a/lib/vssh/libssh2.c +++ b/lib/vssh/libssh2.c @@ -63,7 +63,7 @@ #include "../select.h" #include "../curlx/fopen.h" #include "../curlx/warnless.h" -#include "curl_path.h" +#include "vssh.h" #include "../curlx/strparse.h" #include "../curlx/base64.h" /* for base64 encoding/decoding */ @@ -1434,42 +1434,11 @@ sftp_download_stat(struct Curl_easy *data, return CURLE_BAD_DOWNLOAD_RESUME; } if(data->state.use_range) { - curl_off_t from, to; - const char *p = data->state.range; - int to_t, from_t; - - from_t = curlx_str_number(&p, &from, CURL_OFF_T_MAX); - if(from_t == STRE_OVERFLOW) - return CURLE_RANGE_ERROR; - curlx_str_passblanks(&p); - (void)curlx_str_single(&p, '-'); - - to_t = curlx_str_numblanks(&p, &to); - if(to_t == STRE_OVERFLOW) - return CURLE_RANGE_ERROR; - if((to_t == STRE_NO_NUM) /* no "to" value given */ - || (to >= size)) { - to = size - 1; - } - if(from_t) { - /* from is relative to end of file */ - from = size - to; - to = size - 1; - } - if(from > size) { - failf(data, "Offset (%" FMT_OFF_T ") was beyond file size (%" - FMT_OFF_T ")", from, (curl_off_t)attrs.filesize); - return CURLE_BAD_DOWNLOAD_RESUME; - } - if(from > to) { - from = to; - size = 0; - } - else { - if((to - from) == CURL_OFF_T_MAX) - return CURLE_RANGE_ERROR; - size = to - from + 1; - } + curl_off_t from; + CURLcode result = Curl_ssh_range(data, data->state.range, size, + &from, &size); + if(result) + return result; libssh2_sftp_seek64(sshc->sftp_handle, (libssh2_uint64_t)from); } diff --git a/lib/vssh/ssh.h b/lib/vssh/ssh.h index e09111e63f..9d03b52f40 100644 --- a/lib/vssh/ssh.h +++ b/lib/vssh/ssh.h @@ -36,7 +36,7 @@ #include #endif -#include "curl_path.h" +#include "vssh.h" /* meta key for storing protocol meta at easy handle */ #define CURL_META_SSH_EASY "meta:proto:ssh:easy" diff --git a/lib/vssh/curl_path.c b/lib/vssh/vssh.c similarity index 80% rename from lib/vssh/curl_path.c rename to lib/vssh/vssh.c index a6d572520f..e33386a16a 100644 --- a/lib/vssh/curl_path.c +++ b/lib/vssh/vssh.c @@ -26,9 +26,10 @@ #ifdef USE_SSH -#include "curl_path.h" +#include "vssh.h" #include #include "../curlx/strparse.h" +#include "../curl_trc.h" #include "../curl_memory.h" #include "../escape.h" #include "../memdebug.h" @@ -196,4 +197,50 @@ fail: return CURLE_QUOTE_ERROR; } +CURLcode Curl_ssh_range(struct Curl_easy *data, + const char *p, curl_off_t filesize, + curl_off_t *startp, curl_off_t *sizep) +{ + curl_off_t from, to; + int to_t; + int from_t = curlx_str_number(&p, &from, CURL_OFF_T_MAX); + if(from_t == STRE_OVERFLOW) + return CURLE_RANGE_ERROR; + curlx_str_passblanks(&p); + (void)curlx_str_single(&p, '-'); + + to_t = curlx_str_numblanks(&p, &to); + if((to_t == STRE_OVERFLOW) || (to_t && from_t) || *p) + return CURLE_RANGE_ERROR; + + if(from_t) { + /* no start point given, set from relative to end of file */ + if(!to) + /* "-0" is not a fine range */ + return CURLE_RANGE_ERROR; + else if(to > filesize) + to = filesize; + from = filesize - to; + to = filesize - 1; + } + else if(from > filesize) { + failf(data, "Offset (%" FMT_OFF_T ") was beyond file size (%" + FMT_OFF_T ")", from, filesize); + return CURLE_RANGE_ERROR; + } + else if((to_t == STRE_NO_NUM) || (to >= filesize)) + to = filesize - 1; + + if(from > to) { + failf(data, "Bad range: start offset larger than end offset"); + return CURLE_RANGE_ERROR; + } + if((to - from) == CURL_OFF_T_MAX) + return CURLE_RANGE_ERROR; + + *startp = from; + *sizep = to - from + 1; + return CURLE_OK; +} + #endif /* if SSH is used */ diff --git a/lib/vssh/curl_path.h b/lib/vssh/vssh.h similarity index 88% rename from lib/vssh/curl_path.h rename to lib/vssh/vssh.h index 1c167f9660..1a4e597b15 100644 --- a/lib/vssh/curl_path.h +++ b/lib/vssh/vssh.h @@ -33,4 +33,8 @@ CURLcode Curl_getworkingpath(struct Curl_easy *data, char **path); CURLcode Curl_get_pathname(const char **cpp, char **path, const char *homedir); + +CURLcode Curl_ssh_range(struct Curl_easy *data, + const char *range, curl_off_t filesize, + curl_off_t *startp, curl_off_t *sizep); #endif /* HEADER_CURL_PATH_H */ diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index bbd97853a3..bdec674dff 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -265,7 +265,7 @@ test2400 test2401 test2402 test2403 test2404 test2405 test2406 \ \ test2500 test2501 test2502 test2503 \ \ -test2600 test2601 test2602 test2603 test2604 \ +test2600 test2601 test2602 test2603 test2604 test2605 \ \ test2700 test2701 test2702 test2703 test2704 test2705 test2706 test2707 \ test2708 test2709 test2710 test2711 test2712 test2713 test2714 test2715 \ diff --git a/tests/data/test2605 b/tests/data/test2605 new file mode 100644 index 0000000000..81b580b7b4 --- /dev/null +++ b/tests/data/test2605 @@ -0,0 +1,20 @@ + + + +unittest +sftp + + + +# +# Client-side + + +unittest +sftp + + +Curl_ssh_range unit test + + + diff --git a/tests/data/test637 b/tests/data/test637 index 83da596fed..3052948751 100644 --- a/tests/data/test637 +++ b/tests/data/test637 @@ -35,7 +35,7 @@ for ssh test # Verify data after the test has been "shot" -36 +33 diff --git a/tests/unit/Makefile.inc b/tests/unit/Makefile.inc index f64a1630c8..7027973d2f 100644 --- a/tests/unit/Makefile.inc +++ b/tests/unit/Makefile.inc @@ -40,6 +40,6 @@ TESTS_C = \ unit1650.c unit1651.c unit1652.c unit1653.c unit1654.c unit1655.c unit1656.c \ unit1657.c unit1658.c unit1660.c unit1661.c unit1663.c unit1664.c \ unit1979.c unit1980.c \ - unit2600.c unit2601.c unit2602.c unit2603.c unit2604.c \ + unit2600.c unit2601.c unit2602.c unit2603.c unit2604.c unit2605.c \ unit3200.c unit3205.c \ unit3211.c unit3212.c unit3213.c unit3214.c diff --git a/tests/unit/unit2604.c b/tests/unit/unit2604.c index 7cb82bbcf1..13f7e80d8a 100644 --- a/tests/unit/unit2604.c +++ b/tests/unit/unit2604.c @@ -22,7 +22,7 @@ * ***************************************************************************/ #include "unitcheck.h" -#include "vssh/curl_path.h" +#include "vssh/vssh.h" #include "memdebug.h" static CURLcode test_unit2604(const char *arg) diff --git a/tests/unit/unit2605.c b/tests/unit/unit2605.c new file mode 100644 index 0000000000..b94e87e78c --- /dev/null +++ b/tests/unit/unit2605.c @@ -0,0 +1,113 @@ +/*************************************************************************** + * _ _ ____ _ + * Project ___| | | | _ \| | + * / __| | | | |_) | | + * | (__| |_| | _ <| |___ + * \___|\___/|_| \_\_____| + * + * Copyright (C) Daniel Stenberg, , et al. + * + * This software is licensed as described in the file COPYING, which + * you should have received as part of this distribution. The terms + * are also available at https://curl.se/docs/copyright.html. + * + * You may opt to use, copy, modify, merge, publish, distribute and/or sell + * copies of the Software, and permit persons to whom the Software is + * furnished to do so, under the terms of the COPYING file. + * + * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY + * KIND, either express or implied. + * + * SPDX-License-Identifier: curl + * + ***************************************************************************/ +#include "unitcheck.h" +#include "vssh/vssh.h" +#include "memdebug.h" + +static CURLcode test_unit2605(const char *arg) +{ + UNITTEST_BEGIN_SIMPLE + +#ifdef USE_SSH + CURL *curl; + struct range { + const char *r; + curl_off_t filesize; + curl_off_t start; + curl_off_t size; + CURLcode result; + }; + + int i; + struct range list[] = { + { "0-9", 100, 0, 10, CURLE_OK}, + { "1-10", 100, 1, 10, CURLE_OK}, + { "222222-222222", 300000, 222222, 1, CURLE_OK}, + { "4294967296 - 4294967297", 4294967298, 4294967296, 2, CURLE_OK}, + { "-10", 100, 90, 10, CURLE_OK}, + { "-20", 100, 80, 20, CURLE_OK}, + { "-1", 100, 99, 1, CURLE_OK}, + { "-0", 100, 0, 0, CURLE_RANGE_ERROR}, + { "--2", 100, 0, 0, CURLE_RANGE_ERROR}, + { "-100", 100, 0, 100, CURLE_OK}, + { "-101", 100, 0, 100, CURLE_OK}, + { "-1000", 100, 0, 100, CURLE_OK}, + { "2-1000", 100, 2, 98, CURLE_OK}, + { ".2-3", 100, 0, 0, CURLE_RANGE_ERROR}, + { "+2-3", 100, 0, 0, CURLE_RANGE_ERROR}, + { "2 - 3", 100, 2, 2, CURLE_OK}, + { " 2 - 3", 100, 2, 2, CURLE_RANGE_ERROR}, /* no leading space */ + { "2 - 3 ", 100, 2, 2, CURLE_RANGE_ERROR}, /* no trailing space */ + { "3-2", 100, 0, 0, CURLE_RANGE_ERROR}, + { "2.-3", 100, 0, 0, CURLE_RANGE_ERROR}, + { "-3-2", 100, 0, 0, CURLE_RANGE_ERROR}, + { "101-102", 100, 0, 0, CURLE_RANGE_ERROR}, + { "0-", 100, 0, 100, CURLE_OK}, + { "1-", 100, 1, 99, CURLE_OK}, + { "99-", 100, 99, 1, CURLE_OK}, + { "100-", 100, 0, 0, CURLE_RANGE_ERROR}, + { NULL, 0, 0, 0, CURLE_OK } + }; + + curl_global_init(CURL_GLOBAL_ALL); + curl = curl_easy_init(); + if(curl) { + curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L); + + for(i = 0; list[i].r; i++) { + curl_off_t start; + curl_off_t size; + CURLcode result; + curl_mprintf("%u: '%s' (file size: %" FMT_OFF_T ")\n", + i, list[i].r, list[i].filesize); + result = Curl_ssh_range(curl, list[i].r, list[i].filesize, + &start, &size); + if(result != list[i].result) { + curl_mprintf("... returned %d\n", result); + unitfail++; + } + if(!result) { + if(start != list[i].start) { + curl_mprintf("... start (%" FMT_OFF_T ") was not %" FMT_OFF_T " \n", + start, list[i].start); + unitfail++; + } + if(size != list[i].size) { + curl_mprintf("... size (%" FMT_OFF_T ") was not %" FMT_OFF_T " \n", + size, list[i].size); + unitfail++; + } + } + } + curl_easy_cleanup(curl); + } + curl_global_cleanup(); + + if(!unitfail) + curl_mprintf("ok\n"); + +#endif + + UNITTEST_END_SIMPLE +} -- 2.47.3