From 62f4a20085214408dab4e893428d62eb8a65f630 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Mon, 16 Jun 2025 08:44:36 +0200 Subject: [PATCH 1/5] Refactor `File::Error.new_from_os_error` --- src/file/error.cr | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/file/error.cr b/src/file/error.cr index 355496488bc4..1fba5972eeef 100644 --- a/src/file/error.cr +++ b/src/file/error.cr @@ -10,13 +10,13 @@ class File::Error < IO::Error private def self.new_from_os_error(message, os_error, **opts) case os_error - when Errno::ENOENT, WinError::ERROR_FILE_NOT_FOUND, WinError::ERROR_PATH_NOT_FOUND + when .in?(File::NotFoundError::OS_ERRORS) File::NotFoundError.new(message, **opts) - when Errno::EEXIST, WinError::ERROR_ALREADY_EXISTS + when .in?(File::AlreadyExistsError::OS_ERRORS) File::AlreadyExistsError.new(message, **opts) - when Errno::EACCES, WinError::ERROR_ACCESS_DENIED, WinError::ERROR_PRIVILEGE_NOT_HELD + when .in?(File::AccessDeniedError::OS_ERRORS) File::AccessDeniedError.new(message, **opts) - when Errno::ENOEXEC, WinError::ERROR_BAD_EXE_FORMAT + when .in?(File::BadExecutableError::OS_ERRORS) File::BadExecutableError.new(message, **opts) else super message, os_error, **opts @@ -48,13 +48,37 @@ class File::Error < IO::Error end class File::NotFoundError < File::Error + # :nodoc: + OS_ERRORS = [ + Errno::ENOENT, + WinError::ERROR_DIRECTORY, + WinError::ERROR_FILE_NOT_FOUND, + WinError::ERROR_INVALID_NAME, + WinError::ERROR_PATH_NOT_FOUND, + ] end class File::AlreadyExistsError < File::Error + # :nodoc: + OS_ERRORS = [ + Errno::EEXIST, + WinError::ERROR_ALREADY_EXISTS, + ] end class File::AccessDeniedError < File::Error + # :nodoc: + OS_ERRORS = [ + Errno::EACCES, + WinError::ERROR_ACCESS_DENIED, + WinError::ERROR_PRIVILEGE_NOT_HELD, + ] end class File::BadExecutableError < File::Error + # :nodoc: + OS_ERRORS = [ + Errno::ENOEXEC, + WinError::ERROR_BAD_EXE_FORMAT, + ] end From e0098dd95953f97f36c8356198bd2288f74c8bc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Wed, 23 Jul 2025 10:37:23 +0200 Subject: [PATCH 2/5] Standardize system codes for file errors --- spec/std/file_spec.cr | 35 +++++++++++++++++++++++++++++ spec/std/process_spec.cr | 6 +++++ src/crystal/system/file.cr | 2 +- src/crystal/system/unix/file.cr | 6 ++--- src/crystal/system/unix/process.cr | 2 +- src/crystal/system/win32/file.cr | 15 ++++--------- src/crystal/system/win32/process.cr | 6 ++--- src/file/error.cr | 9 ++++++++ 8 files changed, 62 insertions(+), 19 deletions(-) diff --git a/spec/std/file_spec.cr b/spec/std/file_spec.cr index dd7f69d9f64c..c2b414a8013d 100644 --- a/spec/std/file_spec.cr +++ b/spec/std/file_spec.cr @@ -562,6 +562,41 @@ describe "File" do end end + long_path = "a" * 1000 + describe ".info" do + it "raises for too long pathname" do + expect_raises(File::NotFoundError, /Unable to get file info: '#{long_path}': (File ?name too long|The system cannot find the path specified)/) do + File.info(long_path) + end + end + + it "raises for invalid pathname" do + expect_raises(File::NotFoundError, /Unable to get file info: '': (No such file or directory|The system cannot find the path specified)/) do + File.info("") + end + end + + it "raises for invalid pathname" do + expect_raises(File::NotFoundError, /Unable to get file info: '<': (No such file or directory|The filename, directory name, or volume label syntax is incorrect)/) do + File.info("<") + end + end + end + + describe ".info?" do + it "returns nil for too long pathname" do + File.info?(long_path).should be_nil + end + + it "returns nil for invalid pathname" do + File.info?("").should be_nil + end + + it "returns nil for invalid pathname" do + File.info?("<").should be_nil + end + end + describe "File::Info" do it "gets for this file" do info = File.info(datapath("test_file.txt")) diff --git a/spec/std/process_spec.cr b/spec/std/process_spec.cr index 94e7522f67ca..19e8a6bb2836 100644 --- a/spec/std/process_spec.cr +++ b/spec/std/process_spec.cr @@ -68,6 +68,12 @@ describe Process do end end + it "raises for long path" do + expect_raises(File::NotFoundError, "Error executing process: 'aaaaaaa") do + Process.new("a" * 1000) + end + end + it "accepts nilable string for `chdir` (#13767)" do expect_raises(File::NotFoundError, "Error executing process: 'foobarbaz'") do Process.new("foobarbaz", chdir: nil.as(String?)) diff --git a/src/crystal/system/file.cr b/src/crystal/system/file.cr index 93c88e09b3f4..1cb4f0d92597 100644 --- a/src/crystal/system/file.cr +++ b/src/crystal/system/file.cr @@ -71,7 +71,7 @@ module Crystal::System::File when Tuple(FileDescriptor::Handle, Bool) fd, blocking = result return {fd, path, blocking} - when Errno::EEXIST, WinError::ERROR_FILE_EXISTS + when .in?(::File::AlreadyExistsError::OS_ERRORS) # retry else raise ::File::Error.from_os_error("Error creating temporary file", result, file: path) diff --git a/src/crystal/system/unix/file.cr b/src/crystal/system/unix/file.cr index 584913828077..165b2bcd1bd2 100644 --- a/src/crystal/system/unix/file.cr +++ b/src/crystal/system/unix/file.cr @@ -35,7 +35,7 @@ module Crystal::System::File if ret == 0 ::File::Info.new(stat) else - if Errno.value.in?(Errno::ENOENT, Errno::ENOTDIR) + if ::File::NotFoundError::OS_ERRORS.includes?(Errno.value) nil else raise ::File::Error.from_errno("Unable to get file info", file: path) @@ -129,7 +129,7 @@ module Crystal::System::File err = LibC.unlink(path.check_no_null_byte) if err != -1 true - elsif !raise_on_missing && Errno.value == Errno::ENOENT + elsif !raise_on_missing && ::File::NotFoundError::OS_ERRORS.includes?(Errno.value) false else raise ::File::Error.from_errno("Error deleting file", file: path) @@ -162,7 +162,7 @@ module Crystal::System::File 3.times do |iter| bytesize = LibC.readlink(path, buf, buf.bytesize) if bytesize == -1 - if Errno.value.in?(Errno::EINVAL, Errno::ENOENT, Errno::ENOTDIR) + if ::File::NotFoundError::OS_ERRORS.includes?(Errno.value) || Errno.value == Errno::EINVAL yield end diff --git a/src/crystal/system/unix/process.cr b/src/crystal/system/unix/process.cr index 59714a71bf50..0b5ad55886c5 100644 --- a/src/crystal/system/unix/process.cr +++ b/src/crystal/system/unix/process.cr @@ -343,7 +343,7 @@ struct Crystal::System::Process private def self.raise_exception_from_errno(command, errno = Errno.value) case errno - when Errno::EACCES, Errno::ENOENT, Errno::ENOEXEC + when .in?(::File::NotFoundError::OS_ERRORS), .in?(::File::AccessDeniedError::OS_ERRORS), Errno::ENOEXEC raise ::File::Error.from_os_error("Error executing process", errno, file: command) else raise IO::Error.from_os_error("Error executing process: '#{command}'", errno) diff --git a/src/crystal/system/win32/file.cr b/src/crystal/system/win32/file.cr index 383f65bb9083..53ab2569cfb2 100644 --- a/src/crystal/system/win32/file.cr +++ b/src/crystal/system/win32/file.cr @@ -97,16 +97,9 @@ module Crystal::System::File write_blocking(handle, slice, pos: @system_append ? UInt64::MAX : nil) end - NOT_FOUND_ERRORS = { - WinError::ERROR_FILE_NOT_FOUND, - WinError::ERROR_PATH_NOT_FOUND, - WinError::ERROR_INVALID_NAME, - WinError::ERROR_DIRECTORY, - } - def self.check_not_found_error(message, path) error = WinError.value - if NOT_FOUND_ERRORS.includes? error + if ::File::NotFoundError::OS_ERRORS.includes?(error) nil else raise ::File::Error.from_os_error(message, error, file: path) @@ -429,9 +422,9 @@ module Crystal::System::File info = symlink_info?(path) unless info {% begin %} - if WinError.value.in?({{ NOT_FOUND_ERRORS.splat }}, WinError::ERROR_NOT_A_REPARSE_POINT) - yield - end + if ::File::NotFoundError::OS_ERRORS.includes?(WinError.value) || WinError.value == WinError::ERROR_NOT_A_REPARSE_POINT + yield + end {% end %} raise ::File::Error.from_winerror("Cannot read link", file: path) diff --git a/src/crystal/system/win32/process.cr b/src/crystal/system/win32/process.cr index e475c500d0d2..f22f1de14d8d 100644 --- a/src/crystal/system/win32/process.cr +++ b/src/crystal/system/win32/process.cr @@ -296,8 +296,8 @@ struct Crystal::System::Process pointerof(startup_info), pointerof(process_info) ) == 0 error = WinError.value - case error.to_errno - when Errno::EACCES, Errno::ENOENT, Errno::ENOEXEC + case error + when .in?(::File::NotFoundError::OS_ERRORS), .in?(::File::AccessDeniedError::OS_ERRORS), WinError::ERROR_BAD_EXE_FORMAT raise ::File::Error.from_os_error("Error executing process", error, file: command_args) else raise IO::Error.from_os_error("Error executing process: '#{command_args}'", error) @@ -380,7 +380,7 @@ struct Crystal::System::Process private def self.raise_exception_from_errno(command, errno = Errno.value) case errno - when Errno::EACCES, Errno::ENOENT + when .in?(::File::NotFoundError::OS_ERRORS), .in?(::File::AccessDeniedError::OS_ERRORS) raise ::File::Error.from_os_error("Error executing process", errno, file: command) else raise IO::Error.from_os_error("Error executing process: '#{command}'", errno) diff --git a/src/file/error.cr b/src/file/error.cr index 1fba5972eeef..568eb579e4d9 100644 --- a/src/file/error.cr +++ b/src/file/error.cr @@ -49,12 +49,21 @@ end class File::NotFoundError < File::Error # :nodoc: + # See https://github.com/crystal-lang/crystal/issues/15905#issuecomment-2975820840 OS_ERRORS = [ + Errno::ENAMETOOLONG, Errno::ENOENT, + Errno::ENOTDIR, + WinError::ERROR_BAD_NETPATH, + WinError::ERROR_BAD_NET_NAME, + WinError::ERROR_BAD_PATHNAME, WinError::ERROR_DIRECTORY, WinError::ERROR_FILE_NOT_FOUND, + WinError::ERROR_FILENAME_EXCED_RANGE, + WinError::ERROR_INVALID_DRIVE, WinError::ERROR_INVALID_NAME, WinError::ERROR_PATH_NOT_FOUND, + WinError::WSAENAMETOOLONG, ] end From a50d7f64a11f84b2e1e207204a24dfd04b2e320f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Wed, 23 Jul 2025 18:44:38 +0200 Subject: [PATCH 3/5] Add `WinError::ERROR_FILE_EXISTS` to `AccessDeniedError` --- src/file/error.cr | 1 + 1 file changed, 1 insertion(+) diff --git a/src/file/error.cr b/src/file/error.cr index 568eb579e4d9..9467b66abe0a 100644 --- a/src/file/error.cr +++ b/src/file/error.cr @@ -72,6 +72,7 @@ class File::AlreadyExistsError < File::Error OS_ERRORS = [ Errno::EEXIST, WinError::ERROR_ALREADY_EXISTS, + WinError::ERROR_FILE_EXISTS, ] end From 72fd6ce30ac8eb6da9c3083d7252c5182b9a9429 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Wed, 23 Jul 2025 18:55:45 +0200 Subject: [PATCH 4/5] Refactor to `.os_error?` methods --- src/crystal/system/file.cr | 8 +-- src/crystal/system/unix/file.cr | 6 +-- src/crystal/system/unix/process.cr | 3 +- src/crystal/system/win32/file.cr | 4 +- src/crystal/system/win32/process.cr | 7 ++- src/file/error.cr | 76 ++++++++++++++++------------- 6 files changed, 56 insertions(+), 48 deletions(-) diff --git a/src/crystal/system/file.cr b/src/crystal/system/file.cr index 1cb4f0d92597..c31655dd2102 100644 --- a/src/crystal/system/file.cr +++ b/src/crystal/system/file.cr @@ -71,10 +71,12 @@ module Crystal::System::File when Tuple(FileDescriptor::Handle, Bool) fd, blocking = result return {fd, path, blocking} - when .in?(::File::AlreadyExistsError::OS_ERRORS) - # retry else - raise ::File::Error.from_os_error("Error creating temporary file", result, file: path) + if ::File::AlreadyExistsError.os_error?(result) + # retry + else + raise ::File::Error.from_os_error("Error creating temporary file", result, file: path) + end end end diff --git a/src/crystal/system/unix/file.cr b/src/crystal/system/unix/file.cr index 165b2bcd1bd2..f48c1b0fd0e8 100644 --- a/src/crystal/system/unix/file.cr +++ b/src/crystal/system/unix/file.cr @@ -35,7 +35,7 @@ module Crystal::System::File if ret == 0 ::File::Info.new(stat) else - if ::File::NotFoundError::OS_ERRORS.includes?(Errno.value) + if ::File::NotFoundError.os_error?(Errno.value) nil else raise ::File::Error.from_errno("Unable to get file info", file: path) @@ -129,7 +129,7 @@ module Crystal::System::File err = LibC.unlink(path.check_no_null_byte) if err != -1 true - elsif !raise_on_missing && ::File::NotFoundError::OS_ERRORS.includes?(Errno.value) + elsif !raise_on_missing && ::File::NotFoundError.os_error?(Errno.value) false else raise ::File::Error.from_errno("Error deleting file", file: path) @@ -162,7 +162,7 @@ module Crystal::System::File 3.times do |iter| bytesize = LibC.readlink(path, buf, buf.bytesize) if bytesize == -1 - if ::File::NotFoundError::OS_ERRORS.includes?(Errno.value) || Errno.value == Errno::EINVAL + if ::File::NotFoundError.os_error?(Errno.value) || Errno.value == Errno::EINVAL yield end diff --git a/src/crystal/system/unix/process.cr b/src/crystal/system/unix/process.cr index 0b5ad55886c5..369f086a3a92 100644 --- a/src/crystal/system/unix/process.cr +++ b/src/crystal/system/unix/process.cr @@ -342,8 +342,7 @@ struct Crystal::System::Process end private def self.raise_exception_from_errno(command, errno = Errno.value) - case errno - when .in?(::File::NotFoundError::OS_ERRORS), .in?(::File::AccessDeniedError::OS_ERRORS), Errno::ENOEXEC + if ::File::NotFoundError.os_error?(errno) || ::File::AccessDeniedError.os_error?(errno) || errno == Errno::ENOEXEC raise ::File::Error.from_os_error("Error executing process", errno, file: command) else raise IO::Error.from_os_error("Error executing process: '#{command}'", errno) diff --git a/src/crystal/system/win32/file.cr b/src/crystal/system/win32/file.cr index 53ab2569cfb2..87da0b75664f 100644 --- a/src/crystal/system/win32/file.cr +++ b/src/crystal/system/win32/file.cr @@ -99,7 +99,7 @@ module Crystal::System::File def self.check_not_found_error(message, path) error = WinError.value - if ::File::NotFoundError::OS_ERRORS.includes?(error) + if ::File::NotFoundError.os_error?(error) nil else raise ::File::Error.from_os_error(message, error, file: path) @@ -422,7 +422,7 @@ module Crystal::System::File info = symlink_info?(path) unless info {% begin %} - if ::File::NotFoundError::OS_ERRORS.includes?(WinError.value) || WinError.value == WinError::ERROR_NOT_A_REPARSE_POINT + if ::File::NotFoundError.os_error?(WinError.value) || WinError.value == WinError::ERROR_NOT_A_REPARSE_POINT yield end {% end %} diff --git a/src/crystal/system/win32/process.cr b/src/crystal/system/win32/process.cr index f22f1de14d8d..66a216bcd042 100644 --- a/src/crystal/system/win32/process.cr +++ b/src/crystal/system/win32/process.cr @@ -296,8 +296,8 @@ struct Crystal::System::Process pointerof(startup_info), pointerof(process_info) ) == 0 error = WinError.value - case error - when .in?(::File::NotFoundError::OS_ERRORS), .in?(::File::AccessDeniedError::OS_ERRORS), WinError::ERROR_BAD_EXE_FORMAT + case + when ::File::NotFoundError.os_error?(error) || ::File::AccessDeniedError.os_error?(error) || error == WinError::ERROR_BAD_EXE_FORMAT raise ::File::Error.from_os_error("Error executing process", error, file: command_args) else raise IO::Error.from_os_error("Error executing process: '#{command_args}'", error) @@ -379,8 +379,7 @@ struct Crystal::System::Process end private def self.raise_exception_from_errno(command, errno = Errno.value) - case errno - when .in?(::File::NotFoundError::OS_ERRORS), .in?(::File::AccessDeniedError::OS_ERRORS) + if ::File::NotFoundError.os_error?(errno) || ::File::AccessDeniedError.os_error?(errno) raise ::File::Error.from_os_error("Error executing process", errno, file: command) else raise IO::Error.from_os_error("Error executing process: '#{command}'", errno) diff --git a/src/file/error.cr b/src/file/error.cr index 9467b66abe0a..f2ceafd8663c 100644 --- a/src/file/error.cr +++ b/src/file/error.cr @@ -9,14 +9,14 @@ class File::Error < IO::Error end private def self.new_from_os_error(message, os_error, **opts) - case os_error - when .in?(File::NotFoundError::OS_ERRORS) + case + when File::NotFoundError.os_error?(os_error) File::NotFoundError.new(message, **opts) - when .in?(File::AlreadyExistsError::OS_ERRORS) + when File::AlreadyExistsError.os_error?(os_error) File::AlreadyExistsError.new(message, **opts) - when .in?(File::AccessDeniedError::OS_ERRORS) + when File::AccessDeniedError.os_error?(os_error) File::AccessDeniedError.new(message, **opts) - when .in?(File::BadExecutableError::OS_ERRORS) + when File::BadExecutableError.os_error?(os_error) File::BadExecutableError.new(message, **opts) else super message, os_error, **opts @@ -50,45 +50,53 @@ end class File::NotFoundError < File::Error # :nodoc: # See https://github.com/crystal-lang/crystal/issues/15905#issuecomment-2975820840 - OS_ERRORS = [ - Errno::ENAMETOOLONG, - Errno::ENOENT, - Errno::ENOTDIR, - WinError::ERROR_BAD_NETPATH, - WinError::ERROR_BAD_NET_NAME, - WinError::ERROR_BAD_PATHNAME, - WinError::ERROR_DIRECTORY, - WinError::ERROR_FILE_NOT_FOUND, - WinError::ERROR_FILENAME_EXCED_RANGE, - WinError::ERROR_INVALID_DRIVE, - WinError::ERROR_INVALID_NAME, - WinError::ERROR_PATH_NOT_FOUND, - WinError::WSAENAMETOOLONG, - ] + def self.os_error?(error) + error.in?( + Errno::ENAMETOOLONG, + Errno::ENOENT, + Errno::ENOTDIR, + WinError::ERROR_BAD_NETPATH, + WinError::ERROR_BAD_NET_NAME, + WinError::ERROR_BAD_PATHNAME, + WinError::ERROR_DIRECTORY, + WinError::ERROR_FILE_NOT_FOUND, + WinError::ERROR_FILENAME_EXCED_RANGE, + WinError::ERROR_INVALID_DRIVE, + WinError::ERROR_INVALID_NAME, + WinError::ERROR_PATH_NOT_FOUND, + WinError::WSAENAMETOOLONG, + ) + end end class File::AlreadyExistsError < File::Error # :nodoc: - OS_ERRORS = [ - Errno::EEXIST, - WinError::ERROR_ALREADY_EXISTS, - WinError::ERROR_FILE_EXISTS, - ] + def self.os_error?(error) + error.in?( + Errno::EEXIST, + WinError::ERROR_ALREADY_EXISTS, + WinError::ERROR_FILE_EXISTS, + ) + end end class File::AccessDeniedError < File::Error # :nodoc: - OS_ERRORS = [ - Errno::EACCES, - WinError::ERROR_ACCESS_DENIED, - WinError::ERROR_PRIVILEGE_NOT_HELD, - ] + def self.os_error?(error) + error.in?( + Errno::EACCES, + WinError::ERROR_ACCESS_DENIED, + WinError::ERROR_PRIVILEGE_NOT_HELD, + ) + end end class File::BadExecutableError < File::Error # :nodoc: - OS_ERRORS = [ - Errno::ENOEXEC, - WinError::ERROR_BAD_EXE_FORMAT, - ] + def self.os_error?(error) + error.in?( + Errno::ENOEXEC, + WinError::ERROR_BAD_EXE_FORMAT, + ) + end end From c1ee613193ad9016f995d83f26657958f6a65731 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Mon, 4 Aug 2025 22:34:41 +0200 Subject: [PATCH 5/5] Remove unnecessary macro block --- src/crystal/system/win32/file.cr | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/crystal/system/win32/file.cr b/src/crystal/system/win32/file.cr index 87da0b75664f..ac6398bee5e4 100644 --- a/src/crystal/system/win32/file.cr +++ b/src/crystal/system/win32/file.cr @@ -421,11 +421,9 @@ module Crystal::System::File def self.readlink(path, &) : String info = symlink_info?(path) unless info - {% begin %} - if ::File::NotFoundError.os_error?(WinError.value) || WinError.value == WinError::ERROR_NOT_A_REPARSE_POINT - yield - end - {% end %} + if ::File::NotFoundError.os_error?(WinError.value) || WinError.value == WinError::ERROR_NOT_A_REPARSE_POINT + yield + end raise ::File::Error.from_winerror("Cannot read link", file: path) end