FTP: partly revert eeb7c12807

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 eeb7c12807 shipped in 8.10.0

Reported-by: finkjsc on github
Fixes #14873
Closes #14875

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);

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;

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. */

tests/FILEFORMAT.md

@@ -694,11 +694,14 @@ content
 
 ### `<stripfile4>`
 
-### `<upload [crlf="yes"]>`
+### `<upload [crlf="yes"] [nonewline="yes"]>`
 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.
+
 ### `<valgrind>`
 disable - disables the valgrind log check for this test

tests/data/test475

@@ -16,8 +16,12 @@ ftp
 <name>
 FTP PASV upload ASCII file
 </name>
-<file name="%LOGDIR/test%TESTNUMBER.txt">
+<file name="%LOGDIR/test%TESTNUMBER.txt" nonewline="yes">
+%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
 </file>
 <command>
 "ftp://%HOSTIP:%FTPPORT/%TESTNUMBER;type=a" -T %LOGDIR/test%TESTNUMBER.txt
@@ -29,7 +33,7 @@ FTP PASV upload ASCII file
 <strip>
 QUIT
 </strip>
-<upload crlf="yes">
+<upload crlf="yes" nonewline="yes">
 %repeat[1750 x a line of text used for verifying this !%0a]%
 </upload>
 <protocol>

tests/data/test476

@@ -16,8 +16,8 @@ ftp
 <name>
 FTP PASV upload ASCII file already using CRLF
 </name>
-<file name="%LOGDIR/test%TESTNUMBER.txt" crlf="yes">
-%repeat[1750 x a line of text used for verifying this !%0a]%
+<file name="%LOGDIR/test%TESTNUMBER.txt" nonewline="yes">
+%repeat[1750 x a line of text used for verifying this !%0d%0a]%
 </file>
 <command>
 "ftp://%HOSTIP:%FTPPORT/%TESTNUMBER;type=a" -T %LOGDIR/test%TESTNUMBER.txt
@@ -29,7 +29,7 @@ FTP PASV upload ASCII file already using CRLF
 <strip>
 QUIT
 </strip>
-<upload crlf="yes">
+<upload crlf="yes" nonewline="yes">
 %repeat[1750 x a line of text used for verifying this !%0a]%
 </upload>
 <protocol>

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) {