MDEV-36269: improve error handling for source command

- refactor batch_readline_init to mysql.cc for proper error handling
- add unit test and move it to the end of main/mysql_client_test.test
to resolve embedded test failures
- remove unnecessary code and duplicates to clean up implementation
- redirect error message from stderr to stdout in .test file
- use labels to avoid code duplication
- handle windows check for block device
- ensure file failing to open in windows because being a directory
 is different from any other reason for clear error message

Author: MohamedM216

client/my_readline.h

@@ -34,9 +34,10 @@ typedef struct st_line_buffer
   bool truncated;
 } LINE_BUFFER;
 
-extern LINE_BUFFER *batch_readline_init(ulong max_size,FILE *file);
 extern LINE_BUFFER *batch_readline_command(LINE_BUFFER *buffer, char * str);
 extern char *batch_readline(LINE_BUFFER *buffer, bool binary_mode);
 extern void batch_readline_end(LINE_BUFFER *buffer);
+extern bool init_line_buffer(LINE_BUFFER *buffer, File file, ulong size,
+                             ulong max_size);
 
 #endif /* CLIENT_MY_READLINE_INCLUDED */

client/mysql.cc

@@ -1207,6 +1207,81 @@ inline int get_command_index(char cmd_char)
   return -1;
 }
 
+static LINE_BUFFER *batch_readline_init(ulong max_size, const char *path)
+{
+  LINE_BUFFER *line_buff;
+  File file;
+  MY_STAT input_file_stat;
+  char buff[FN_REFLEN + 512];
+
+  if (path)
+  {
+    if ((file= my_open(path, O_RDONLY | O_BINARY, MYF(0))) < 0)
+    {
+#ifdef _WIN32
+      if (my_errno == EACCES && my_stat(path, &input_file_stat, MYF(0)) &&
+          MY_S_ISDIR(input_file_stat.st_mode))
+        my_snprintf(buff, sizeof(buff), "Can't read from a directory '%.*s'",
+                    FN_REFLEN, path);
+      else
+#endif
+      my_snprintf(buff, sizeof(buff), "Failed to open file '%.*s', error: %d",
+                  FN_REFLEN, path, my_errno);
+      put_info(buff, INFO_ERROR, 0);
+      return 0;
+    }
+  }
+  else
+  {
+    file= my_fileno(stdin);
+  }
+
+  if (my_fstat(file, &input_file_stat, MYF(0)))
+  {
+    my_snprintf(buff, sizeof(buff), "Failed to stat file '%.*s', error: %d",
+                FN_REFLEN, path ? path : "stdin", my_errno);
+    goto err1;
+  }
+
+  if (MY_S_ISDIR(input_file_stat.st_mode))
+  {
+    my_snprintf(buff, sizeof(buff), "Can't read from a directory '%.*s'",
+                FN_REFLEN, path ? path : "stdin");
+    goto err1;
+  }
+
+#ifndef _WIN32
+  if (MY_S_ISBLK(input_file_stat.st_mode))
+  {
+    my_snprintf(buff, sizeof(buff), "Can't read from a block device '%.*s'",
+                FN_REFLEN, path ? path : "stdin");
+    goto err1;
+  }
+#endif
+
+  if (!(line_buff= (LINE_BUFFER*) my_malloc(PSI_NOT_INSTRUMENTED,
+                                            sizeof(*line_buff),
+                                            MYF(MY_WME | MY_ZEROFILL))))
+  {
+    goto err;
+  }
+
+  if (init_line_buffer(line_buff, file, IO_SIZE, max_size))
+  {
+    my_free(line_buff);
+    goto err;
+  }
+
+  return line_buff;
+
+err1:
+  put_info(buff, INFO_ERROR, 0);
+err:
+  if (path)
+    my_close(file, MYF(0));
+  return 0;
+}
+
 static int delimiter_index= -1;
 static int charset_index= -1;
 static int sandbox_index= -1;
