diff --git a/internal/headers/hcs/hcs_test.go b/internal/headers/hcs/hcs_test.go index 72715d12..b4b36fe1 100644 --- a/internal/headers/hcs/hcs_test.go +++ b/internal/headers/hcs/hcs_test.go @@ -18,16 +18,38 @@ package hcs_test import ( + "errors" "testing" "github.com/prometheus-community/windows_exporter/internal/headers/hcs" "github.com/stretchr/testify/require" ) +// skipWithoutHCS skips the test if the Host Compute Service is not available, +// e.g. on hosts without the Containers feature. +func skipWithoutHCS(t *testing.T, err error) { + t.Helper() + + if errors.Is(err, hcs.ErrServiceNotAvailable) { + t.Skip("Host Compute Service is not available") + } +} + +func TestCreateOperation(t *testing.T) { + t.Parallel() + + operation, err := hcs.CreateOperation() + require.NoError(t, err) + require.NotZero(t, operation) + + hcs.CloseOperation(operation) +} + func TestGetContainers(t *testing.T) { t.Parallel() containers, err := hcs.GetContainers() + skipWithoutHCS(t, err) require.NoError(t, err) require.NotNil(t, containers) } @@ -36,6 +58,7 @@ func TestOpenContainer(t *testing.T) { t.Parallel() containers, err := hcs.GetContainers() + skipWithoutHCS(t, err) require.NoError(t, err) if len(containers) == 0 { @@ -51,5 +74,6 @@ func TestOpenContainerNotFound(t *testing.T) { t.Parallel() _, err := hcs.GetContainerStatistics("f3056b79b36ddfe203376473e2aeb4922a8ca7c5d8100764e5829eb5552fe09b") + skipWithoutHCS(t, err) require.ErrorIs(t, err, hcs.ErrIDNotFound) } diff --git a/internal/headers/hcs/syscall.go b/internal/headers/hcs/syscall.go index 71a95c75..04971521 100644 --- a/internal/headers/hcs/syscall.go +++ b/internal/headers/hcs/syscall.go @@ -18,6 +18,7 @@ package hcs import ( + "errors" "fmt" "unsafe" @@ -38,10 +39,18 @@ var ( ) // CreateOperation creates a new operation. +// +// HcsCreateOperation returns the operation handle, or NULL on failure. +// +// https://learn.microsoft.com/en-us/virtualization/api/hcs/reference/hcscreateoperation func CreateOperation() (Operation, error) { - r1, r2, _ := procHcsCreateOperation.Call(0, 0) - if r2 != 0 { - return 0, fmt.Errorf("HcsCreateOperation failed: HRESULT 0x%X: %w", r2, Win32FromHResult(r2)) + r1, _, err := procHcsCreateOperation.Call(0, 0) + if r1 == 0 { + if errors.Is(err, windows.ERROR_SUCCESS) { + err = ErrInvalidHandle + } + + return 0, fmt.Errorf("HcsCreateOperation failed: %w", err) } return Operation(r1), nil @@ -102,13 +111,13 @@ func OpenComputeSystem(id string) (ComputeSystem, error) { } func GetComputeSystemProperties(system ComputeSystem, operation Operation, propertyQuery *uint16) error { - r1, _, err := procHcsGetComputeSystemProperties.Call( + r1, _, _ := procHcsGetComputeSystemProperties.Call( uintptr(system), uintptr(operation), uintptr(unsafe.Pointer(propertyQuery)), ) if r1 != 0 { - return fmt.Errorf("HcsGetComputeSystemProperties failed: HRESULT 0x%X: %w", r1, err) + return fmt.Errorf("HcsGetComputeSystemProperties failed: HRESULT 0x%X: %w", r1, Win32FromHResult(r1)) } return nil diff --git a/internal/headers/hcs/types.go b/internal/headers/hcs/types.go index 31267696..fcdcc82c 100644 --- a/internal/headers/hcs/types.go +++ b/internal/headers/hcs/types.go @@ -26,7 +26,11 @@ import ( var ( ErrEmptyResultDocument = errors.New("empty result document") + ErrInvalidHandle = errors.New("invalid handle") ErrIDNotFound = windows.Errno(2151088398) + // ErrServiceNotAvailable is HCS_E_SERVICE_NOT_AVAILABLE, returned when the + // Host Compute Service is not installed, e.g. without the Containers feature. + ErrServiceNotAvailable = windows.Errno(2151088411) ) type ( diff --git a/internal/mi/application.go b/internal/mi/application.go index bddcca19..560bbe44 100644 --- a/internal/mi/application.go +++ b/internal/mi/application.go @@ -105,17 +105,20 @@ type DestinationOptionsFT struct { // // https://learn.microsoft.com/en-us/windows/win32/api/mi/nf-mi-mi_application_initializev1 func ApplicationInitialize() (*Application, error) { + if err := procMIApplicationInitialize.Find(); err != nil { + return nil, err + } + application := &Application{} - r0, _, err := procMIApplicationInitialize.Call( + // The status is the MI_Result return value. The thread's last error is + // unrelated and may be set even on success. + r0, _, _ := procMIApplicationInitialize.Call( 0, uintptr(unsafe.Pointer(applicationID)), 0, uintptr(unsafe.Pointer(application)), ) - if !errors.Is(err, windows.NOERROR) { - return nil, fmt.Errorf("syscall returned: %w", err) - } if result := ResultError(r0); !errors.Is(result, MI_RESULT_OK) { return nil, result @@ -168,10 +171,15 @@ func (application *Application) NewSession(options *DestinationOptions) (*Sessio defaultOperationOptions, err := application.NewOperationOptions() if err != nil { + _ = session.Close() + return nil, fmt.Errorf("failed to create default operation options: %w", err) } if err = defaultOperationOptions.SetTimeout(5 * time.Second); err != nil { + _ = defaultOperationOptions.Delete() + _ = session.Close() + return nil, fmt.Errorf("failed to set timeout: %w", err) } @@ -289,15 +297,19 @@ func (do *DestinationOptions) SetLocale(locale string) error { return nil } +// Delete deletes the destination options. MI_DestinationOptions_Delete +// returns void, so the only error is ErrNotInitialized. +// +// https://learn.microsoft.com/en-us/windows/win32/api/mi/nf-mi-mi_destinationoptions_delete func (do *DestinationOptions) Delete() error { - r0, _, _ := syscall.SyscallN( + if do == nil || do.ft == nil { + return ErrNotInitialized + } + + _, _, _ = syscall.SyscallN( do.ft.Delete, uintptr(unsafe.Pointer(do)), ) - if result := ResultError(r0); !errors.Is(result, MI_RESULT_OK) { - return result - } - return nil } diff --git a/internal/mi/mi_test.go b/internal/mi/mi_test.go index ee9b088e..43459d28 100644 --- a/internal/mi/mi_test.go +++ b/internal/mi/mi_test.go @@ -1009,3 +1009,73 @@ func Test_MI_Operation_CloseEarly(t *testing.T) { require.NoError(t, session.Query(&processes, mi.NamespaceRootCIMv2, query, 5*time.Second)) require.Equal(t, []win32Process{{Name: "System Idle Process"}}, processes) } + +// Test_MI_Query_StringArray reads a STRINGA element. Win32_OperatingSystem.MUILanguages +// is a string[] with at least the installed UI language. +func Test_MI_Query_StringArray(t *testing.T) { + application, err := mi.ApplicationInitialize() + require.NoError(t, err) + + t.Cleanup(func() { require.NoError(t, application.Close()) }) + + session, err := application.NewSession(nil) + require.NoError(t, err) + + t.Cleanup(func() { require.NoError(t, session.Close()) }) + + operation, err := session.QueryInstances(mi.OperationFlagsStandardRTTI, nil, mi.NamespaceRootCIMv2, mi.QueryDialectWQL, + "SELECT MUILanguages FROM Win32_OperatingSystem") + require.NoError(t, err) + + t.Cleanup(func() { require.NoError(t, operation.Close()) }) + + instance, _, err := operation.GetInstance() + require.NoError(t, err) + require.NotNil(t, instance) + + element, err := instance.GetElement("MUILanguages") + require.NoError(t, err) + + value, err := element.GetValue() + require.NoError(t, err) + + languages, ok := value.([]string) + require.True(t, ok, "expected []string, got %T", value) + require.NotEmpty(t, languages) + + for _, language := range languages { + require.Contains(t, language, "-", "unexpected language tag %q", language) + } +} + +// Test_MI_QueryUnmarshal_InvalidClass checks that a failing query is reported +// through the operation result, not the return value of the void +// MI_Session_QueryInstances, and that the session stays usable afterwards. +func Test_MI_QueryUnmarshal_InvalidClass(t *testing.T) { + application, err := mi.ApplicationInitialize() + require.NoError(t, err) + + t.Cleanup(func() { require.NoError(t, application.Close()) }) + + session, err := application.NewSession(nil) + require.NoError(t, err) + + t.Cleanup(func() { require.NoError(t, session.Close()) }) + + invalidQuery, err := mi.NewQuery("SELECT Name FROM Win32_DoesNotExist") + require.NoError(t, err) + + validQuery, err := mi.NewQuery("SELECT Name FROM Win32_Process WHERE Handle = 0") + require.NoError(t, err) + + for range 3 { + var processes []win32Process + + err = session.Query(&processes, mi.NamespaceRootCIMv2, invalidQuery, 5*time.Second) + require.ErrorIs(t, err, mi.MI_RESULT_INVALID_CLASS) + require.Empty(t, processes) + + require.NoError(t, session.Query(&processes, mi.NamespaceRootCIMv2, validQuery, 5*time.Second)) + require.Equal(t, []win32Process{{Name: "System Idle Process"}}, processes) + } +} diff --git a/internal/mi/operation.go b/internal/mi/operation.go index f6d5639f..ff2e10f8 100644 --- a/internal/mi/operation.go +++ b/internal/mi/operation.go @@ -331,30 +331,16 @@ func (o *OperationOptions) SetTimeout(timeout time.Duration) error { return nil } -func (o *OperationOptions) Close() error { - if o == nil || o.ft == nil { - return ErrNotInitialized - } - - r0, _, _ := syscall.SyscallN(o.ft.Clone, uintptr(unsafe.Pointer(o))) - - if result := ResultError(r0); !errors.Is(result, MI_RESULT_OK) { - return result - } - - return nil -} - +// Delete deletes the operation options. MI_OperationOptions_Delete returns +// void, so the only error is ErrNotInitialized. +// +// https://learn.microsoft.com/en-us/windows/win32/api/mi/nf-mi-mi_operationoptions_delete func (o *OperationOptions) Delete() error { if o == nil || o.ft == nil { return ErrNotInitialized } - r0, _, _ := syscall.SyscallN(o.ft.Delete, uintptr(unsafe.Pointer(o))) - - if result := ResultError(r0); !errors.Is(result, MI_RESULT_OK) { - return result - } + _, _, _ = syscall.SyscallN(o.ft.Delete, uintptr(unsafe.Pointer(o))) return nil } diff --git a/internal/mi/session.go b/internal/mi/session.go index 50c0f34c..cce450d6 100644 --- a/internal/mi/session.go +++ b/internal/mi/session.go @@ -98,8 +98,11 @@ func (s *Session) TestConnection() error { operation := &Operation{} + // MI_Session_TestConnection returns void. The outcome, e.g. a timeout, is + // reported by GetInstance, and the operation must be closed either way. + // // ref: https://github.com/KurtDeGreeff/omi/blob/9caa55032a1070a665e14fd282a091f6247d13c3/Unix/scriptext/py/PMI_Session.c#L92-L105 - r0, _, _ := syscall.SyscallN( + _, _, _ = syscall.SyscallN( s.ft.TestConnection, uintptr(unsafe.Pointer(s)), 0, @@ -107,18 +110,16 @@ func (s *Session) TestConnection() error { uintptr(unsafe.Pointer(operation)), ) - if result := ResultError(r0); !errors.Is(result, MI_RESULT_OK) { - return result + _, _, err := operation.GetInstance() + + if closeErr := operation.Close(); err == nil && closeErr != nil { + return fmt.Errorf("failed to close operation: %w", closeErr) } - if _, _, err := operation.GetInstance(); err != nil { + if err != nil { return fmt.Errorf("failed to get instance: %w", err) } - if err := operation.Close(); err != nil { - return fmt.Errorf("failed to close operation: %w", err) - } - return nil } @@ -160,21 +161,25 @@ func (s *Session) QueryInstances(flags OperationFlags, operationOptions *Operati return nil, err } - return s.queryInstances(flags, operationOptions, namespaceName, queryDialect, queryExpressionUTF16) + return s.queryInstances(flags, operationOptions, namespaceName, queryDialect, queryExpressionUTF16), nil } // queryInstances starts an MI_Session_QueryInstances operation. Nil operationOptions // fall back to the session defaults. The caller must close the returned operation. +// +// MI_Session_QueryInstances returns void: a failed query still yields an +// operation, and its status, e.g. a timeout or an invalid class, is reported +// by [Operation.GetInstance]. func (s *Session) queryInstances(flags OperationFlags, operationOptions *OperationOptions, namespaceName Namespace, queryDialect QueryDialect, queryExpression Query, -) (*Operation, error) { +) *Operation { operation := &Operation{} if operationOptions == nil { operationOptions = s.defaultOperationOptions } - r0, _, _ := syscall.SyscallN( + _, _, _ = syscall.SyscallN( s.ft.QueryInstances, uintptr(unsafe.Pointer(s)), uintptr(flags), @@ -186,11 +191,7 @@ func (s *Session) queryInstances(flags OperationFlags, operationOptions *Operati uintptr(unsafe.Pointer(operation)), ) - if result := ResultError(r0); !errors.Is(result, MI_RESULT_OK) { - return nil, result - } - - return operation, nil + return operation } // unmarshalInstance populates structValue from instance using the `mi` struct tags. @@ -368,10 +369,7 @@ func (s *Session) QueryUnmarshal[T any](dst *[]T, return err } - operation, err := s.queryInstances(flags, operationOptions, namespaceName, queryDialect, queryExpression) - if err != nil { - return fmt.Errorf("failed to query instances: %w", err) - } + operation := s.queryInstances(flags, operationOptions, namespaceName, queryDialect, queryExpression) defer func() { _ = operation.Close() @@ -420,10 +418,7 @@ func (s *Session) QueryFunc(namespaceName Namespace, queryExpression Query, quer }() } - operation, err := s.queryInstances(OperationFlagsStandardRTTI, operationOptions, namespaceName, QueryDialectWQL, queryExpression) - if err != nil { - return fmt.Errorf("failed to query instances: %w", err) - } + operation := s.queryInstances(OperationFlagsStandardRTTI, operationOptions, namespaceName, QueryDialectWQL, queryExpression) // Close cancels the operation if fn aborted it early. defer func() { diff --git a/internal/mi/value.go b/internal/mi/value.go index 2fed2c11..00059563 100644 --- a/internal/mi/value.go +++ b/internal/mi/value.go @@ -265,19 +265,7 @@ func (e *Element) GetValue() (any, error) { // Convert the UTF-16 string to a Go string return windows.UTF16PtrToString((*uint16)(unsafe.Pointer(e.value))), nil case ValueTypeSTRINGA: - if e.value == 0 { - return nil, errors.New("invalid pointer: value is nil") - } - - // Assuming array of pointers to UTF-16 strings - ptrArray := *(*[]*uint16)(unsafe.Pointer(e.value)) - strArray := make([]string, len(ptrArray)) - - for i, ptr := range ptrArray { - strArray[i] = windows.UTF16PtrToString(ptr) - } - - return strArray, nil + return e.getStringArray(), nil case ValueTypeUINT16A: return e.getUint16Array(), nil default: @@ -299,3 +287,21 @@ func (e *Element) getUint16Array() []uint16 { return out } + +// getStringArray reads a STRINGA element into a Go []string. The element's +// value holds the pointer to the array of MI_Char pointers and arrayLen its +// length. NULL entries become empty strings. +func (e *Element) getStringArray() []string { + if e.value == 0 || e.arrayLen == 0 { + return nil + } + + src := unsafe.Slice((**uint16)(unsafe.Pointer(e.value)), e.arrayLen) + + out := make([]string, e.arrayLen) + for i, ptr := range src { + out[i] = windows.UTF16PtrToString(ptr) + } + + return out +} diff --git a/internal/mi/value_internal_test.go b/internal/mi/value_internal_test.go index 98bea619..10613d0b 100644 --- a/internal/mi/value_internal_test.go +++ b/internal/mi/value_internal_test.go @@ -24,6 +24,7 @@ import ( "unsafe" "github.com/stretchr/testify/require" + "golang.org/x/sys/windows" ) // newElement builds an Element the way Instance.GetElement does, from a raw @@ -174,3 +175,49 @@ func TestElementIsNull(t *testing.T) { require.True(t, newElement(ValueTypeUINT32, [5]uint64{}, flagNull).IsNull()) require.False(t, newElement(ValueTypeUINT32, [5]uint64{}, 0).IsNull()) } + +// nativeAlloc returns zeroed memory outside the Go heap, like the buffers MI +// returns. The checkptr instrumentation enabled by -race rejects Go heap +// pointers that are passed through a uintptr, as the MI_Value buffer does. +func nativeAlloc(t *testing.T, size int) uintptr { + t.Helper() + + p, err := windows.LocalAlloc(windows.LPTR, uint32(size)) + require.NoError(t, err) + + t.Cleanup(func() { _, _ = windows.LocalFree(windows.Handle(p)) }) + + return p +} + +// nativeUTF16 copies s as a NUL-terminated UTF-16 string into native memory. +func nativeUTF16(t *testing.T, s string) uintptr { + t.Helper() + + u := windows.StringToUTF16(s) + p := nativeAlloc(t, len(u)*2) + copy(unsafe.Slice((*uint16)(unsafe.Pointer(p)), len(u)), u) + + return p +} + +func TestElementGetValueStringArray(t *testing.T) { + t.Parallel() + + strs := []uintptr{nativeUTF16(t, "en-US"), 0, nativeUTF16(t, "de-DE")} + + array := nativeAlloc(t, len(strs)*int(unsafe.Sizeof(uintptr(0)))) + copy(unsafe.Slice((*uintptr)(unsafe.Pointer(array)), len(strs)), strs) + + // An MI_StringA is the pointer to the MI_Char* array in word 0 and the + // element count in word 1. + raw := [5]uint64{uint64(array), uint64(len(strs))} + + got, err := newElement(ValueTypeSTRINGA, raw, 0).GetValue() + require.NoError(t, err) + require.Equal(t, []string{"en-US", "", "de-DE"}, got) + + got, err = newElement(ValueTypeSTRINGA, [5]uint64{}, flagNull).GetValue() + require.NoError(t, err) + require.Nil(t, got) +}