From 7eda757d99f43cb76cd4ef98acfec792d8a30e75 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 12 Sep 2024 08:15:14 +0200 Subject: [PATCH] FTP: partly revert eeb7c1280742f5c8fa48a4340fc1e1a1a2c7075a Since ASCII transfers on FTP means sending CRLF line endings, we should still keep converting them to LF-only on platforms where text files typically do not use CRLF. This also DOES NOT convert existing CRLF line endings on ASCII uploads but only does stand-alone LF => CRLF. Regression from eeb7c1280742f5c8 shipped in 8.10.0 Reported-by: finkjsc on github Fixes #14873 Closes #14875 --- lib/ftp.c | 27 +++++++++++++++++---------- lib/sendf.c | 16 +++++++++++++--- lib/urldata.h | 6 ++++++ tests/FILEFORMAT.md | 5 ++++- tests/data/test475 | 8 ++++++-- tests/data/test476 | 6 +++--- tests/runtests.pl | 5 +++++ 7 files changed, 54 insertions(+), 19 deletions(-) diff --git a/lib/ftp.c b/lib/ftp.c index f9b7e089e..02477fd1d 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -327,6 +327,7 @@ static void freedirs(struct ftp_conn *ftpc) Curl_safefree(ftpc->newhost); } +#ifdef CURL_PREFER_LF_LINEENDS /*********************************************************************** * * Lineend Conversions @@ -415,6 +416,7 @@ static const struct Curl_cwtype ftp_cw_lc = { sizeof(struct ftp_cw_lc_ctx) }; +#endif /* CURL_PREFER_LF_LINEENDS */ /*********************************************************************** * * AcceptServerConnect() @@ -4138,22 +4140,27 @@ static CURLcode ftp_do(struct Curl_easy *data, bool *done) CURLcode result = CURLE_OK; struct connectdata *conn = data->conn; struct ftp_conn *ftpc = &conn->proto.ftpc; - /* FTP data may need conversion. */ - struct Curl_cwriter *ftp_lc_writer; *done = FALSE; /* default to false */ ftpc->wait_data_conn = FALSE; /* default to no such wait */ - result = Curl_cwriter_create(&ftp_lc_writer, data, &ftp_cw_lc, - CURL_CW_CONTENT_DECODE); - if(result) - return result; +#ifdef CURL_PREFER_LF_LINEENDS + { + /* FTP data may need conversion. */ + struct Curl_cwriter *ftp_lc_writer; - result = Curl_cwriter_add(data, ftp_lc_writer); - if(result) { - Curl_cwriter_free(data, ftp_lc_writer); - return result; + result = Curl_cwriter_create(&ftp_lc_writer, data, &ftp_cw_lc, + CURL_CW_CONTENT_DECODE); + if(result) + return result; + + result = Curl_cwriter_add(data, ftp_lc_writer); + if(result) { + Curl_cwriter_free(data, ftp_lc_writer); + return result; + } } +#endif /* CURL_PREFER_LF_LINEENDS */ if(data->state.wildcardmatch) { result = wc_statemach(data); diff --git a/lib/sendf.c b/lib/sendf.c index 791dff220..d95564e6b 100644 --- a/lib/sendf.c +++ b/lib/sendf.c @@ -949,6 +949,7 @@ struct cr_lc_ctx { struct bufq buf; BIT(read_eos); /* we read an EOS from the next reader */ BIT(eos); /* we have returned an EOS */ + BIT(prev_cr); /* the last byte was a CR */ }; static CURLcode cr_lc_init(struct Curl_easy *data, struct Curl_creader *reader) @@ -1005,10 +1006,15 @@ static CURLcode cr_lc_read(struct Curl_easy *data, goto out; } - /* at least one \n needs conversion to '\r\n', place into ctx->buf */ + /* at least one \n might need conversion to '\r\n', place into ctx->buf */ for(i = start = 0; i < nread; ++i) { - if(buf[i] != '\n') + /* if this byte is not an LF character, or if the preceding character is + a CR (meaning this already is a CRLF pair), go to next */ + if((buf[i] != '\n') || ctx->prev_cr) { + ctx->prev_cr = (buf[i] == '\r'); continue; + } + ctx->prev_cr = false; /* on a soft limit bufq, we do not need to check length */ result = Curl_bufq_cwrite(&ctx->buf, buf + start, i - start, &n); if(!result) @@ -1101,7 +1107,11 @@ static CURLcode do_init_reader_stack(struct Curl_easy *data, clen = r->crt->total_length(data, r); /* if we do not have 0 length init, and crlf conversion is wanted, * add the reader for it */ - if(clen && (data->set.crlf || data->state.prefer_ascii)) { + if(clen && (data->set.crlf +#ifdef CURL_PREFER_LF_LINEENDS + || data->state.prefer_ascii +#endif + )) { result = cr_lc_add(data); if(result) return result; diff --git a/lib/urldata.h b/lib/urldata.h index 3eb7b845e..22dceeed8 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -105,6 +105,12 @@ typedef unsigned int curl_prot_t; #define CURL_DEFAULT_USER "anonymous" #define CURL_DEFAULT_PASSWORD "ftp@example.com" +#if !defined(_WIN32) && !defined(MSDOS) && !defined(__EMX__) +/* do FTP line-end CRLF => LF conversions on platforms that prefer LF-only. It + also means: keep CRLF line endings on the CRLF platforms */ +#define CURL_PREFER_LF_LINEENDS +#endif + /* Convenience defines for checking protocols or their SSL based version. Each protocol handler should only ever have a single CURLPROTO_ in its protocol field. */ diff --git a/tests/FILEFORMAT.md b/tests/FILEFORMAT.md index cbf2ac339..b0c4de5ef 100644 --- a/tests/FILEFORMAT.md +++ b/tests/FILEFORMAT.md @@ -694,11 +694,14 @@ content ### `` -### `` +### `` the contents of the upload data curl should have sent `crlf=yes` forces *upload* newlines to become CRLF even if not written so in the source file. +`nonewline=yes` means that the last byte (the trailing newline character) +should be cut off from the upload data before comparing it. + ### `` disable - disables the valgrind log check for this test diff --git a/tests/data/test475 b/tests/data/test475 index b0e4c2ca4..75e0e6b93 100644 --- a/tests/data/test475 +++ b/tests/data/test475 @@ -16,8 +16,12 @@ ftp FTP PASV upload ASCII file - + +%if win32 +%repeat[1750 x a line of text used for verifying this !%0d%0a]% +%else %repeat[1750 x a line of text used for verifying this !%0a]% +%endif "ftp://%HOSTIP:%FTPPORT/%TESTNUMBER;type=a" -T %LOGDIR/test%TESTNUMBER.txt @@ -29,7 +33,7 @@ FTP PASV upload ASCII file QUIT - + %repeat[1750 x a line of text used for verifying this !%0a]% diff --git a/tests/data/test476 b/tests/data/test476 index 00aed7c44..2396d3eea 100644 --- a/tests/data/test476 +++ b/tests/data/test476 @@ -16,8 +16,8 @@ ftp FTP PASV upload ASCII file already using CRLF - -%repeat[1750 x a line of text used for verifying this !%0a]% + +%repeat[1750 x a line of text used for verifying this !%0d%0a]% "ftp://%HOSTIP:%FTPPORT/%TESTNUMBER;type=a" -T %LOGDIR/test%TESTNUMBER.txt @@ -29,7 +29,7 @@ FTP PASV upload ASCII file already using CRLF QUIT - + %repeat[1750 x a line of text used for verifying this !%0a]% diff --git a/tests/runtests.pl b/tests/runtests.pl index 60482743d..c9854a600 100755 --- a/tests/runtests.pl +++ b/tests/runtests.pl @@ -1497,6 +1497,11 @@ sub singletest_check { if($hash{'crlf'}) { subnewlines(1, \$_) for @upload; } + if($hash{'nonewline'}) { + # Yes, we must cut off the final newline from the final line + # of the upload data + chomp($upload[-1]); + } $res = compare($runnerid, $testnum, $testname, "upload", \@out, \@upload); if ($res) {