@@ -1281,10 +1356,8 @@ int main(int argc,char *argv[])
   }
 
   if (status.batch && !status.line_buff &&
-      !(status.line_buff= batch_readline_init(MAX_BATCH_BUFFER_SIZE, stdin)))
+      !(status.line_buff= batch_readline_init(MAX_BATCH_BUFFER_SIZE, NULL)))
   {
-    put_info("Can't initialize batch_readline - may be the input source is "
-             "a directory or a block device.", INFO_ERROR, 0);
     free_defaults(defaults_argv);
     my_end(0);
     exit(1);
@@ -4682,7 +4755,6 @@ static int com_source(String *, char *line)
   LINE_BUFFER *line_buff;
   int error;
   STATUS old_status;
-  FILE *sql_file;
   my_bool save_ignore_errors;
 
   if (status.sandbox)
@@ -4702,18 +4774,10 @@ static int com_source(String *, char *line)
     end--;
   end[0]=0;
   unpack_filename(source_name,source_name);
-  /* open file name */
-  if (!(sql_file = my_fopen(source_name, O_RDONLY | O_BINARY,MYF(0))))
-  {
-    char buff[FN_REFLEN+60];
-    sprintf(buff,"Failed to open file '%s', error: %d", source_name,errno);
-    return put_info(buff, INFO_ERROR, 0);
-  }
 
-  if (!(line_buff= batch_readline_init(MAX_BATCH_BUFFER_SIZE, sql_file)))
+  if (!(line_buff= batch_readline_init(MAX_BATCH_BUFFER_SIZE, source_name)))
   {
-    my_fclose(sql_file,MYF(0));
-    return put_info("Can't initialize batch_readline", INFO_ERROR, 0);
+    return ignore_errors ? -1 : 1;
   }
 
   /* Save old status */
@@ -4732,7 +4796,7 @@ static int com_source(String *, char *line)
   ignore_errors= save_ignore_errors;
   status=old_status;				// Continue as before
   in_com_source= aborted= 0;
-  my_fclose(sql_file,MYF(0));
+  my_close(line_buff->file, MYF(0));
   batch_readline_end(line_buff);
   /*
     If we got an error during source operation, don't abort the client

client/readline.cc

@@ -23,38 +23,11 @@
 #include <my_dir.h>
 #include "my_readline.h"
 
-static bool init_line_buffer(LINE_BUFFER *buffer,File file,ulong size,
-			    ulong max_size);
 static bool init_line_buffer_from_string(LINE_BUFFER *buffer,char * str);
 static size_t fill_buffer(LINE_BUFFER *buffer);
 static char *intern_read_line(LINE_BUFFER *buffer, ulong *out_length);
 
 
-LINE_BUFFER *batch_readline_init(ulong max_size,FILE *file)
-{
-  LINE_BUFFER *line_buff;
-
-#ifndef _WIN32
-  MY_STAT input_file_stat;
-  if (my_fstat(fileno(file), &input_file_stat, MYF(MY_WME)) ||
-      MY_S_ISDIR(input_file_stat.st_mode) ||
-      MY_S_ISBLK(input_file_stat.st_mode))
-    return 0;
-#endif
-
-  if (!(line_buff=(LINE_BUFFER*)
-        my_malloc(PSI_NOT_INSTRUMENTED, sizeof(*line_buff),
-                  MYF(MY_WME | MY_ZEROFILL))))
-    return 0;
-  if (init_line_buffer(line_buff,my_fileno(file),IO_SIZE,max_size))
-  {
-    my_free(line_buff);
-    return 0;
-  }
-  return line_buff;
-}
-
-
 char *batch_readline(LINE_BUFFER *line_buff, bool binary_mode)
 {
   char *pos;
@@ -105,8 +78,7 @@ LINE_BUFFER *batch_readline_command(LINE_BUFFER *line_buff, char * str)
       Functions to handle buffered readings of lines from a stream
 ******************************************************************************/
 
-static bool
-init_line_buffer(LINE_BUFFER *buffer,File file,ulong size,ulong max_buffer)
+bool init_line_buffer(LINE_BUFFER *buffer,File file,ulong size,ulong max_buffer)
 {
   buffer->file=file;
   buffer->bufread=size;

mysql-test/main/mysql_client_test.result

@@ -262,3 +262,9 @@ SET @@global.collation_server= @save_collation_server;
 SET @@global.character_set_client= @save_character_set_client;
 SET @@global.collation_connection= @save_collation_connection;
 FOUND 1 /Aborted connection.*'u' host: '192.0.2.1' real ip: '(localhost|::1)'/ in mysqld.1.err
+#
+# MDEV-36269: Improve error handling for SOURCE command
+# Test that SOURCE on an existent directory returns a clear error
+#
+ERROR at line 1: Can't read from a directory '.'
+End of 10.11 tests

mysql-test/main/mysql_client_test.test

@@ -63,3 +63,13 @@ SET @@global.collation_connection= @save_collation_connection;
 let SEARCH_FILE=$MYSQLTEST_VARDIR/log/mysqld.1.err;
 let SEARCH_PATTERN= Aborted connection.*'u' host: '192.0.2.1' real ip: '(localhost|::1)';
 source include/search_pattern_in_file.inc;
+
+--echo #
+--echo # MDEV-36269: Improve error handling for SOURCE command
+--echo # Test that SOURCE on an existent directory returns a clear error
+--echo #
+
+--error 1
+--exec $MYSQL -e "source ." 2>&1
+
+--echo End of 10.11 tests