From c315bc7590cbe59207e7708304bca661f1375fc8 Mon Sep 17 00:00:00 2001 From: Takuro Ashie Date: Tue, 7 Jun 2022 11:32:17 +0900 Subject: [PATCH 1/3] Simplify WindowsFile We don't need to call CreateFile() by ourself, use standard File.open and _get_osfhandle() instead. It will also fix a bug that WindowsFile cannot open non-ascii path. Fix #3769 Signed-off-by: Takuro Ashie --- lib/fluent/plugin/file_wrapper.rb | 164 +++++++++--------------------- 1 file changed, 50 insertions(+), 114 deletions(-) diff --git a/lib/fluent/plugin/file_wrapper.rb b/lib/fluent/plugin/file_wrapper.rb index f05af89205..ca864b0943 100644 --- a/lib/fluent/plugin/file_wrapper.rb +++ b/lib/fluent/plugin/file_wrapper.rb @@ -16,35 +16,8 @@ module Fluent module FileWrapper - include File::Constants - - def self.mode2flags(mode) - # Always need BINARY to enable SHARE_DELETE - # https://bugs.ruby-lang.org/issues/11218 - # https://github.com/ruby/ruby/blob/d6684f063bc53e3cab025bd39526eca3b480b5e7/win32/win32.c#L6332-L6345 - flags = BINARY | SHARE_DELETE - case mode.delete("b") - when "r" - flags |= RDONLY - when "r+" - flags |= RDWR - when "w" - flags |= WRONLY | CREAT | TRUNC - when "w+" - flags |= RDWR | CREAT | TRUNC - when "a" - flags |= WRONLY | CREAT | APPEND - when "a+" - flags |= RDWR | CREAT | APPEND - else - raise Errno::EINVAL.new("Unsupported mode by Fluent::FileWrapper: #{mode}") - end - end - def self.open(path, mode='r') - # inject File::Constants::SHARE_DELETE - # https://github.com/fluent/fluentd/pull/3585#issuecomment-1101502617 - io = File.open(path, mode2flags(mode)) + io = WindowsFile.new(path, mode).io if block_given? v = yield io io.close @@ -62,97 +35,35 @@ def self.stat(path) end end - class Win32Error < StandardError - require 'windows/error' - include Windows::Error - - attr_reader :errcode, :msg - - WSABASEERR = 10000 - - def initialize(errcode, msg = nil) - @errcode = errcode - @msg = msg - end - - def format_english_message(errcode) - buf = 0.chr * 260 - flags = FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ARGUMENT_ARRAY - english_lang_id = 1033 # The result of MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US) - FormatMessageA.call(flags, 0, errcode, english_lang_id, buf, buf.size, 0) - buf.force_encoding(Encoding.default_external).strip - end - - def to_s - msg = super - msg << ": code: #{@errcode}, #{format_english_message(@errcode)}" - msg << " - #{@msg}" if @msg - msg - end - - def inspect - "#<#{to_s}>" - end - - def ==(other) - return false if other.class != Win32Error - @errcode == other.errcode && @msg == other.msg - end - - def wsaerr? - @errcode >= WSABASEERR - end - end - - # To open and get stat with setting FILE_SHARE_DELETE. - # Although recent Ruby's File.stat uses it, we still need this to keep - # backward compatibility of ino and delete_pending methods. class WindowsFile require 'windows/file' - require 'windows/error' require 'windows/handle' - require 'windows/nio' - include Windows::Error + include File::Constants include Windows::File include Windows::Handle - include Windows::NIO - - def initialize(path, mode='r', sharemode=FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE) - @path = path - @file_handle = INVALID_HANDLE_VALUE - @mode = mode + attr_reader :io - access, creationdisposition, _seektoend = case mode.delete('b') - when "r" ; [FILE_GENERIC_READ , OPEN_EXISTING, false] - when "r+"; [FILE_GENERIC_READ | FILE_GENERIC_WRITE, OPEN_ALWAYS , false] - when "w" ; [FILE_GENERIC_WRITE , CREATE_ALWAYS, false] - when "w+"; [FILE_GENERIC_READ | FILE_GENERIC_WRITE, CREATE_ALWAYS, false] - when "a" ; [FILE_GENERIC_WRITE , OPEN_ALWAYS , true] - when "a+"; [FILE_GENERIC_READ | FILE_GENERIC_WRITE, OPEN_ALWAYS , true] - else raise "unknown mode '#{mode}'" - end - - @file_handle = CreateFile.call(@path, access, sharemode, - 0, creationdisposition, FILE_ATTRIBUTE_NORMAL, 0) - if @file_handle == INVALID_HANDLE_VALUE - win32err = Win32Error.new(Win32::API.last_error, path) - errno = ServerEngine::RbWinSock.rb_w32_map_errno(win32err.errcode) - if errno == Errno::EINVAL::Errno || win32err.wsaerr? - # maybe failed to map - raise win32err - else - raise SystemCallError.new(win32err.message, errno) - end + def initialize(path, mode='r') + @path = path + @io = File.open(path, mode2flags(mode)) + @file_handle = _get_osfhandle(@io.to_i) + @io.instance_variable_set(:@file_index, self.ino) + def @io.ino + @file_index end end def close - CloseHandle.call(@file_handle) + @io.close @file_handle = INVALID_HANDLE_VALUE end + # To keep backward compatibility, we continue to use GetFileInformationByHandle() + # to get file id. + # Node that Ruby's File.stat uses GetFileInformationByHandleEx() with FileIdInfo + # and returned value is different with above one. def ino by_handle_file_information = '\0'*(4+8+8+8+4+4+4+4+4+4) #72bytes @@ -163,6 +74,41 @@ def ino by_handle_file_information.unpack("I11Q1")[11] # fileindex end + def stat + raise Errno::ENOENT if delete_pending + s = File.stat(@path) + s.instance_variable_set :@ino, self.ino + def s.ino; @ino; end + s + end + + private + + def mode2flags(mode) + # Always inject File::Constants::SHARE_DELETE + # https://github.com/fluent/fluentd/pull/3585#issuecomment-1101502617 + # To enable SHARE_DELETE, BINARY is also required. + # https://bugs.ruby-lang.org/issues/11218 + # https://github.com/ruby/ruby/blob/d6684f063bc53e3cab025bd39526eca3b480b5e7/win32/win32.c#L6332-L6345 + flags = BINARY | SHARE_DELETE + case mode.delete("b") + when "r" + flags |= RDONLY + when "r+" + flags |= RDWR + when "w" + flags |= WRONLY | CREAT | TRUNC + when "w+" + flags |= RDWR | CREAT | TRUNC + when "a" + flags |= WRONLY | CREAT | APPEND + when "a+" + flags |= RDWR | CREAT | APPEND + else + raise Errno::EINVAL.new("Unsupported mode by Fluent::FileWrapper: #{mode}") + end + end + # DeletePending is a Windows-specific file state that roughly means # "this file is queued for deletion, so close any open handlers" # @@ -181,15 +127,5 @@ def delete_pending return buf.unpack("QQICC")[3] != 0 end - - private :delete_pending - - def stat - raise Errno::ENOENT if delete_pending - s = File.stat(@path) - s.instance_variable_set :@ino, self.ino - def s.ino; @ino; end - s - end end end if Fluent.windows? From e1e9c470dc47097ddb2c7cbea00b50a1d4d5fdd5 Mon Sep 17 00:00:00 2001 From: Takuro Ashie Date: Tue, 7 Jun 2022 12:42:47 +0900 Subject: [PATCH 2/3] test_file_wrapper: Remove obsolete tests Signed-off-by: Takuro Ashie --- test/plugin/test_file_wrapper.rb | 68 -------------------------------- 1 file changed, 68 deletions(-) diff --git a/test/plugin/test_file_wrapper.rb b/test/plugin/test_file_wrapper.rb index ea1c9d508e..380cb4257c 100644 --- a/test/plugin/test_file_wrapper.rb +++ b/test/plugin/test_file_wrapper.rb @@ -17,58 +17,6 @@ def teardown FileUtils.rm_rf(TMP_DIR) end - sub_test_case 'Win32Error' do - test 'equal' do - assert_equal(Fluent::Win32Error.new(ERROR_SHARING_VIOLATION, "message"), - Fluent::Win32Error.new(ERROR_SHARING_VIOLATION, "message")) - end - - test 'different error code' do - assert_not_equal(Fluent::Win32Error.new(ERROR_FILE_NOT_FOUND), - Fluent::Win32Error.new(ERROR_SHARING_VIOLATION)) - end - - test 'different error message' do - assert_not_equal(Fluent::Win32Error.new(ERROR_FILE_NOT_FOUND, "message1"), - Fluent::Win32Error.new(ERROR_FILE_NOT_FOUND, "message2")) - end - - test 'different class' do - assert_not_equal(Errno::EPIPE, - Fluent::Win32Error.new(ERROR_SHARING_VIOLATION)) - end - - test 'ERROR_SHARING_VIOLATION message' do - assert_equal(Fluent::Win32Error.new(ERROR_SHARING_VIOLATION).message, - "Fluent::Win32Error: code: 32, The process cannot access the file because it is being used by another process.") - end - - test 'ERROR_SHARING_VIOLATION with a message' do - assert_equal(Fluent::Win32Error.new(ERROR_SHARING_VIOLATION, "cannot open the file").message, - "Fluent::Win32Error: code: 32, The process cannot access the file because it is being used by another process." + - " - cannot open the file") - end - - test 'to_s' do - assert_equal("Fluent::Win32Error: code: 32, The process cannot access the file because it is being used by another process. - C:\file.txt", - Fluent::Win32Error.new(ERROR_SHARING_VIOLATION, "C:\file.txt").to_s) - end - - test 'inspect' do - assert_equal("#", - Fluent::Win32Error.new(ERROR_SHARING_VIOLATION, "C:\file.txt").inspect) - end - - data('0' => [false, 0], - '9999' => [false, 9999], - '10000' => [true, 10000], - '10001' => [true, 10001]) - test 'wsaerr?' do |data| - expected, code = data - assert_equal(expected, Fluent::Win32Error.new(code).wsaerr?) - end - end - sub_test_case 'WindowsFile exceptions' do test 'nothing raised' do begin @@ -106,21 +54,5 @@ def teardown file.close if file end end - - test 'ERROR_SHARING_VIOLATION raised' do - begin - path = "#{TMP_DIR}/test_windows_file.txt" - file1 = file2 = nil - file1 = File.open(path, "wb") - win32err = Fluent::Win32Error.new(ERROR_SHARING_VIOLATION, path) - assert_raise(Errno::EACCES.new(win32err.message)) do - file2 = Fluent::WindowsFile.new(path, 'r', FILE_SHARE_READ) - ensure - file2.close if file2 - end - ensure - file1.close if file1 - end - end end end if Fluent.windows? From c95e3d1af7509087b3a6c1ee481c7545d936df89 Mon Sep 17 00:00:00 2001 From: Takuro Ashie Date: Thu, 9 Jun 2022 13:19:24 +0900 Subject: [PATCH 3/3] file_wrapper: Fix a typo in a comment In addition, add more descritpion about FileId. [skip ci] Signed-off-by: Takuro Ashie --- fluentd.gemspec | 1 + lib/fluent/plugin/file_wrapper.rb | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/fluentd.gemspec b/fluentd.gemspec index cb8f1701c6..3b1a95e0e5 100644 --- a/fluentd.gemspec +++ b/fluentd.gemspec @@ -38,6 +38,7 @@ Gem::Specification.new do |gem| gem.add_runtime_dependency("win32-service", ["~> 2.3.0"]) gem.add_runtime_dependency("win32-ipc", ["~> 0.7.0"]) gem.add_runtime_dependency("win32-event", ["~> 0.6.3"]) + gem.add_runtime_dependency("windows-api", ["~> 0.4.5"]) gem.add_runtime_dependency("windows-pr", ["~> 1.2.6"]) gem.add_runtime_dependency("certstore_c", ["~> 0.1.7"]) end diff --git a/lib/fluent/plugin/file_wrapper.rb b/lib/fluent/plugin/file_wrapper.rb index ca864b0943..0ef875b14e 100644 --- a/lib/fluent/plugin/file_wrapper.rb +++ b/lib/fluent/plugin/file_wrapper.rb @@ -62,8 +62,9 @@ def close # To keep backward compatibility, we continue to use GetFileInformationByHandle() # to get file id. - # Node that Ruby's File.stat uses GetFileInformationByHandleEx() with FileIdInfo - # and returned value is different with above one. + # Note that Ruby's File.stat uses GetFileInformationByHandleEx() with FileIdInfo + # and returned value is different with above one, former one is 64 bit while + # later one is 128bit. def ino by_handle_file_information = '\0'*(4+8+8+8+4+4+4+4+4+4) #72bytes