MySQL Bugs: #43332: Avoidable NullReferenceException thrown in NativeDriver.SendFileToServer()

Bug #43332	Avoidable NullReferenceException thrown in NativeDriver.SendFileToServer()
Submitted:	3 Mar 15:53	Modified:	5 Mar 12:52
Reporter:	Yvan Rodrigues
Status:	Closed
Category:	Connector/Net	Severity:	S3 (Non-critical)
Version:	5.2.5	OS:	Any
Assigned to:	Reggie Burnett	Target Version:
Tags:	LOAD DATA LOCAL INFILE, MySqlBulkLoader, load, SendFileToServer, FileStream, null, close, Dispose
Triage:	D2 (Serious)

Description:
When using MySqlBulkLoader.Load(), the text file is opened by
NativeDriver.SendFileToServer. If it encounters a problem opening the file as a stream,
an Exception is caught. An attempt clean up resources is made in the finally{} clause by
calling fs.Close() but since the stream never successfully opened, this is an attempt to
execute a method of a null reference.

How to repeat:
EXAMPLE OF HOW TO USE BULK LOADER:

                MySql.Data.MySqlClient.MySqlBulkLoader bulk = new
MySql.Data.MySqlClient.MySqlBulkLoader(MySqlDataAccess.NewConnection());
                bulk.LineTerminator = "\n";
                bulk.FieldTerminator = "\t";
                bulk.Local = true;
                bulk.Priority = MySqlBulkLoaderPriority.Low;
                bulk.TableName = "System_ActivityLog";
                bulk.FileName = "myfile.txt";
                bulk.ConflictOption = MySqlBulkLoaderConflictOption.Replace;

                string[] columns = new string[]
                {
                    "EventId",
                    "UserLogin", 
                    "Type", 
                    "Verbose", 
                    "Importance",
                    "TimeStamp",
                    "ClientIP", 
                    "HostName", 
                };
                bulk.Columns.AddRange(columns);
                bulk.Load();

IN THE ABOVE SCENARIO, IF THE FILE CANNOT BE OPENED, FOR EXAMPLE IF ANOTHER THREAD HAS
THE FILE LOCKED, A STACK TRACE LIKE THIS WILL HAPPEN:

Suggested fix:
REPLACE THIS:

        /// <summary>
        /// Sends the specified file to the server. 
        /// This supports the LOAD DATA LOCAL INFILE
        /// </summary>
        /// <param name="filename"></param>
        private void SendFileToServer(string filename)
        {
            byte[] buffer = new byte[8196];
            FileStream fs = null;

            long len = 0;
            try
            {
                fs = new FileStream(filename, FileMode.Open);
                len = fs.Length;
                while (len > 0)
                {
                    int count = fs.Read(buffer, 4, (int)(len > 8192 ? 8192 : len));
                    stream.SendEntirePacketDirectly(buffer, count);
                    len -= count;
                }
                stream.SendEntirePacketDirectly(buffer, 0);
            }
            catch (Exception ex)
            {
                throw new MySqlException("Error during LOAD DATA LOCAL INFILE", ex);
            }
            finally
            {
                fs.Close();
            }
        }

WITH THIS:

        /// <summary>
        /// Sends the specified file to the server. 
        /// This supports the LOAD DATA LOCAL INFILE
        /// </summary>
        /// <param name="filename"></param>
        private void SendFileToServer(string filename)
        {
            byte[] buffer = new byte[8196];
            FileStream fs = null;

            long len = 0;
            try
            {
                using (fs = new FileStream(filename, FileMode.Open))
                {
                    len = fs.Length;
                    while (len > 0)
                    {
                        int count = fs.Read(buffer, 4, (int)(len > 8192 ? 8192 : len));
                        stream.SendEntirePacketDirectly(buffer, count);
                        len -= count;
                    }
                    stream.SendEntirePacketDirectly(buffer, 0);
                }
            }
            catch (Exception ex)
            {
                throw new MySqlException("Error during LOAD DATA LOCAL INFILE", ex);
            }
        }

When the code goes out of scope of the using() {} section, FileStream.Dispose() will be
called by the garbage collector and the stream will be closed. (See Stream.Close() in the
.NET platform documentation.)

Hi Yves and thanks for great report.

Verified as described using latest sources. In this case (file does not exist for
example) by the time we get into NativeDriver.cs LN:593
      finally
      {
          fs.Close();
      }

file stream is null and exception is thrown. We should add check on fs before calling
fs.close().

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/68280

fixed in 5.2.6

An entry was added to the 5.2.6 changelog:

When using MySqlBulkLoader.Load(), the text file is opened by
NativeDriver.SendFileToServer. If it encountered a problem opening the file as a stream,
an exception was generated and caught. An attempt to clean up resources was then made in
the finally{} clause by calling fs.Close(), but since the stream was never successfully
opened, this was an attempt to execute a method of a null reference.