diff --git a/internal/collector/netframework/netframework_clrjit.go b/internal/collector/netframework/netframework_clrjit.go index d05345f7..6377c129 100644 --- a/internal/collector/netframework/netframework_clrjit.go +++ b/internal/collector/netframework/netframework_clrjit.go @@ -57,7 +57,7 @@ func (c *Collector) buildClrJIT() { type Win32_PerfRawData_NETFramework_NETCLRJit struct { Name string `mi:"Name"` - Frequency_PerfTime uint32 `mi:"Frequency_PerfTime"` + Frequency_PerfTime uint64 `mi:"Frequency_PerfTime"` ILBytesJittedPersec uint32 `mi:"ILBytesJittedPersec"` NumberofILBytesJitted uint32 `mi:"NumberofILBytesJitted"` NumberofMethodsJitted uint32 `mi:"NumberofMethodsJitted"` diff --git a/internal/collector/netframework/netframework_clrsecurity.go b/internal/collector/netframework/netframework_clrsecurity.go index a25f7629..87e33036 100644 --- a/internal/collector/netframework/netframework_clrsecurity.go +++ b/internal/collector/netframework/netframework_clrsecurity.go @@ -57,7 +57,7 @@ func (c *Collector) buildClrSecurity() { type Win32_PerfRawData_NETFramework_NETCLRSecurity struct { Name string `mi:"Name"` - Frequency_PerfTime uint32 `mi:"Frequency_PerfTime"` + Frequency_PerfTime uint64 `mi:"Frequency_PerfTime"` NumberLinkTimeChecks uint32 `mi:"NumberLinkTimeChecks"` PercentTimeinRTchecks uint32 `mi:"PercentTimeinRTchecks"` PercentTimeSigAuthenticating uint64 `mi:"PercentTimeSigAuthenticating"` diff --git a/internal/mi/char16_test.go b/internal/mi/char16_test.go new file mode 100644 index 00000000..ecad280a --- /dev/null +++ b/internal/mi/char16_test.go @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +package mi + +import ( + "testing" +) + +// TestGetValue_CHAR16 verifies that GetValue treats CHAR16 as a scalar +// (returning the code-unit directly as uint16) rather than dereferencing +// element.value as a string pointer, which would crash. +func TestGetValue_CHAR16(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + code uintptr + want uint16 + }{ + {"letter_A", 'A', 'A'}, + {"digit_0", '0', '0'}, + {"space", ' ', ' '}, + {"euro_sign", 0x20AC, 0x20AC}, + {"null", 0, 0}, + {"max_bmp", 0xFFFF, 0xFFFF}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + e := Element{ + value: tt.code, + valueType: ValueTypeCHAR16, + } + + got, err := e.GetValue() + if err != nil { + t.Fatalf("GetValue() error: %v", err) + } + + v, ok := got.(uint16) + if !ok { + t.Fatalf("GetValue() returned %T, want uint16", got) + } + + if v != tt.want { + t.Errorf("GetValue() = 0x%04X (%c), want 0x%04X (%c)", v, v, tt.want, tt.want) + } + }) + } +} + +// Note: The CHAR16 unmarshal branch in unmarshalInstance (session.go) handles +// string and uint fields. Integration testing of this path requires a WMI class +// with a CHAR16 property, which is extremely rare. The setUintField and +// setIntField helpers used by the branch are covered by the signed/unsigned +// integer tests (Test_MI_QueryUnmarshal_SignedInt, Test_MI_Unmarshal_TypeMismatch). diff --git a/internal/mi/mi_test.go b/internal/mi/mi_test.go index a9b9c105..5ae9616f 100644 --- a/internal/mi/mi_test.go +++ b/internal/mi/mi_test.go @@ -18,6 +18,7 @@ package mi_test import ( + "errors" "testing" "time" @@ -296,6 +297,414 @@ func Test_MI_FD_Leak(t *testing.T) { require.NoError(t, application.Close()) } +type reliabilityMetrics struct { + SystemStabilityIndex float64 `mi:"SystemStabilityIndex"` +} + +// Test_MI_Query_REAL64 verifies that GetValue correctly returns float64 +// for REAL64 MI properties by querying Win32_ReliabilityStabilityMetrics. +func Test_MI_Query_REAL64(t *testing.T) { + application, err := mi.ApplicationInitialize() + require.NoError(t, err) + require.NotEmpty(t, application) + + t.Cleanup(func() { _ = application.Close() }) + + destinationOptions, err := application.NewDestinationOptions() + require.NoError(t, err) + require.NotEmpty(t, destinationOptions) + + err = destinationOptions.SetTimeout(5 * time.Second) + require.NoError(t, err) + + err = destinationOptions.SetLocale(mi.LocaleEnglish) + require.NoError(t, err) + + session, err := application.NewSession(destinationOptions) + require.NoError(t, err) + require.NotEmpty(t, session) + + t.Cleanup(func() { _ = session.Close() }) + + operation, err := session.QueryInstances(mi.OperationFlagsStandardRTTI, nil, mi.NamespaceRootCIMv2, mi.QueryDialectWQL, + "SELECT SystemStabilityIndex FROM Win32_ReliabilityStabilityMetrics") + if err != nil { + if errors.Is(err, mi.MI_RESULT_INVALID_CLASS) { + t.Skip("Win32_ReliabilityStabilityMetrics class not available on this system") + } + + require.NoError(t, err) + } + + require.NotEmpty(t, operation) + + t.Cleanup(func() { _ = operation.Close() }) + + var firstValue float64 + + var foundFloat bool + + for { + instance, moreResults, err := operation.GetInstance() + if err != nil { + if errors.Is(err, mi.MI_RESULT_INVALID_CLASS) { + t.Skip("Win32_ReliabilityStabilityMetrics class not available on this system") + } + + require.NoError(t, err) + } + + if instance == nil { + break + } + + if !foundFloat { + element, err := instance.GetElement("SystemStabilityIndex") + require.NoError(t, err) + + value, err := element.GetValue() + require.NoError(t, err) + + v, ok := value.(float64) + require.True(t, ok, "expected float64, got %T", value) + + if v > 0 { + firstValue = v + foundFloat = true + } + } + + if !moreResults { + break + } + } + + if !foundFloat { + t.Skip("Win32_ReliabilityStabilityMetrics: no records with non-zero SystemStabilityIndex") + } + + require.Greater(t, firstValue, float64(0), "SystemStabilityIndex should be positive") + require.LessOrEqual(t, firstValue, float64(10), "SystemStabilityIndex should be at most 10 (documented maximum)") + + t.Logf("SystemStabilityIndex = %v", firstValue) +} + +// Test_MI_QueryUnmarshal_REAL64 verifies that the unmarshal code path +// correctly handles REAL64→float64 struct field conversion. +func Test_MI_QueryUnmarshal_REAL64(t *testing.T) { + application, err := mi.ApplicationInitialize() + require.NoError(t, err) + require.NotEmpty(t, application) + + t.Cleanup(func() { _ = application.Close() }) + + destinationOptions, err := application.NewDestinationOptions() + require.NoError(t, err) + require.NotEmpty(t, destinationOptions) + + err = destinationOptions.SetTimeout(5 * time.Second) + require.NoError(t, err) + + err = destinationOptions.SetLocale(mi.LocaleEnglish) + require.NoError(t, err) + + session, err := application.NewSession(destinationOptions) + require.NoError(t, err) + require.NotEmpty(t, session) + + t.Cleanup(func() { _ = session.Close() }) + + var metrics []reliabilityMetrics + + query, err := mi.NewQuery("SELECT SystemStabilityIndex FROM Win32_ReliabilityStabilityMetrics") + require.NoError(t, err) + + err = session.QueryUnmarshal(&metrics, mi.OperationFlagsStandardRTTI, nil, mi.NamespaceRootCIMv2, mi.QueryDialectWQL, query) + if err != nil { + if errors.Is(err, mi.MI_RESULT_INVALID_CLASS) { + t.Skip("Win32_ReliabilityStabilityMetrics class not available on this system") + } + + require.NoError(t, err) + } + + if len(metrics) == 0 { + t.Skip("Win32_ReliabilityStabilityMetrics returned no records") + } + + var found bool + + for _, m := range metrics { + if m.SystemStabilityIndex > 0 { + require.LessOrEqual(t, m.SystemStabilityIndex, float64(10), "SystemStabilityIndex should be at most 10 (documented maximum)") + + t.Logf("SystemStabilityIndex = %.3f (from %d records)", m.SystemStabilityIndex, len(metrics)) + + found = true + + break + } + } + + if !found { + t.Skip("Win32_ReliabilityStabilityMetrics: no records with non-zero SystemStabilityIndex") + } +} + +type computerSystemSigned struct { + ResetCount int16 `mi:"ResetCount"` + ResetLimit int16 `mi:"ResetLimit"` + PauseAfterReset int64 `mi:"PauseAfterReset"` +} + +// computerSystemWide unmarshals SINT16 properties into int64 fields +// to verify sign extension works correctly across type widths. +// With the old (broken) code, a SINT16 value of -1 (0xFFFF) would +// become 65535 in an int64 field instead of -1. +type computerSystemWide struct { + ResetCount int64 `mi:"ResetCount"` + ResetLimit int64 `mi:"ResetLimit"` +} + +// Test_MI_Query_SignedInt verifies that GetValue correctly sign-extends +// SINT8/SINT16/SINT32 values. Win32_ComputerSystem has SInt16 properties +// (ResetCount, ResetLimit) that are typically -1, meaning "not supported". +func Test_MI_Query_SignedInt(t *testing.T) { + application, err := mi.ApplicationInitialize() + require.NoError(t, err) + require.NotEmpty(t, application) + + t.Cleanup(func() { _ = application.Close() }) + + destinationOptions, err := application.NewDestinationOptions() + require.NoError(t, err) + require.NotEmpty(t, destinationOptions) + + err = destinationOptions.SetTimeout(5 * time.Second) + require.NoError(t, err) + + err = destinationOptions.SetLocale(mi.LocaleEnglish) + require.NoError(t, err) + + session, err := application.NewSession(destinationOptions) + require.NoError(t, err) + require.NotEmpty(t, session) + + t.Cleanup(func() { _ = session.Close() }) + + operation, err := session.QueryInstances(mi.OperationFlagsStandardRTTI, nil, mi.NamespaceRootCIMv2, mi.QueryDialectWQL, + "SELECT ResetCount, ResetLimit, PauseAfterReset FROM Win32_ComputerSystem") + require.NoError(t, err) + require.NotEmpty(t, operation) + + t.Cleanup(func() { _ = operation.Close() }) + + instance, moreResults, err := operation.GetInstance() + require.NoError(t, err) + require.NotEmpty(t, instance) + require.False(t, moreResults) + + // ResetCount (SInt16): verify the value is returned as int16 (not uint16 or int64). + // The value is typically -1 ("not supported") but the exact value is system-dependent. + element, err := instance.GetElement("ResetCount") + require.NoError(t, err) + + value, err := element.GetValue() + require.NoError(t, err) + + resetCount, ok := value.(int16) + require.True(t, ok, "expected int16, got %T", value) + + t.Logf("ResetCount = %d, ResetLimit = (same type)", resetCount) +} + +// Test_MI_QueryUnmarshal_SignedInt verifies that the unmarshal code path +// correctly sign-extends SINT16 values into Go struct fields. +func Test_MI_QueryUnmarshal_SignedInt(t *testing.T) { + application, err := mi.ApplicationInitialize() + require.NoError(t, err) + require.NotEmpty(t, application) + + t.Cleanup(func() { _ = application.Close() }) + + destinationOptions, err := application.NewDestinationOptions() + require.NoError(t, err) + require.NotEmpty(t, destinationOptions) + + err = destinationOptions.SetTimeout(5 * time.Second) + require.NoError(t, err) + + err = destinationOptions.SetLocale(mi.LocaleEnglish) + require.NoError(t, err) + + session, err := application.NewSession(destinationOptions) + require.NoError(t, err) + require.NotEmpty(t, session) + + t.Cleanup(func() { _ = session.Close() }) + + var systems []computerSystemSigned + + query, err := mi.NewQuery("SELECT ResetCount, ResetLimit, PauseAfterReset FROM Win32_ComputerSystem") + require.NoError(t, err) + + err = session.QueryUnmarshal(&systems, mi.OperationFlagsStandardRTTI, nil, mi.NamespaceRootCIMv2, mi.QueryDialectWQL, query) + require.NoError(t, err) + require.Len(t, systems, 1) + + // Verify that signed fields were populated without error. The actual values + // are system-dependent (typically -1), so we only check that unmarshalling + // succeeded and log the values for manual inspection. + s := systems[0] + + t.Logf("ResetCount=%d ResetLimit=%d PauseAfterReset=%d", s.ResetCount, s.ResetLimit, s.PauseAfterReset) +} + +// Test_MI_QueryUnmarshal_SignedInt_Wide verifies that the sign-extension fix +// works when a SINT16 value is unmarshalled into a wider Go field (int64). +// Before the fix, -1 (0xFFFF) would become 65535 in an int64 field. +func Test_MI_QueryUnmarshal_SignedInt_Wide(t *testing.T) { + application, err := mi.ApplicationInitialize() + require.NoError(t, err) + require.NotEmpty(t, application) + + t.Cleanup(func() { _ = application.Close() }) + + destinationOptions, err := application.NewDestinationOptions() + require.NoError(t, err) + require.NotEmpty(t, destinationOptions) + + err = destinationOptions.SetTimeout(5 * time.Second) + require.NoError(t, err) + + err = destinationOptions.SetLocale(mi.LocaleEnglish) + require.NoError(t, err) + + session, err := application.NewSession(destinationOptions) + require.NoError(t, err) + require.NotEmpty(t, session) + + t.Cleanup(func() { _ = session.Close() }) + + // First, read the actual values with the narrow type to learn what to expect. + var narrow []computerSystemSigned + + narrowQuery, err := mi.NewQuery("SELECT ResetCount, ResetLimit FROM Win32_ComputerSystem") + require.NoError(t, err) + + err = session.QueryUnmarshal(&narrow, mi.OperationFlagsStandardRTTI, nil, mi.NamespaceRootCIMv2, mi.QueryDialectWQL, narrowQuery) + require.NoError(t, err) + require.Len(t, narrow, 1) + + if narrow[0].ResetCount >= 0 { + t.Skipf("ResetCount is %d (non-negative); cannot verify sign extension into wider type", narrow[0].ResetCount) + } + + // Now unmarshal the same SINT16 values into int64 fields. + var wide []computerSystemWide + + wideQuery, err := mi.NewQuery("SELECT ResetCount, ResetLimit FROM Win32_ComputerSystem") + require.NoError(t, err) + + err = session.QueryUnmarshal(&wide, mi.OperationFlagsStandardRTTI, nil, mi.NamespaceRootCIMv2, mi.QueryDialectWQL, wideQuery) + require.NoError(t, err) + require.Len(t, wide, 1) + + // The key assertion: with the old code, ResetCount = -1 would become 65535 + // in the int64 field. With the fix, it must remain negative. + require.Negative(t, wide[0].ResetCount, + "SINT16 value %d should remain negative when unmarshalled into int64 (got %d)", + narrow[0].ResetCount, wide[0].ResetCount) + require.Equal(t, int64(narrow[0].ResetCount), wide[0].ResetCount, + "int64 field should match int16 value after sign extension") + + t.Logf("ResetCount: int16=%d, int64=%d (sign extension correct)", narrow[0].ResetCount, wide[0].ResetCount) +} + +// Test_MI_Unmarshal_TypeMismatch verifies that unmarshalInstance rejects +// Go struct fields whose kind does not match the MI value type. +func Test_MI_Unmarshal_TypeMismatch(t *testing.T) { + application, err := mi.ApplicationInitialize() + require.NoError(t, err) + + t.Cleanup(func() { _ = application.Close() }) + + session, err := application.NewSession(nil) + require.NoError(t, err) + + t.Cleanup(func() { _ = session.Close() }) + + // Map a string MI property (Name) to an int Go field → type error. + t.Run("string_to_int", func(t *testing.T) { + type bad struct { + Name int `mi:"Name"` + } + + var dst []bad + + query, err := mi.NewQuery("SELECT Name FROM Win32_Process WHERE Handle = 0") + require.NoError(t, err) + + err = session.QueryUnmarshal(&dst, mi.OperationFlagsStandardRTTI, nil, mi.NamespaceRootCIMv2, mi.QueryDialectWQL, query) + require.Error(t, err) + require.Contains(t, err.Error(), "Name") + + t.Logf("got expected error: %v", err) + }) + + // Map a uint32 MI property (ProcessId) to a bool Go field → type error. + t.Run("uint_to_bool", func(t *testing.T) { + type bad struct { + ProcessId bool `mi:"ProcessId"` + } + + var dst []bad + + query, err := mi.NewQuery("SELECT ProcessId FROM Win32_Process WHERE Handle = 0") + require.NoError(t, err) + + err = session.QueryUnmarshal(&dst, mi.OperationFlagsStandardRTTI, nil, mi.NamespaceRootCIMv2, mi.QueryDialectWQL, query) + require.Error(t, err) + require.Contains(t, err.Error(), "ProcessId") + + t.Logf("got expected error: %v", err) + }) + + // Map a SInt16 MI property to an unsigned Go field. If the value is + // negative (e.g. -1 = "not supported") this must error; if it is + // non-negative and fits uint16 it is accepted. Skip when non-negative. + t.Run("negative_sint_to_uint", func(t *testing.T) { + // First, read the actual value to decide whether to test or skip. + var probe []computerSystemSigned + + probeQuery, err := mi.NewQuery("SELECT ResetCount FROM Win32_ComputerSystem") + require.NoError(t, err) + + err = session.QueryUnmarshal(&probe, mi.OperationFlagsStandardRTTI, nil, mi.NamespaceRootCIMv2, mi.QueryDialectWQL, probeQuery) + require.NoError(t, err) + require.Len(t, probe, 1) + + if probe[0].ResetCount >= 0 { + t.Skipf("ResetCount is %d (non-negative); cannot test negative→uint rejection", probe[0].ResetCount) + } + + type bad struct { + ResetCount uint16 `mi:"ResetCount"` + } + + var dst []bad + + query, err := mi.NewQuery("SELECT ResetCount FROM Win32_ComputerSystem") + require.NoError(t, err) + + err = session.QueryUnmarshal(&dst, mi.OperationFlagsStandardRTTI, nil, mi.NamespaceRootCIMv2, mi.QueryDialectWQL, query) + require.Error(t, err) + require.Contains(t, err.Error(), "ResetCount") + + t.Logf("got expected error: %v", err) + }) +} + func Test_MI_QueryTimeout(t *testing.T) { application, err := mi.ApplicationInitialize() require.NoError(t, err) diff --git a/internal/mi/operation.go b/internal/mi/operation.go index f23bd9e2..e8d65087 100644 --- a/internal/mi/operation.go +++ b/internal/mi/operation.go @@ -201,6 +201,12 @@ func (o *Operation) GetInstance() (*Instance, bool, error) { } func (o *Operation) Unmarshal(dst any) error { + return o.unmarshal(dst, false) +} + +// unmarshal iterates over the operation's instances and populates dst. +// skipMissing controls how missing elements are handled (see unmarshalInstance). +func (o *Operation) unmarshal(dst any, skipMissing bool) error { if o == nil || o.ft == nil { return ErrNotInitialized } @@ -213,7 +219,6 @@ func (o *Operation) Unmarshal(dst any) error { dv = dv.Elem() elemType := dv.Type().Elem() - elemValue := reflect.ValueOf(reflect.New(elemType).Interface()).Elem() if dv.Kind() != reflect.Slice || elemType.Kind() != reflect.Struct { return ErrInvalidEntityType @@ -241,45 +246,10 @@ func (o *Operation) Unmarshal(dst any) error { break } - for i := range elemType.NumField() { - field := elemValue.Field(i) + elemValue := reflect.New(elemType).Elem() - // Check if the field has an `mi` tag - miTag := elemType.Field(i).Tag.Get("mi") - if miTag == "" { - continue - } - - element, err := instance.GetElement(miTag) - if err != nil { - return fmt.Errorf("failed to get element %s: %w", miTag, err) - } - - switch element.valueType { - case ValueTypeBOOLEAN: - field.SetBool(element.value == 1) - case ValueTypeUINT8, ValueTypeUINT16, ValueTypeUINT32, ValueTypeUINT64: - field.SetUint(uint64(element.value)) - case ValueTypeSINT8, ValueTypeSINT16, ValueTypeSINT32, ValueTypeSINT64: - field.SetInt(int64(element.value)) - case ValueTypeSTRING: - if element.value == 0 { - field.SetString("") // Set empty string for nil values - - continue - } - - // Convert uintptr to *uint16 for Windows UTF-16 string - // This is safe because element.value comes directly from Windows MI API - //goland:noinspection GoVetUnsafePointer - stringValue := windows.UTF16PtrToString((*uint16)(unsafe.Pointer(element.value))) - - field.SetString(stringValue) - case ValueTypeREAL32, ValueTypeREAL64: - field.SetFloat(float64(element.value)) - default: - return fmt.Errorf("unsupported value type: %d", element.valueType) - } + if err := unmarshalInstance(instance, elemType, elemValue, skipMissing); err != nil { + return err } dv.Set(reflect.Append(dv, elemValue)) diff --git a/internal/mi/session.go b/internal/mi/session.go index 7bb4f46a..6e2a4a05 100644 --- a/internal/mi/session.go +++ b/internal/mi/session.go @@ -20,6 +20,7 @@ package mi import ( "errors" "fmt" + "math" "reflect" "syscall" "time" @@ -184,6 +185,176 @@ func (s *Session) QueryInstances(flags OperationFlags, operationOptions *Operati return operation, nil } +// unmarshalInstance populates structValue from instance using the `mi` struct tags. +// +// skipMissing controls what happens when the instance has no element for a tagged field. +// QueryUnmarshal passes true, tolerating classes whose MOF gained fields the query did not +// select. Operation.Unmarshal passes false, so a missing element is reported as an error +// instead of being silently left at its zero value. +func unmarshalInstance(instance *Instance, structType reflect.Type, structValue reflect.Value, skipMissing bool) error { + for i := range structType.NumField() { + field := structValue.Field(i) + + miTag := structType.Field(i).Tag.Get("mi") + if miTag == "" { + continue + } + + element, err := instance.GetElement(miTag) + if err != nil { + if skipMissing && errors.Is(err, MI_RESULT_NO_SUCH_PROPERTY) { + continue + } + + return fmt.Errorf("failed to get element %s: %w", miTag, err) + } + + switch element.valueType { + case ValueTypeBOOLEAN: + if field.Kind() != reflect.Bool { + return fieldTypeError(miTag, field, "boolean") + } + + field.SetBool(element.value == 1) + case ValueTypeUINT8, ValueTypeUINT16, ValueTypeUINT32, ValueTypeUINT64: + if err := setUintField(miTag, field, uint64(element.value)); err != nil { + return err + } + case ValueTypeSINT8: + if err := setIntField(miTag, field, int64(int8(element.value))); err != nil { + return err + } + case ValueTypeSINT16: + if err := setIntField(miTag, field, int64(int16(element.value))); err != nil { + return err + } + case ValueTypeSINT32: + if err := setIntField(miTag, field, int64(int32(element.value))); err != nil { + return err + } + case ValueTypeSINT64: + if err := setIntField(miTag, field, int64(element.value)); err != nil { + return err + } + case ValueTypeSTRING: + if field.Kind() != reflect.String { + return fieldTypeError(miTag, field, "string") + } + + if element.value == 0 { + continue + } + + stringValue := windows.UTF16PtrToString((*uint16)(unsafe.Pointer(element.value))) + + field.SetString(stringValue) + case ValueTypeCHAR16: + // A CHAR16 is a single UTF-16 code unit, not a pointer to a string. + // Note: lone UTF-16 surrogates (0xD800–0xDFFF) are replaced with U+FFFD + // by string(rune(...)). Use a uint16 Go field when the raw code unit is needed. + if field.Kind() == reflect.String { + field.SetString(string(rune(element.value))) + + continue + } + + if err := setUintField(miTag, field, uint64(element.value)); err != nil { + return err + } + case ValueTypeREAL32: + if field.Kind() != reflect.Float32 && field.Kind() != reflect.Float64 { + return fieldTypeError(miTag, field, "float") + } + + field.SetFloat(float64(math.Float32frombits(uint32(element.value)))) + case ValueTypeREAL64: + if field.Kind() != reflect.Float64 { + return fieldTypeError(miTag, field, "float") + } + + field.SetFloat(math.Float64frombits(uint64(element.value))) + case ValueTypeUINT16A: + if field.Type() != reflect.TypeFor[[]uint16]() { + return fmt.Errorf("cannot unmarshal UINT16A into field of type %s, expected []uint16", field.Type()) + } + + field.Set(reflect.ValueOf(element.getUint16Array())) + default: + return fmt.Errorf("unsupported value type: %d", element.valueType) + } + } + + return nil +} + +// setUintField assigns value to a numeric field, rejecting fields that cannot hold it +// instead of letting the reflect package panic or silently wrap. +func setUintField(miTag string, field reflect.Value, value uint64) error { + if !isUnsignedKind(field.Kind()) { + return fieldTypeError(miTag, field, "unsigned integer") + } + + if field.OverflowUint(value) { + return fmt.Errorf("field %s of Go type %s cannot hold the MI unsigned value %d", miTag, field.Type(), value) + } + + field.SetUint(value) + + return nil +} + +// setIntField assigns value to a numeric field, rejecting fields that cannot hold it. +// If the Go field is unsigned, non-negative values that fit are accepted; negative +// values are rejected with an error rather than silently wrapping. +func setIntField(miTag string, field reflect.Value, value int64) error { + if !isNumericKind(field.Kind()) { + return fieldTypeError(miTag, field, "signed integer") + } + + if isUnsignedKind(field.Kind()) { + if value < 0 || field.OverflowUint(uint64(value)) { + return fmt.Errorf("field %s of Go type %s cannot hold the MI signed value %d", miTag, field.Type(), value) + } + + field.SetUint(uint64(value)) + + return nil + } + + if field.OverflowInt(value) { + return fmt.Errorf("field %s of Go type %s cannot hold the MI signed value %d", miTag, field.Type(), value) + } + + field.SetInt(value) + + return nil +} + +func isNumericKind(k reflect.Kind) bool { + //nolint:exhaustive // We only care about fixed-width integer kinds. + switch k { + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + return true + default: + return false + } +} + +func isUnsignedKind(k reflect.Kind) bool { + //nolint:exhaustive // We only care about unsigned integer kinds. + switch k { + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + return true + default: + return false + } +} + +func fieldTypeError(miTag string, field reflect.Value, miType string) error { + return fmt.Errorf("field %s is of Go type %s but the MI value is a %s", miTag, field.Type(), miType) +} + // QueryUnmarshal queries for a set of instances based on a query expression. // // https://learn.microsoft.com/en-us/windows/win32/api/mi/nf-mi-mi_session_queryinstances @@ -201,22 +372,6 @@ func (s *Session) QueryUnmarshal(dst any, operationOptions = s.defaultOperationOptions } - dv := reflect.ValueOf(dst) - if dv.Kind() != reflect.Pointer || dv.IsNil() { - return ErrInvalidEntityType - } - - dv = dv.Elem() - - elemType := dv.Type().Elem() - elemValue := reflect.ValueOf(reflect.New(elemType).Interface()).Elem() - - if dv.Kind() != reflect.Slice || elemType.Kind() != reflect.Struct { - return ErrInvalidEntityType - } - - dv.Set(reflect.MakeSlice(dv.Type(), 0, 0)) - r0, _, _ := syscall.SyscallN( s.ft.QueryInstances, uintptr(unsafe.Pointer(s)), @@ -237,104 +392,32 @@ func (s *Session) QueryUnmarshal(dst any, _ = operation.Close() }() - for { - instance, moreResults, err := operation.GetInstance() - if err != nil { - return fmt.Errorf("failed to get instance: %w", err) - } - - if instance == nil { - break - } - - counter, err := instance.GetElementCount() - if err != nil { - return fmt.Errorf("failed to get element count: %w", err) - } - - if counter == 0 { - break - } - - for i := range elemType.NumField() { - field := elemValue.Field(i) - - // Check if the field has an `mi` tag - miTag := elemType.Field(i).Tag.Get("mi") - if miTag == "" { - continue - } - - element, err := instance.GetElement(miTag) - if err != nil { - if errors.Is(err, MI_RESULT_NO_SUCH_PROPERTY) { - continue - } - - return fmt.Errorf("failed to get element %s: %w", miTag, err) - } - - switch element.valueType { - case ValueTypeBOOLEAN: - field.SetBool(element.value == 1) - case ValueTypeUINT8, ValueTypeUINT16, ValueTypeUINT32, ValueTypeUINT64: - field.SetUint(uint64(element.value)) - case ValueTypeSINT8, ValueTypeSINT16, ValueTypeSINT32, ValueTypeSINT64: - field.SetInt(int64(element.value)) - case ValueTypeSTRING: - if element.value == 0 { - // value is null - continue - } - - // Convert the UTF-16 string to a Go string - stringValue := windows.UTF16PtrToString((*uint16)(unsafe.Pointer(element.value))) - - field.SetString(stringValue) - case ValueTypeREAL32, ValueTypeREAL64: - field.SetFloat(float64(element.value)) - case ValueTypeUINT16A: - if field.Type() != reflect.TypeFor[[]uint16]() { - return fmt.Errorf("cannot unmarshal UINT16A into field of type %s, expected []uint16", field.Type()) - } - - field.Set(reflect.ValueOf(element.getUint16Array())) - default: - return fmt.Errorf("unsupported value type: %d", element.valueType) - } - } - - dv.Set(reflect.Append(dv, elemValue)) - - if !moreResults { - break - } - } - - return nil + return operation.unmarshal(dst, true) } // Query queries for a set of instances based on a query expression. -// -//nolint:nestif func (s *Session) Query(dst any, namespaceName Namespace, queryExpression Query, queryTimeout time.Duration) error { - var operationOptions *OperationOptions + if queryTimeout < 0 { + return s.QueryUnmarshal(dst, OperationFlagsStandardRTTI, nil, namespaceName, QueryDialectWQL, queryExpression) + } - if queryTimeout >= 0 { - app, err := s.GetApplication() - if err != nil { - return fmt.Errorf("failed to get application: %w", err) - } + app, err := s.GetApplication() + if err != nil { + return fmt.Errorf("failed to get application: %w", err) + } - operationOptions, err = app.NewOperationOptions() - if err != nil { - return fmt.Errorf("failed to create operation options: %w", err) - } + operationOptions, err := app.NewOperationOptions() + if err != nil { + return fmt.Errorf("failed to create operation options: %w", err) + } - if queryTimeout > 0 { - if err = operationOptions.SetTimeout(queryTimeout); err != nil { - return fmt.Errorf("failed to set timeout: %w", err) - } + defer func() { + _ = operationOptions.Delete() + }() + + if queryTimeout > 0 { + if err = operationOptions.SetTimeout(queryTimeout); err != nil { + return fmt.Errorf("failed to set timeout: %w", err) } } diff --git a/internal/mi/value.go b/internal/mi/value.go index faa91d0f..b0a56a87 100644 --- a/internal/mi/value.go +++ b/internal/mi/value.go @@ -20,6 +20,7 @@ package mi import ( "errors" "fmt" + "math" "unsafe" "golang.org/x/sys/windows" @@ -91,9 +92,9 @@ func (e *Element) GetValue() (any, error) { case ValueTypeSINT64: return int64(e.value), nil case ValueTypeREAL32: - return float32(e.value), nil + return math.Float32frombits(uint32(e.value)), nil case ValueTypeREAL64: - return float64(e.value), nil + return math.Float64frombits(uint64(e.value)), nil case ValueTypeCHAR16: return uint16(e.value), nil case ValueTypeDATETIME: diff --git a/internal/utils/testutils/testutils.go b/internal/utils/testutils/testutils.go index 9f8fdf46..a4e2a51e 100644 --- a/internal/utils/testutils/testutils.go +++ b/internal/utils/testutils/testutils.go @@ -92,7 +92,7 @@ func TestCollector[C collector.Collector, V any](t *testing.T, fn func(*V) C, co t.Cleanup(func() { assert.NoError(t, c.Close()) }) if err := c.Build(logger, miSession); err != nil { - if !required && unsupportedCollector(err) { + if !required && unsupportedBuild(err) { t.Skipf("collector %s is not supported: %v", c.GetName(), err) } @@ -111,7 +111,27 @@ func TestCollector[C collector.Collector, V any](t *testing.T, fn func(*V) C, co return families } -func unsupportedCollector(err error) bool { +// unsupportedBuild returns true for errors that indicate a collector's WMI +// class or PDH counter is not present on this system. MI_RESULT_INVALID_CLASS +// is only accepted here (Build path) so that a mistyped class in production +// code still fails Collect. +func unsupportedBuild(err error) bool { + return errors.Is(err, mi.MI_RESULT_INVALID_CLASS) || + errors.Is(err, mi.MI_RESULT_INVALID_NAMESPACE) || + errors.Is(err, mi.MI_RESULT_INVALID_QUERY) || + errors.Is(err, pdh.NewPdhError(pdh.CstatusNoCounter)) || + errors.Is(err, pdh.NewPdhError(pdh.CstatusNoObject)) || + errors.Is(err, pdh.ErrPerformanceCounterNotInitialized) || + errors.Is(err, pdh.ErrNoData) || + errors.Is(err, update.ErrUpdateServiceDisabled) || + errors.Is(err, os.ErrNotExist) || + errors.Is(err, windows.Errno(2151088411)) +} + +// unsupportedCollect returns true for Collect-path errors that indicate a +// collector cannot run on this system. MI_RESULT_INVALID_CLASS is deliberately +// excluded: a class missing at collect time is a real bug, not an expected skip. +func unsupportedCollect(err error) bool { return errors.Is(err, mi.MI_RESULT_INVALID_NAMESPACE) || errors.Is(err, mi.MI_RESULT_INVALID_QUERY) || errors.Is(err, pdh.NewPdhError(pdh.CstatusNoCounter)) || @@ -167,7 +187,7 @@ func collectMetrics(t *testing.T, c collector.Collector, required bool) map[stri collectErr = nil } - if !required && (unsupportedCollector(collectErr) || errors.Is(collectErr, update.ErrNoUpdates)) { + if !required && (unsupportedCollect(collectErr) || errors.Is(collectErr, update.ErrNoUpdates)) { t.Skipf("collector %s is not supported: %v", c.GetName(), collectErr) } diff --git a/pkg/collector/collection.go b/pkg/collector/collection.go index e8a6c5d6..6bb998ce 100644 --- a/pkg/collector/collection.go +++ b/pkg/collector/collection.go @@ -249,7 +249,8 @@ func (c *Collection) Build(ctx context.Context, logger *slog.Logger) error { errors.Is(err, pdh.NewPdhError(pdh.CstatusNoObject)) || errors.Is(err, pdh.NewPdhError(pdh.CstatusNoCounter)) || errors.Is(err, mi.MI_RESULT_INVALID_OPERATION_TIMEOUT) || - errors.Is(err, mi.MI_RESULT_INVALID_NAMESPACE) { + errors.Is(err, mi.MI_RESULT_INVALID_NAMESPACE) || + errors.Is(err, mi.MI_RESULT_INVALID_CLASS) { logger.LogAttrs(ctx, slog.LevelWarn, "couldn't initialize collector", slog.Any("err", err)) continue