[ruby/prism] Check errno for parsing directory

https://github.com/ruby/prism/commit/d68ea29d04

lib/prism/ffi.rb

@@ -72,6 +72,7 @@ module Prism
     end
 
     callback :pm_parse_stream_fgets_t, [:pointer, :int, :pointer], :pointer
+    enum :pm_string_init_result_t, %i[PM_STRING_INIT_SUCCESS PM_STRING_INIT_ERROR_GENERIC PM_STRING_INIT_ERROR_DIRECTORY]
 
     load_exported_functions_from(
       "prism.h",
@@ -176,13 +177,26 @@ module Prism
       def self.with_file(filepath)
         raise TypeError unless filepath.is_a?(String)
 
+        # On Windows and Mac, it's expected that filepaths will be encoded in
+        # UTF-8. If they are not, we need to convert them to UTF-8 before
+        # passing them into pm_string_mapped_init.
+        if RbConfig::CONFIG["host_os"].match?(/bccwin|cygwin|djgpp|mingw|mswin|wince|darwin/i) &&
+           (encoding = filepath.encoding) != Encoding::ASCII_8BIT && encoding != Encoding::UTF_8
+          filepath = filepath.encode(Encoding::UTF_8)
+        end
+
         FFI::MemoryPointer.new(SIZEOF) do |pm_string|
-          if LibRubyParser.pm_string_mapped_init(pm_string, filepath)
+          case (result = LibRubyParser.pm_string_mapped_init(pm_string, filepath))
+          when :PM_STRING_INIT_SUCCESS
             pointer = LibRubyParser.pm_string_source(pm_string)
             length = LibRubyParser.pm_string_length(pm_string)
             return yield new(pointer, length, false)
-          else
+          when :PM_STRING_INIT_ERROR_GENERIC
             raise SystemCallError.new(filepath, FFI.errno)
+          when :PM_STRING_INIT_ERROR_DIRECTORY
+            raise Errno::EISDIR.new(filepath)
+          else
+            raise "Unknown error initializing pm_string_t: #{result.inspect}"
           end
         ensure
           LibRubyParser.pm_string_free(pm_string)

prism/extension.c

@@ -263,18 +263,32 @@ file_options(int argc, VALUE *argv, pm_string_t *input, pm_options_t *options, V
     *encoded_filepath = rb_str_encode_ospath(filepath);
     extract_options(options, *encoded_filepath, keywords);
 
-    const char * string_source = (const char *) pm_string_source(&options->filepath);
+    const char *source = (const char *) pm_string_source(&options->filepath);
+    pm_string_init_result_t result;
 
-    if (!pm_string_file_init(input, string_source)) {
-        pm_options_free(options);
+    switch (result = pm_string_file_init(input, source)) {
+        case PM_STRING_INIT_SUCCESS:
+            break;
+        case PM_STRING_INIT_ERROR_GENERIC: {
+            pm_options_free(options);
 
 #ifdef _WIN32
-        int e = rb_w32_map_errno(GetLastError());
+            int e = rb_w32_map_errno(GetLastError());
 #else
-        int e = errno;
+            int e = errno;
 #endif
 
-        rb_syserr_fail(e, string_source);
+            rb_syserr_fail(e, source);
+            break;
+        }
+        case PM_STRING_INIT_ERROR_DIRECTORY:
+            pm_options_free(options);
+            rb_syserr_fail(EISDIR, source);
+            break;
+        default:
+            pm_options_free(options);
+            rb_raise(rb_eRuntimeError, "Unknown error (%d) initializing file: %s", result, source);
+            break;
     }
 }
 

prism/util/pm_string.c

@@ -64,24 +64,33 @@ typedef struct {
  * Open the file indicated by the filepath parameter for reading on Windows.
  * Perform any kind of normalization that needs to happen on the filepath.
  */
-static bool
+static pm_string_init_result_t
 pm_string_file_handle_open(pm_string_file_handle_t *handle, const char *filepath) {
     int length = MultiByteToWideChar(CP_UTF8, 0, filepath, -1, NULL, 0);
-    if (length == 0) return false;
+    if (length == 0) return PM_STRING_INIT_ERROR_GENERIC;
 
     handle->path = xmalloc(sizeof(WCHAR) * ((size_t) length));
     if ((handle->path == NULL) || (MultiByteToWideChar(CP_UTF8, 0, filepath, -1, handle->path, length) == 0)) {
         xfree(handle->path);
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     handle->file = CreateFileW(handle->path, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_READONLY, NULL);
     if (handle->file == INVALID_HANDLE_VALUE) {
+        pm_string_init_result_t result = PM_STRING_INIT_ERROR_GENERIC;
+
+        if (GetLastError() == ERROR_ACCESS_DENIED) {
+            DWORD attributes = GetFileAttributesW(handle->path);
+            if ((attributes != INVALID_FILE_ATTRIBUTES) && (attributes & FILE_ATTRIBUTE_DIRECTORY)) {
+                result = PM_STRING_INIT_ERROR_DIRECTORY;
+            }
+        }
+
         xfree(handle->path);
-        return false;
+        return result;
     }
 
-    return true;
+    return PM_STRING_INIT_SUCCESS;
 }
 
 /**
@@ -105,18 +114,19 @@ pm_string_file_handle_close(pm_string_file_handle_t *handle) {
  * `MapViewOfFile`, on POSIX systems that have access to `mmap` we'll use
  * `mmap`, and on other POSIX systems we'll use `read`.
  */
-PRISM_EXPORTED_FUNCTION bool
+PRISM_EXPORTED_FUNCTION pm_string_init_result_t
 pm_string_mapped_init(pm_string_t *string, const char *filepath) {
 #ifdef _WIN32
     // Open the file for reading.
     pm_string_file_handle_t handle;
-    if (!pm_string_file_handle_open(&handle, filepath)) return false;
+    pm_string_init_result_t result = pm_string_file_handle_open(&handle, filepath);
+    if (result != PM_STRING_INIT_SUCCESS) return result;
 
     // Get the file size.
     DWORD file_size = GetFileSize(handle.file, NULL);
     if (file_size == INVALID_FILE_SIZE) {
         pm_string_file_handle_close(&handle);
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     // If the file is empty, then we don't need to do anything else, we'll set
@@ -125,14 +135,14 @@ pm_string_mapped_init(pm_string_t *string, const char *filepath) {
         pm_string_file_handle_close(&handle);
         const uint8_t source[] = "";
         *string = (pm_string_t) { .type = PM_STRING_CONSTANT, .source = source, .length = 0 };
-        return true;
+        return PM_STRING_INIT_SUCCESS;
     }
 
     // Create a mapping of the file.
     HANDLE mapping = CreateFileMapping(handle.file, NULL, PAGE_READONLY, 0, 0, NULL);
     if (mapping == NULL) {
         pm_string_file_handle_close(&handle);
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     // Map the file into memory.
@@ -141,30 +151,29 @@ pm_string_mapped_init(pm_string_t *string, const char *filepath) {
     pm_string_file_handle_close(&handle);
 
     if (source == NULL) {
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     *string = (pm_string_t) { .type = PM_STRING_MAPPED, .source = source, .length = (size_t) file_size };
-    return true;
+    return PM_STRING_INIT_SUCCESS;
 #elif defined(_POSIX_MAPPED_FILES)
     // Open the file for reading
     int fd = open(filepath, O_RDONLY);
     if (fd == -1) {
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     // Stat the file to get the file size
     struct stat sb;
     if (fstat(fd, &sb) == -1) {
         close(fd);
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     // Ensure it is a file and not a directory
     if (S_ISDIR(sb.st_mode)) {
-        errno = EISDIR;
         close(fd);
-        return false;
+        return PM_STRING_INIT_ERROR_DIRECTORY;
     }
 
     // mmap the file descriptor to virtually get the contents
@@ -175,17 +184,17 @@ pm_string_mapped_init(pm_string_t *string, const char *filepath) {
         close(fd);
         const uint8_t source[] = "";
         *string = (pm_string_t) { .type = PM_STRING_CONSTANT, .source = source, .length = 0 };
-        return true;
+        return PM_STRING_INIT_SUCCESS;
     }
 
     source = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
     if (source == MAP_FAILED) {
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     close(fd);
     *string = (pm_string_t) { .type = PM_STRING_MAPPED, .source = source, .length = size };
-    return true;
+    return PM_STRING_INIT_SUCCESS;
 #else
     return pm_string_file_init(string, filepath);
 #endif
@@ -196,18 +205,19 @@ pm_string_mapped_init(pm_string_t *string, const char *filepath) {
  * contents and size into the given `pm_string_t`. The given `pm_string_t`
  * should be freed using `pm_string_free` when it is no longer used.
  */
-PRISM_EXPORTED_FUNCTION bool
+PRISM_EXPORTED_FUNCTION pm_string_init_result_t
 pm_string_file_init(pm_string_t *string, const char *filepath) {
 #ifdef _WIN32
     // Open the file for reading.
     pm_string_file_handle_t handle;
-    if (!pm_string_file_handle_open(&handle, filepath)) return false;
+    pm_string_init_result_t result = pm_string_file_handle_open(&handle, filepath);
+    if (result != PM_STRING_INIT_SUCCESS) return result;
 
     // Get the file size.
     DWORD file_size = GetFileSize(handle.file, NULL);
     if (file_size == INVALID_FILE_SIZE) {
         pm_string_file_handle_close(&handle);
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     // If the file is empty, then we don't need to do anything else, we'll set
@@ -216,37 +226,37 @@ pm_string_file_init(pm_string_t *string, const char *filepath) {
         pm_string_file_handle_close(&handle);
         const uint8_t source[] = "";
         *string = (pm_string_t) { .type = PM_STRING_CONSTANT, .source = source, .length = 0 };
-        return true;
+        return PM_STRING_INIT_SUCCESS;
     }
 
     // Create a buffer to read the file into.
     uint8_t *source = xmalloc(file_size);
     if (source == NULL) {
         pm_string_file_handle_close(&handle);
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     // Read the contents of the file
     DWORD bytes_read;
     if (!ReadFile(handle.file, source, file_size, &bytes_read, NULL)) {
         pm_string_file_handle_close(&handle);
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     // Check the number of bytes read
     if (bytes_read != file_size) {
         xfree(source);
         pm_string_file_handle_close(&handle);
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     pm_string_file_handle_close(&handle);
     *string = (pm_string_t) { .type = PM_STRING_OWNED, .source = source, .length = (size_t) file_size };
-    return true;
+    return PM_STRING_INIT_SUCCESS;
 #elif defined(PRISM_HAS_FILESYSTEM)
     FILE *file = fopen(filepath, "rb");
     if (file == NULL) {
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     fseek(file, 0, SEEK_END);
@@ -254,21 +264,21 @@ pm_string_file_init(pm_string_t *string, const char *filepath) {
 
     if (file_size == -1) {
         fclose(file);
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     if (file_size == 0) {
         fclose(file);
         const uint8_t source[] = "";
         *string = (pm_string_t) { .type = PM_STRING_CONSTANT, .source = source, .length = 0 };
-        return true;
+        return PM_STRING_INIT_SUCCESS;
     }
 
     size_t length = (size_t) file_size;
     uint8_t *source = xmalloc(length);
     if (source == NULL) {
         fclose(file);
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     fseek(file, 0, SEEK_SET);
@@ -277,16 +287,16 @@ pm_string_file_init(pm_string_t *string, const char *filepath) {
 
     if (bytes_read != 1) {
         xfree(source);
-        return false;
+        return PM_STRING_INIT_ERROR_GENERIC;
     }
 
     *string = (pm_string_t) { .type = PM_STRING_OWNED, .source = source, .length = length };
-    return true;
+    return PM_STRING_INIT_SUCCESS;
 #else
     (void) string;
     (void) filepath;
     perror("pm_string_file_init is not implemented for this platform");
-    return false;
+    return PM_STRING_INIT_ERROR_GENERIC;
 #endif
 }
 

prism/util/pm_string.h

@@ -93,6 +93,26 @@ void pm_string_owned_init(pm_string_t *string, uint8_t *source, size_t length);
  */
 void pm_string_constant_init(pm_string_t *string, const char *source, size_t length);
 
+/**
+ * Represents the result of calling pm_string_mapped_init or
+ * pm_string_file_init. We need this additional information because there is
+ * not a platform-agnostic way to indicate that the file that was attempted to
+ * be opened was a directory.
+ */
+typedef enum {
+    /** Indicates that the string was successfully initialized. */
+    PM_STRING_INIT_SUCCESS = 0,
+    /**
+     * Indicates a generic error from a string_*_init function, where the type
+     * of error should be read from `errno` or `GetLastError()`.
+     */
+    PM_STRING_INIT_ERROR_GENERIC = 1,
+    /**
+     * Indicates that the file that was attempted to be opened was a directory.
+     */
+    PM_STRING_INIT_ERROR_DIRECTORY = 2
+} pm_string_init_result_t;
+
 /**
  * Read the file indicated by the filepath parameter into source and load its
  * contents and size into the given `pm_string_t`. The given `pm_string_t`
@@ -106,9 +126,9 @@ void pm_string_constant_init(pm_string_t *string, const char *source, size_t len
  *
  * @param string The string to initialize.
  * @param filepath The filepath to read.
- * @return Whether or not the file was successfully mapped.
+ * @return The success of the read, indicated by the value of the enum.
  */
-PRISM_EXPORTED_FUNCTION bool pm_string_mapped_init(pm_string_t *string, const char *filepath);
+PRISM_EXPORTED_FUNCTION pm_string_init_result_t pm_string_mapped_init(pm_string_t *string, const char *filepath);
 
 /**
  * Read the file indicated by the filepath parameter into source and load its
@@ -117,9 +137,9 @@ PRISM_EXPORTED_FUNCTION bool pm_string_mapped_init(pm_string_t *string, const ch
  *
  * @param string The string to initialize.
  * @param filepath The filepath to read.
- * @return Whether or not the file was successfully read.
+ * @return The success of the read, indicated by the value of the enum.
  */
-PRISM_EXPORTED_FUNCTION bool pm_string_file_init(pm_string_t *string, const char *filepath);
+PRISM_EXPORTED_FUNCTION pm_string_init_result_t pm_string_file_init(pm_string_t *string, const char *filepath);
 
 /**
  * Ensure the string is owned. If it is not, then reinitialize it as owned and

test/prism/api/parse_test.rb

@@ -79,6 +79,20 @@ module Prism
       end
     end
 
+    def test_parse_directory
+      error = nil
+
+      begin
+        Prism.parse_file(__dir__)
+      rescue SystemCallError => error
+      end
+
+      refute_nil error
+      return if error.is_a?(Errno::ENOMEM)
+
+      assert_kind_of Errno::EISDIR, error
+    end
+
     private
 
     def find_source_file_node(program)