Merge pull request #2641 from Iristyle/ticket/master/PUP-2382-standar…

…dize-FFI

(PUP-2382) Refactor Windows API FFI definitions / module conventions

lib/puppet/feature/base.rb

@@ -76,13 +76,23 @@
   if ! Puppet::Util::Platform.windows?
     true
   else
-    begin
-      require 'Win32API'
-      Win32API.new('kernel32', 'CreateSymbolicLink', 'SSL', 'B')
-      true
-    rescue LoadError => err
-      Puppet.debug("CreateSymbolicLink is not available")
-      false
+    module WindowsSymlink
+      require 'ffi'
+      extend FFI::Library
+
+      def self.is_implemented
+        begin
+          ffi_lib :kernel32
+          attach_function :CreateSymbolicLinkW, [:lpwstr, :lpwstr, :dword], :bool
+
+          true
+        rescue LoadError => err
+          Puppet.debug("CreateSymbolicLink is not available")
+          false
+        end
+      end
     end
+
+    WindowsSymlink.is_implemented
   end
 end

lib/puppet/util/windows.rb

@@ -1,12 +1,13 @@
 module Puppet::Util::Windows
   if Puppet::Util::Platform.windows?
     # these reference platform specific gems
+    require 'puppet/util/windows/api_types'
+    require 'puppet/util/windows/string'
     require 'puppet/util/windows/error'
     require 'puppet/util/windows/sid'
     require 'puppet/util/windows/security'
     require 'puppet/util/windows/user'
     require 'puppet/util/windows/process'
-    require 'puppet/util/windows/string'
     require 'puppet/util/windows/file'
     require 'puppet/util/windows/root_certs'
     require 'puppet/util/windows/access_control_entry'

lib/puppet/util/windows/api_types.rb

@@ -0,0 +1,57 @@
+require 'ffi'
+
+module Puppet::Util::Windows::APITypes
+  module ::FFI::Library
+    # Wrapper method for attach_function + private
+    def attach_function_private(*args)
+      attach_function(*args)
+      private args[0]
+    end
+  end
+
+  # FFI Types
+  # https://github.com/ffi/ffi/wiki/Types
+
+  # Windows - Common Data Types
+  # http://msdn.microsoft.com/en-us/library/cc230309.aspx
+
+  # Windows Data Types
+  # http://msdn.microsoft.com/en-us/library/windows/desktop/aa383751(v=vs.85).aspx
+
+  FFI.typedef :uint16, :word
+  FFI.typedef :uint32, :dword
+  # uintptr_t is defined in an FFI conf as platform specific, either
+  # ulong_long on x64 or just ulong on x86
+  FFI.typedef :uintptr_t, :handle
+
+  # buffer_inout is similar to pointer (platform specific), but optimized for buffers
+  FFI.typedef :buffer_inout, :lpwstr
+  # buffer_in is similar to pointer (platform specific), but optimized for CONST read only buffers
+  FFI.typedef :buffer_in, :lpcwstr
+
+  # string is also similar to pointer, but should be used for const char *
+  # NOTE that this is not wide, useful only for A suffixed functions
+  FFI.typedef :string, :lpcstr
+
+  # pointer in FFI is platform specific
+  # NOTE: for API calls with reserved lpvoid parameters, pass a FFI::Pointer::NULL
+  FFI.typedef :pointer, :lpvoid
+  FFI.typedef :pointer, :lpword
+  FFI.typedef :pointer, :lpdword
+  FFI.typedef :pointer, :pdword
+  FFI.typedef :pointer, :phandle
+  FFI.typedef :pointer, :ulong_ptr
+
+  # any time LONG / ULONG is in a win32 API definition DO NOT USE platform specific width
+  # which is what FFI uses by default
+  # instead create new aliases for these very special cases
+  # NOTE: not a good idea to redefine FFI :ulong since other typedefs may rely on it
+  FFI.typedef :uint32, :win32_ulong
+  FFI.typedef :int32, :win32_long
+
+  # NOTE: FFI already defines ushort as a 16-bit unsigned like this:
+  # FFI.typedef :uint16, :ushort
+
+  # 8 bits per byte
+  FFI.typedef :uchar, :byte
+end

lib/puppet/util/windows/file.rb

@@ -3,20 +3,21 @@
 module Puppet::Util::Windows::File
   require 'ffi'
   require 'windows/api'
+  extend FFI::Library
 
   def replace_file(target, source)
     target_encoded = Puppet::Util::Windows::String.wide_string(target.to_s)
     source_encoded = Puppet::Util::Windows::String.wide_string(source.to_s)
 
     flags = 0x1
     backup_file = nil
