Problems after upgrade to 2.0.4.21

ts678 · July 8, 2019, 1:21am

Dealing with backends for implementing parallel uploads original post said this. I have no opinion myself:

I’ve started looking into a theory that the below on file transfers is doing an incomplete clone, therefore coming up with an FTPClient with CurrentDataType uninitialized, and therefore 0, and therefore ASCII:

github.com

robinrodricks/FluentFTP/blob/bd17a2d2e0f7f8ac4d76c0d047fdacc6b128e921/FluentFTP/Client/FtpClient_Connection.cs#L802


      
          
          		#endregion
          
          		#region Clone
          
          		/// <summary>
          		/// Clones the control connection for opening multiple data streams
          		/// </summary>
          		/// <returns>A new control connection with the same property settings as this one</returns>
          		/// <example><code source="..\Examples\CloneConnection.cs" lang="cs" /></example>
          		protected FtpClient CloneConnection() {
          			FtpClient conn = Create();
          
          			conn.m_isClone = true;
          
          			// configure new connection as clone of self
          			conn.InternetProtocolVersions = InternetProtocolVersions;
          			conn.SocketPollInterval = SocketPollInterval;
          			conn.StaleDataCheck = StaleDataCheck;
          			conn.EnableThreadSafeDataConnections = EnableThreadSafeDataConnections;
          			conn.Encoding = Encoding;

I’m not sure if I can patch this in the debugger (haven’t tried yet), and am not set up on Windows to build. Another way of testing if it’s the problem is to set EnableThreadSafeDataConnections to false if we dare:

github.com

duplicati/duplicati/blob/a1058203e7eef93d7abfb50be3d25ee4f98d0e9e/Duplicati/Library/Backend/AlternativeFTP/AlternativeFTPBackend.cs#L549


      
          if (this.Client == null) // Create connection if it doesn't exist yet

          {

              var ftpClient = new FtpClient

              {

                  Host = uri.Host,

                  Port = uri.Port == -1 ? 21 : uri.Port,

                  Credentials = _userInfo,

                  EncryptionMode = _encryptionMode,

                  DataConnectionType = _dataConnectionType,

                  SslProtocols = _sslProtocols,

                  EnableThreadSafeDataConnections = true, // Required to work properly but can result in up to 3 connections being used even when you expect just one..

              };

          

              ftpClient.ValidateCertificate += HandleValidateCertificate;

          

              this.Client = ftpClient;

          } // else reuse existing connection

          

          // Change working directory to the remote path

          // Do this every time to prevent issues when FtpClient silently reconnects after failure.

          var remotePath = uri.AbsolutePath.EndsWith("/", StringComparison.Ordinal) ? uri.AbsolutePath.Substring(0, uri.AbsolutePath.Length - 1) : uri.AbsolutePath;

Current data type not set when using EnableThreadSafeDataConnections #184 was a similar partial clone, and Fix download file data type bug. #202 added the DownloadDataType and UploadDataType, but it looks like those are for the high-level API whereas Duplicati downloads use the low-level API. The TYPE I before the SIZE might be a force-to-binary, and TYPE A after it might be a return to the incorrect CurrentDataType:

github.com

robinrodricks/FluentFTP/blob/bd17a2d2e0f7f8ac4d76c0d047fdacc6b128e921/FluentFTP/Client/FtpClient_Management.cs#L1891-L1893


      
          				if (savedDataType != FtpDataType.Binary) {
          					this.SetDataTypeInternal(savedDataType);
          				}

This needs a whole lot more testing (and I’d like to come up with a simpler test case if a FluentFTP issue gets filed, but once again I’m somewhat limited in C# build abilities…) but that’s the update I wanted to do.

EDIT:

Looking at the prior fix, and the current CloneConnection calls in FtpClient_LowLevel.cs, I notice there are SetDataType* calls done afterwards for an OpenReadAsync, OpenWrite, OpenWriteAsync, OpenAppend, and OpenAppendAsync, however in OpenRead (which is what Duplicati uses) the SetDataType precedes CloneConnection. Maybe the ordering fixes up the incomplete cloning, except for the case Duplicati uses?

EDIT 2:

Getting way too speculative, possibly attempted fix for #318 made the problem more visible by being more definite about setting the connection transfer type to what’s usually the right value, but in our case is wrong. That would be a good outcome, because downgrading to some version before this recent fix might help us.