-    result = API.replace_file(
+    result = ReplaceFileW(
       target_encoded,
       source_encoded,
       backup_file,
       flags,
-      0,
-      0
+      FFI::Pointer::NULL,
+      FFI::Pointer::NULL
     )
 
     return true if result
@@ -35,97 +36,101 @@ def move_file_ex(source, target, flags = 0)
   end
   module_function :move_file_ex
 
-  module API
-    extend FFI::Library
+  ffi_convention :stdcall
+
+  # http://msdn.microsoft.com/en-us/library/windows/desktop/aa365512(v=vs.85).aspx
+  # BOOL WINAPI ReplaceFile(
+  #   _In_        LPCTSTR lpReplacedFileName,
+  #   _In_        LPCTSTR lpReplacementFileName,
+  #   _In_opt_    LPCTSTR lpBackupFileName,
+  #   _In_        DWORD dwReplaceFlags - 0x1 REPLACEFILE_WRITE_THROUGH,
+  #                                      0x2 REPLACEFILE_IGNORE_MERGE_ERRORS,
+  #                                      0x4 REPLACEFILE_IGNORE_ACL_ERRORS
+  #   _Reserved_  LPVOID lpExclude,
+  #   _Reserved_  LPVOID lpReserved
+  # );
+  ffi_lib 'kernel32'
+  attach_function_private :ReplaceFileW,
+    [:lpcwstr, :lpcwstr, :lpcwstr, :dword, :lpvoid, :lpvoid], :bool
+
+  # BOOLEAN WINAPI CreateSymbolicLink(
+  #   _In_  LPTSTR lpSymlinkFileName, - symbolic link to be created
+  #   _In_  LPTSTR lpTargetFileName, - name of target for symbolic link
+  #   _In_  DWORD dwFlags - 0x0 target is a file, 0x1 target is a directory
+  # );
+  # rescue on Windows < 6.0 so that code doesn't explode
+  begin
     ffi_lib 'kernel32'
-    ffi_convention :stdcall
-
-    # http://msdn.microsoft.com/en-us/library/windows/desktop/aa365512(v=vs.85).aspx
-    # BOOL WINAPI ReplaceFile(
-    #   _In_        LPCTSTR lpReplacedFileName,
-    #   _In_        LPCTSTR lpReplacementFileName,
-    #   _In_opt_    LPCTSTR lpBackupFileName,
-    #   _In_        DWORD dwReplaceFlags - 0x1 REPLACEFILE_WRITE_THROUGH,
-    #                                      0x2 REPLACEFILE_IGNORE_MERGE_ERRORS,
-    #                                      0x4 REPLACEFILE_IGNORE_ACL_ERRORS
-    #   _Reserved_  LPVOID lpExclude,
-    #   _Reserved_  LPVOID lpReserved
-    # );
-    attach_function :replace_file, :ReplaceFileW,
-      [:buffer_in, :buffer_in, :buffer_in, :uint, :uint, :uint], :bool
-
-    # BOOLEAN WINAPI CreateSymbolicLink(
-    #   _In_  LPTSTR lpSymlinkFileName, - symbolic link to be created
-    #   _In_  LPTSTR lpTargetFileName, - name of target for symbolic link
-    #   _In_  DWORD dwFlags - 0x0 target is a file, 0x1 target is a directory
-    # );
-    # rescue on Windows < 6.0 so that code doesn't explode
-    begin
-      attach_function :create_symbolic_link, :CreateSymbolicLinkW,
-        [:buffer_in, :buffer_in, :uint], :bool
-    rescue LoadError
-    end
-
-    # DWORD WINAPI GetFileAttributes(
-    #   _In_  LPCTSTR lpFileName
-    # );
-    attach_function :get_file_attributes, :GetFileAttributesW,
-      [:buffer_in], :uint
-
-    # HANDLE WINAPI CreateFile(
-    #   _In_      LPCTSTR lpFileName,
-    #   _In_      DWORD dwDesiredAccess,
-    #   _In_      DWORD dwShareMode,
-    #   _In_opt_  LPSECURITY_ATTRIBUTES lpSecurityAttributes,
-    #   _In_      DWORD dwCreationDisposition,
-    #   _In_      DWORD dwFlagsAndAttributes,
-    #   _In_opt_  HANDLE hTemplateFile
-    # );
-    attach_function :create_file, :CreateFileW,
-      [:buffer_in, :uint, :uint, :pointer, :uint, :uint, :uint], :uint
-
-    # http://msdn.microsoft.com/en-us/library/windows/desktop/aa363216(v=vs.85).aspx
-    # BOOL WINAPI DeviceIoControl(
-    #   _In_         HANDLE hDevice,
-    #   _In_         DWORD dwIoControlCode,
-    #   _In_opt_     LPVOID lpInBuffer,
-    #   _In_         DWORD nInBufferSize,
-    #   _Out_opt_    LPVOID lpOutBuffer,
-    #   _In_         DWORD nOutBufferSize,
-    #   _Out_opt_    LPDWORD lpBytesReturned,
-    #   _Inout_opt_  LPOVERLAPPED lpOverlapped
-    # );
-    attach_function :device_io_control, :DeviceIoControl,
-      [:uint, :uint, :pointer, :uint, :pointer, :uint, :pointer, :pointer], :bool
-
-    MAXIMUM_REPARSE_DATA_BUFFER_SIZE = 16384
-
-    # REPARSE_DATA_BUFFER
-    # http://msdn.microsoft.com/en-us/library/cc232006.aspx
-    # http://msdn.microsoft.com/en-us/library/windows/hardware/ff552012(v=vs.85).aspx
-    # struct is always MAXIMUM_REPARSE_DATA_BUFFER_SIZE bytes
-    class ReparseDataBuffer < FFI::Struct
-      layout :reparse_tag, :uint,
-             :reparse_data_length, :ushort,
-             :reserved, :ushort,
-             :substitute_name_offset, :ushort,
-             :substitute_name_length, :ushort,
-             :print_name_offset, :ushort,
-             :print_name_length, :ushort,
-             :flags, :uint,
-             # max less above fields dword / uint 4 bytes, ushort 2 bytes
-             :path_buffer, [:uchar, MAXIMUM_REPARSE_DATA_BUFFER_SIZE - 20]
-    end
+    attach_function_private :CreateSymbolicLinkW,
+      [:lpwstr, :lpwstr, :dword], :bool
+  rescue LoadError
+  end
 
-    # BOOL WINAPI CloseHandle(
-    #   _In_  HANDLE hObject
-    # );
-    attach_function :close_handle, :CloseHandle, [:uint], :bool
+  # DWORD WINAPI GetFileAttributes(
+  #   _In_  LPCTSTR lpFileName
+  # );
+  ffi_lib 'kernel32'
+  attach_function_private :GetFileAttributesW,
+    [:lpcwstr], :dword
+
+  # HANDLE WINAPI CreateFile(
+  #   _In_      LPCTSTR lpFileName,
+  #   _In_      DWORD dwDesiredAccess,
+  #   _In_      DWORD dwShareMode,
+  #   _In_opt_  LPSECURITY_ATTRIBUTES lpSecurityAttributes,
+  #   _In_      DWORD dwCreationDisposition,
+  #   _In_      DWORD dwFlagsAndAttributes,
+  #   _In_opt_  HANDLE hTemplateFile
+  # );
+  ffi_lib 'kernel32'
+  attach_function_private :CreateFileW,
+    [:lpcwstr, :dword, :dword, :pointer, :dword, :dword, :handle], :handle
+
+  # http://msdn.microsoft.com/en-us/library/windows/desktop/aa363216(v=vs.85).aspx
+  # BOOL WINAPI DeviceIoControl(
+  #   _In_         HANDLE hDevice,
+  #   _In_         DWORD dwIoControlCode,
+  #   _In_opt_     LPVOID lpInBuffer,
+  #   _In_         DWORD nInBufferSize,
+  #   _Out_opt_    LPVOID lpOutBuffer,
+  #   _In_         DWORD nOutBufferSize,
+  #   _Out_opt_    LPDWORD lpBytesReturned,
+  #   _Inout_opt_  LPOVERLAPPED lpOverlapped
+  # );
+  ffi_lib 'kernel32'
+  attach_function_private :DeviceIoControl,
+    [:handle, :dword, :lpvoid, :dword, :lpvoid, :dword, :lpdword, :pointer], :bool
+
+  MAXIMUM_REPARSE_DATA_BUFFER_SIZE = 16384
+
+  # REPARSE_DATA_BUFFER
+  # http://msdn.microsoft.com/en-us/library/cc232006.aspx
+  # http://msdn.microsoft.com/en-us/library/windows/hardware/ff552012(v=vs.85).aspx
+  # struct is always MAXIMUM_REPARSE_DATA_BUFFER_SIZE bytes
+  class REPARSE_DATA_BUFFER < FFI::Struct
+    layout :ReparseTag, :win32_ulong,
+           :ReparseDataLength, :ushort,
+           :Reserved, :ushort,
+           :SubstituteNameOffset, :ushort,
+           :SubstituteNameLength, :ushort,
+           :PrintNameOffset, :ushort,
+           :PrintNameLength, :ushort,
+           :Flags, :win32_ulong,
+           # max less above fields dword / uint 4 bytes, ushort 2 bytes
+           # technically a WCHAR buffer, but we care about size in bytes here
+           :PathBuffer, [:byte, MAXIMUM_REPARSE_DATA_BUFFER_SIZE - 20]
   end
 
+  # http://msdn.microsoft.com/en-us/library/windows/desktop/ms724211(v=vs.85).aspx
+  # BOOL WINAPI CloseHandle(
+  #   _In_  HANDLE hObject
+  # );
+  ffi_lib 'kernel32'
+  attach_function_private :CloseHandle, [:handle], :bool
+
   def symlink(target, symlink)
     flags = File.directory?(target) ? 0x1 : 0x0
-    result = API.create_symbolic_link(Puppet::Util::Windows::String.wide_string(symlink.to_s),
+    result = CreateSymbolicLinkW(Puppet::Util::Windows::String.wide_string(symlink.to_s),
       Puppet::Util::Windows::String.wide_string(target.to_s), flags)
     return true if result
     raise Puppet::Util::Windows::Error.new(
@@ -135,7 +140,7 @@ def symlink(target, symlink)
 
   INVALID_FILE_ATTRIBUTES = 0xFFFFFFFF #define INVALID_FILE_ATTRIBUTES (DWORD (-1))
   def self.get_file_attributes(file_name)
-    result = API.get_file_attributes(Puppet::Util::Windows::String.wide_string(file_name.to_s))
+    result = GetFileAttributesW(Puppet::Util::Windows::String.wide_string(file_name.to_s))
     return result unless result == INVALID_FILE_ATTRIBUTES
     raise Puppet::Util::Windows::Error.new("GetFileAttributes(#{file_name})")
   end
@@ -144,7 +149,7 @@ def self.get_file_attributes(file_name)
   def self.create_file(file_name, desired_access, share_mode, security_attributes,
     creation_disposition, flags_and_attributes, template_file_handle)
 
-    result = API.create_file(Puppet::Util::Windows::String.wide_string(file_name.to_s),
+    result = CreateFileW(Puppet::Util::Windows::String.wide_string(file_name.to_s),
       desired_access, share_mode, security_attributes, creation_disposition,
       flags_and_attributes, template_file_handle)
 
@@ -160,12 +165,12 @@ def self.device_io_control(handle, io_control_code, in_buffer = nil, out_buffer
       raise Puppet::Util::Windows::Error.new("out_buffer is required")
     end
 
-    result = API.device_io_control(
+    result = DeviceIoControl(
       handle,
       io_control_code,
       in_buffer, in_buffer.nil? ? 0 : in_buffer.size,
       out_buffer, out_buffer.size,
-      FFI::MemoryPointer.new(:uint, 1),
+      FFI::MemoryPointer.new(:dword, 1),
       nil
     )
 
@@ -205,7 +210,7 @@ def self.open_symlink(link_name)
       FILE_FLAG_OPEN_REPARSE_POINT | FILE_FLAG_BACKUP_SEMANTICS,
       0) # template_file
     ensure
-      API.close_handle(handle) if handle
+      CloseHandle(handle) if handle
     end
   end
 
@@ -266,14 +271,14 @@ def stat.ftype
 
   def self.resolve_symlink(handle)
     # must be multiple of 1024, min 10240
-    out_buffer = FFI::MemoryPointer.new(API::ReparseDataBuffer.size)
+    out_buffer = FFI::MemoryPointer.new(REPARSE_DATA_BUFFER.size)
     device_io_control(handle, FSCTL_GET_REPARSE_POINT, nil, out_buffer)
 
-    reparse_data = API::ReparseDataBuffer.new(out_buffer)
-    offset = reparse_data[:print_name_offset]
-    length = reparse_data[:print_name_length]
+    reparse_data = REPARSE_DATA_BUFFER.new(out_buffer)
+    offset = reparse_data[:PrintNameOffset]
+    length = reparse_data[:PrintNameLength]
 
-    result = reparse_data[:path_buffer].to_a[offset, length].pack('C*')
+    result = reparse_data[:PathBuffer].to_a[offset, length].pack('C*')
     result.force_encoding('UTF-16LE').encode(Encoding.default_external)
   end
 end