From a3c512aa95fd26cb688fdcb63ad6cbd661d83bc8 Mon Sep 17 00:00:00 2001 From: Damien Mathieu <42@dmathieu.com> Date: Fri, 13 Sep 2024 09:11:50 +0200 Subject: [PATCH] Fix gosec overflow alerts (#5799) To allow the golangci-lint upgrade in #5796. --------- Co-authored-by: Tyler Yahn --- attribute/value_test.go | 6 +++ bridge/opencensus/internal/ocmetric/metric.go | 6 +-- bridge/opencensus/internal/span.go | 2 +- bridge/opencensus/internal/span_test.go | 41 +++++++++++++++---- .../otlploggrpc/internal/transform/log.go | 5 ++- .../internal/transform/log_test.go | 28 +++++++++++++ .../otlploghttp/internal/transform/log.go | 5 ++- .../internal/transform/log_test.go | 28 +++++++++++++ .../internal/transform/metricdata.go | 5 +-- .../internal/transform/metricdata.go | 5 +-- .../otlptrace/internal/tracetransform/span.go | 8 ++-- .../internal/tracetransform/span_test.go | 10 ++++- .../internaltest/text_map_propagator.go | 4 +- internal/internaltest/text_map_propagator.go | 4 +- internal/rawhelpers.go | 3 +- .../internaltest/text_map_propagator.go.tmpl | 4 +- .../shared/otlp/otlplog/transform/log.go.tmpl | 5 ++- .../otlp/otlplog/transform/log_test.go.tmpl | 28 +++++++++++++ .../otlpmetric/transform/metricdata.go.tmpl | 5 +-- log/keyvalue.go | 3 +- log/keyvalue_test.go | 1 + schema/internal/parser_checks.go | 11 ++++- schema/internal/parser_checks_test.go | 2 + .../internaltest/text_map_propagator.go | 4 +- sdk/metric/internal/exemplar/value.go | 3 +- sdk/metric/internal/exemplar/value_test.go | 8 +++- 26 files changed, 184 insertions(+), 50 deletions(-) diff --git a/attribute/value_test.go b/attribute/value_test.go index 4e17396208e..d1c88702d25 100644 --- a/attribute/value_test.go +++ b/attribute/value_test.go @@ -38,6 +38,12 @@ func TestValue(t *testing.T) { wantType: attribute.INT64, wantValue: int64(42), }, + { + name: "Key.Int64() correctly returns negative keys's internal int64 value", + value: k.Int64(-42).Value, + wantType: attribute.INT64, + wantValue: int64(-42), + }, { name: "Key.Int64Slice() correctly returns keys's internal []int64 value", value: k.Int64Slice([]int64{42, -3, 12}).Value, diff --git a/bridge/opencensus/internal/ocmetric/metric.go b/bridge/opencensus/internal/ocmetric/metric.go index 0d8d9a067a6..f38598bbfda 100644 --- a/bridge/opencensus/internal/ocmetric/metric.go +++ b/bridge/opencensus/internal/ocmetric/metric.go @@ -144,7 +144,7 @@ func convertHistogram(labelKeys []ocmetricdata.LabelKey, ts []*ocmetricdata.Time Attributes: attrs, StartTime: t.StartTime, Time: p.Time, - Count: uint64(dist.Count), + Count: uint64(max(0, dist.Count)), // nolint:gosec // A count should never be negative. Sum: dist.Sum, Bounds: dist.BucketOptions.Bounds, BucketCounts: bucketCounts, @@ -166,7 +166,7 @@ func convertBuckets(buckets []ocmetricdata.Bucket) ([]uint64, []metricdata.Exemp err = errors.Join(err, fmt.Errorf("%w: %q", errNegativeBucketCount, bucket.Count)) continue } - bucketCounts[i] = uint64(bucket.Count) + bucketCounts[i] = uint64(max(0, bucket.Count)) // nolint:gosec // A count should never be negative. if bucket.Exemplar != nil { exemplar, exemplarErr := convertExemplar(bucket.Exemplar) @@ -357,7 +357,7 @@ func convertSummary(labelKeys []ocmetricdata.LabelKey, ts []*ocmetricdata.TimeSe Attributes: attrs, StartTime: t.StartTime, Time: p.Time, - Count: uint64(summary.Count), + Count: uint64(max(0, summary.Count)), // nolint:gosec // A count should never be negative. QuantileValues: convertQuantiles(summary.Snapshot), Sum: summary.Sum, } diff --git a/bridge/opencensus/internal/span.go b/bridge/opencensus/internal/span.go index 9e7ee39fb5b..e3311ee98f3 100644 --- a/bridge/opencensus/internal/span.go +++ b/bridge/opencensus/internal/span.go @@ -61,7 +61,7 @@ func (s *Span) SetName(name string) { // SetStatus sets the status of this span, if it is recording events. func (s *Span) SetStatus(status octrace.Status) { - s.otelSpan.SetStatus(codes.Code(status.Code), status.Message) + s.otelSpan.SetStatus(codes.Code(max(0, status.Code)), status.Message) // nolint:gosec // Overflow checked. } // AddAttributes sets attributes in this span. diff --git a/bridge/opencensus/internal/span_test.go b/bridge/opencensus/internal/span_test.go index 949018b8f0c..3844add629d 100644 --- a/bridge/opencensus/internal/span_test.go +++ b/bridge/opencensus/internal/span_test.go @@ -96,15 +96,40 @@ func TestSpanSetStatus(t *testing.T) { s := &span{recording: true} ocS := internal.NewSpan(s) - c, d := codes.Error, "error" - status := octrace.Status{Code: int32(c), Message: d} - ocS.SetStatus(status) + for _, tt := range []struct { + name string - if s.sCode != c { - t.Error("span.SetStatus failed to set OpenTelemetry status code") - } - if s.sMsg != d { - t.Error("span.SetStatus failed to set OpenTelemetry status description") + code int32 + message string + + wantCode codes.Code + }{ + { + name: "with an error code", + code: int32(codes.Error), + message: "error", + + wantCode: codes.Error, + }, + { + name: "with a negative/invalid code", + code: -42, + message: "error", + + wantCode: codes.Unset, + }, + } { + t.Run(tt.name, func(t *testing.T) { + status := octrace.Status{Code: tt.code, Message: tt.message} + ocS.SetStatus(status) + + if s.sCode != tt.wantCode { + t.Errorf("span.SetStatus failed to set OpenTelemetry status code. Expected %d, got %d", tt.wantCode, s.sCode) + } + if s.sMsg != tt.message { + t.Errorf("span.SetStatus failed to set OpenTelemetry status description. Expected %s, got %s", tt.message, s.sMsg) + } + }) } } diff --git a/exporters/otlp/otlplog/otlploggrpc/internal/transform/log.go b/exporters/otlp/otlplog/otlploggrpc/internal/transform/log.go index 89758f0d073..886d983d143 100644 --- a/exporters/otlp/otlplog/otlploggrpc/internal/transform/log.go +++ b/exporters/otlp/otlplog/otlploggrpc/internal/transform/log.go @@ -139,10 +139,11 @@ func LogRecord(record log.Record) *lpb.LogRecord { // year 1678 or after 2262). timeUnixNano on the zero Time returns 0. The // result does not depend on the location associated with t. func timeUnixNano(t time.Time) uint64 { - if t.IsZero() { + nano := t.UnixNano() + if nano < 0 { return 0 } - return uint64(t.UnixNano()) + return uint64(nano) // nolint:gosec // Overflow checked. } // AttrIter transforms an [attribute.Iterator] into OTLP key-values. diff --git a/exporters/otlp/otlplog/otlploggrpc/internal/transform/log_test.go b/exporters/otlp/otlplog/otlploggrpc/internal/transform/log_test.go index 0cf8918c2d9..aaa2fad3dcc 100644 --- a/exporters/otlp/otlplog/otlploggrpc/internal/transform/log_test.go +++ b/exporters/otlp/otlplog/otlploggrpc/internal/transform/log_test.go @@ -30,6 +30,9 @@ var ( ts = time.Date(2000, time.January, 0o1, 0, 0, 0, 0, time.FixedZone("GMT", 0)) obs = ts.Add(30 * time.Second) + // A time before unix 0. + negativeTs = time.Date(1969, 7, 20, 20, 17, 0, 0, time.UTC) + alice = api.String("user", "alice") bob = api.String("user", "bob") @@ -158,6 +161,20 @@ var ( Resource: res, }.NewRecord()) + out = append(out, logtest.RecordFactory{ + Timestamp: negativeTs, + ObservedTimestamp: obs, + Severity: sevB, + SeverityText: "B", + Body: bodyB, + Attributes: []api.KeyValue{bob}, + TraceID: trace.TraceID(traceIDB), + SpanID: trace.SpanID(spanIDB), + TraceFlags: trace.TraceFlags(flagsB), + InstrumentationScope: &scope, + Resource: res, + }.NewRecord()) + return out }() @@ -206,6 +223,17 @@ var ( TraceId: traceIDB, SpanId: spanIDB, }, + { + TimeUnixNano: 0, + ObservedTimeUnixNano: uint64(obs.UnixNano()), + SeverityNumber: pbSevB, + SeverityText: "B", + Body: pbBodyB, + Attributes: []*cpb.KeyValue{pbBob}, + Flags: uint32(flagsB), + TraceId: traceIDB, + SpanId: spanIDB, + }, } pbScopeLogs = &lpb.ScopeLogs{ diff --git a/exporters/otlp/otlplog/otlploghttp/internal/transform/log.go b/exporters/otlp/otlplog/otlploghttp/internal/transform/log.go index 54c9c2b4d9e..a911450e29d 100644 --- a/exporters/otlp/otlplog/otlploghttp/internal/transform/log.go +++ b/exporters/otlp/otlplog/otlploghttp/internal/transform/log.go @@ -139,10 +139,11 @@ func LogRecord(record log.Record) *lpb.LogRecord { // year 1678 or after 2262). timeUnixNano on the zero Time returns 0. The // result does not depend on the location associated with t. func timeUnixNano(t time.Time) uint64 { - if t.IsZero() { + nano := t.UnixNano() + if nano < 0 { return 0 } - return uint64(t.UnixNano()) + return uint64(nano) // nolint:gosec // Overflow checked. } // AttrIter transforms an [attribute.Iterator] into OTLP key-values. diff --git a/exporters/otlp/otlplog/otlploghttp/internal/transform/log_test.go b/exporters/otlp/otlplog/otlploghttp/internal/transform/log_test.go index 0cf8918c2d9..aaa2fad3dcc 100644 --- a/exporters/otlp/otlplog/otlploghttp/internal/transform/log_test.go +++ b/exporters/otlp/otlplog/otlploghttp/internal/transform/log_test.go @@ -30,6 +30,9 @@ var ( ts = time.Date(2000, time.January, 0o1, 0, 0, 0, 0, time.FixedZone("GMT", 0)) obs = ts.Add(30 * time.Second) + // A time before unix 0. + negativeTs = time.Date(1969, 7, 20, 20, 17, 0, 0, time.UTC) + alice = api.String("user", "alice") bob = api.String("user", "bob") @@ -158,6 +161,20 @@ var ( Resource: res, }.NewRecord()) + out = append(out, logtest.RecordFactory{ + Timestamp: negativeTs, + ObservedTimestamp: obs, + Severity: sevB, + SeverityText: "B", + Body: bodyB, + Attributes: []api.KeyValue{bob}, + TraceID: trace.TraceID(traceIDB), + SpanID: trace.SpanID(spanIDB), + TraceFlags: trace.TraceFlags(flagsB), + InstrumentationScope: &scope, + Resource: res, + }.NewRecord()) + return out }() @@ -206,6 +223,17 @@ var ( TraceId: traceIDB, SpanId: spanIDB, }, + { + TimeUnixNano: 0, + ObservedTimeUnixNano: uint64(obs.UnixNano()), + SeverityNumber: pbSevB, + SeverityText: "B", + Body: pbBodyB, + Attributes: []*cpb.KeyValue{pbBob}, + Flags: uint32(flagsB), + TraceId: traceIDB, + SpanId: spanIDB, + }, } pbScopeLogs = &lpb.ScopeLogs{ diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/transform/metricdata.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/transform/metricdata.go index 975e3b7aa1a..47901d46c02 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/transform/metricdata.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/transform/metricdata.go @@ -279,10 +279,7 @@ func Temporality(t metricdata.Temporality) (mpb.AggregationTemporality, error) { // timeUnixNano on the zero Time returns 0. // The result does not depend on the location associated with t. func timeUnixNano(t time.Time) uint64 { - if t.IsZero() { - return 0 - } - return uint64(t.UnixNano()) + return uint64(max(0, t.UnixNano())) // nolint:gosec // Overflow checked. } // Exemplars returns a slice of OTLP Exemplars generated from exemplars. diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/transform/metricdata.go b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/transform/metricdata.go index 0a1a65c44d2..ad1b0b0f35f 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/transform/metricdata.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/transform/metricdata.go @@ -279,10 +279,7 @@ func Temporality(t metricdata.Temporality) (mpb.AggregationTemporality, error) { // timeUnixNano on the zero Time returns 0. // The result does not depend on the location associated with t. func timeUnixNano(t time.Time) uint64 { - if t.IsZero() { - return 0 - } - return uint64(t.UnixNano()) + return uint64(max(0, t.UnixNano())) // nolint:gosec // Overflow checked. } // Exemplars returns a slice of OTLP Exemplars generated from exemplars. diff --git a/exporters/otlp/otlptrace/internal/tracetransform/span.go b/exporters/otlp/otlptrace/internal/tracetransform/span.go index 81157a71c5c..bf27ef0220e 100644 --- a/exporters/otlp/otlptrace/internal/tracetransform/span.go +++ b/exporters/otlp/otlptrace/internal/tracetransform/span.go @@ -97,8 +97,8 @@ func span(sd tracesdk.ReadOnlySpan) *tracepb.Span { SpanId: sid[:], TraceState: sd.SpanContext().TraceState().String(), Status: status(sd.Status().Code, sd.Status().Description), - StartTimeUnixNano: uint64(sd.StartTime().UnixNano()), - EndTimeUnixNano: uint64(sd.EndTime().UnixNano()), + StartTimeUnixNano: uint64(max(0, sd.StartTime().UnixNano())), // nolint:gosec // Overflow checked. + EndTimeUnixNano: uint64(max(0, sd.EndTime().UnixNano())), // nolint:gosec // Overflow checked. Links: links(sd.Links()), Kind: spanKind(sd.SpanKind()), Name: sd.Name(), @@ -178,7 +178,7 @@ func buildSpanFlags(sc trace.SpanContext) uint32 { flags |= tracepb.SpanFlags_SPAN_FLAGS_CONTEXT_IS_REMOTE_MASK } - return uint32(flags) + return uint32(flags) // nolint:gosec // Flags is a bitmask and can't be negative } // spanEvents transforms span Events to an OTLP span events. @@ -192,7 +192,7 @@ func spanEvents(es []tracesdk.Event) []*tracepb.Span_Event { for i := 0; i < len(es); i++ { events[i] = &tracepb.Span_Event{ Name: es[i].Name, - TimeUnixNano: uint64(es[i].Time.UnixNano()), + TimeUnixNano: uint64(max(0, es[i].Time.UnixNano())), // nolint:gosec // Overflow checked. Attributes: KeyValues(es[i].Attributes), DroppedAttributesCount: clampUint32(es[i].DroppedAttributeCount), } diff --git a/exporters/otlp/otlptrace/internal/tracetransform/span_test.go b/exporters/otlp/otlptrace/internal/tracetransform/span_test.go index 50cb9e123df..9fb85db7801 100644 --- a/exporters/otlp/otlptrace/internal/tracetransform/span_test.go +++ b/exporters/otlp/otlptrace/internal/tracetransform/span_test.go @@ -68,6 +68,7 @@ func TestEmptySpanEvent(t *testing.T) { func TestSpanEvent(t *testing.T) { attrs := []attribute.KeyValue{attribute.Int("one", 1), attribute.Int("two", 2)} eventTime := time.Date(2020, 5, 20, 0, 0, 0, 0, time.UTC) + negativeEventTime := time.Date(1969, 7, 20, 20, 17, 0, 0, time.UTC) got := spanEvents([]tracesdk.Event{ { Name: "test 1", @@ -80,14 +81,21 @@ func TestSpanEvent(t *testing.T) { Time: eventTime, DroppedAttributeCount: 2, }, + { + Name: "test 3", + Attributes: attrs, + Time: negativeEventTime, + DroppedAttributeCount: 2, + }, }) - if !assert.Len(t, got, 2) { + if !assert.Len(t, got, 3) { return } eventTimestamp := uint64(1589932800 * 1e9) assert.Equal(t, &tracepb.Span_Event{Name: "test 1", Attributes: nil, TimeUnixNano: eventTimestamp}, got[0]) // Do not test Attributes directly, just that the return value goes to the correct field. assert.Equal(t, &tracepb.Span_Event{Name: "test 2", Attributes: KeyValues(attrs), TimeUnixNano: eventTimestamp, DroppedAttributesCount: 2}, got[1]) + assert.Equal(t, &tracepb.Span_Event{Name: "test 3", Attributes: KeyValues(attrs), TimeUnixNano: 0, DroppedAttributesCount: 2}, got[2]) } func TestNilLinks(t *testing.T) { diff --git a/exporters/zipkin/internal/internaltest/text_map_propagator.go b/exporters/zipkin/internal/internaltest/text_map_propagator.go index e9e164844e0..c773d9365fb 100644 --- a/exporters/zipkin/internal/internaltest/text_map_propagator.go +++ b/exporters/zipkin/internal/internaltest/text_map_propagator.go @@ -75,8 +75,8 @@ func (p *TextMapPropagator) Inject(ctx context.Context, carrier propagation.Text } // InjectedN tests if p has made n injections to carrier. -func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n int) bool { - if actual := p.stateFromCarrier(carrier).Injections; actual != uint64(n) { +func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n uint64) bool { + if actual := p.stateFromCarrier(carrier).Injections; actual != n { t.Errorf("TextMapPropagator{%q} injected %d times, not %d", p.name, actual, n) return false } diff --git a/internal/internaltest/text_map_propagator.go b/internal/internaltest/text_map_propagator.go index 17bd53f3386..ddcb690de23 100644 --- a/internal/internaltest/text_map_propagator.go +++ b/internal/internaltest/text_map_propagator.go @@ -75,8 +75,8 @@ func (p *TextMapPropagator) Inject(ctx context.Context, carrier propagation.Text } // InjectedN tests if p has made n injections to carrier. -func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n int) bool { - if actual := p.stateFromCarrier(carrier).Injections; actual != uint64(n) { +func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n uint64) bool { + if actual := p.stateFromCarrier(carrier).Injections; actual != n { t.Errorf("TextMapPropagator{%q} injected %d times, not %d", p.name, actual, n) return false } diff --git a/internal/rawhelpers.go b/internal/rawhelpers.go index 9b1da2c02b9..b2fe3e41d3b 100644 --- a/internal/rawhelpers.go +++ b/internal/rawhelpers.go @@ -20,7 +20,8 @@ func RawToBool(r uint64) bool { } func Int64ToRaw(i int64) uint64 { - return uint64(i) + // Assumes original was a valid int64 (overflow not checked). + return uint64(i) // nolint: gosec } func RawToInt64(r uint64) int64 { diff --git a/internal/shared/internaltest/text_map_propagator.go.tmpl b/internal/shared/internaltest/text_map_propagator.go.tmpl index 3ef2745dba9..1e11934133f 100644 --- a/internal/shared/internaltest/text_map_propagator.go.tmpl +++ b/internal/shared/internaltest/text_map_propagator.go.tmpl @@ -75,8 +75,8 @@ func (p *TextMapPropagator) Inject(ctx context.Context, carrier propagation.Text } // InjectedN tests if p has made n injections to carrier. -func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n int) bool { - if actual := p.stateFromCarrier(carrier).Injections; actual != uint64(n) { +func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n uint64) bool { + if actual := p.stateFromCarrier(carrier).Injections; actual != n { t.Errorf("TextMapPropagator{%q} injected %d times, not %d", p.name, actual, n) return false } diff --git a/internal/shared/otlp/otlplog/transform/log.go.tmpl b/internal/shared/otlp/otlplog/transform/log.go.tmpl index 54c9c2b4d9e..a911450e29d 100644 --- a/internal/shared/otlp/otlplog/transform/log.go.tmpl +++ b/internal/shared/otlp/otlplog/transform/log.go.tmpl @@ -139,10 +139,11 @@ func LogRecord(record log.Record) *lpb.LogRecord { // year 1678 or after 2262). timeUnixNano on the zero Time returns 0. The // result does not depend on the location associated with t. func timeUnixNano(t time.Time) uint64 { - if t.IsZero() { + nano := t.UnixNano() + if nano < 0 { return 0 } - return uint64(t.UnixNano()) + return uint64(nano) // nolint:gosec // Overflow checked. } // AttrIter transforms an [attribute.Iterator] into OTLP key-values. diff --git a/internal/shared/otlp/otlplog/transform/log_test.go.tmpl b/internal/shared/otlp/otlplog/transform/log_test.go.tmpl index 0cf8918c2d9..aaa2fad3dcc 100644 --- a/internal/shared/otlp/otlplog/transform/log_test.go.tmpl +++ b/internal/shared/otlp/otlplog/transform/log_test.go.tmpl @@ -30,6 +30,9 @@ var ( ts = time.Date(2000, time.January, 0o1, 0, 0, 0, 0, time.FixedZone("GMT", 0)) obs = ts.Add(30 * time.Second) + // A time before unix 0. + negativeTs = time.Date(1969, 7, 20, 20, 17, 0, 0, time.UTC) + alice = api.String("user", "alice") bob = api.String("user", "bob") @@ -158,6 +161,20 @@ var ( Resource: res, }.NewRecord()) + out = append(out, logtest.RecordFactory{ + Timestamp: negativeTs, + ObservedTimestamp: obs, + Severity: sevB, + SeverityText: "B", + Body: bodyB, + Attributes: []api.KeyValue{bob}, + TraceID: trace.TraceID(traceIDB), + SpanID: trace.SpanID(spanIDB), + TraceFlags: trace.TraceFlags(flagsB), + InstrumentationScope: &scope, + Resource: res, + }.NewRecord()) + return out }() @@ -206,6 +223,17 @@ var ( TraceId: traceIDB, SpanId: spanIDB, }, + { + TimeUnixNano: 0, + ObservedTimeUnixNano: uint64(obs.UnixNano()), + SeverityNumber: pbSevB, + SeverityText: "B", + Body: pbBodyB, + Attributes: []*cpb.KeyValue{pbBob}, + Flags: uint32(flagsB), + TraceId: traceIDB, + SpanId: spanIDB, + }, } pbScopeLogs = &lpb.ScopeLogs{ diff --git a/internal/shared/otlp/otlpmetric/transform/metricdata.go.tmpl b/internal/shared/otlp/otlpmetric/transform/metricdata.go.tmpl index 1e1edc4f872..72e59ef0aca 100644 --- a/internal/shared/otlp/otlpmetric/transform/metricdata.go.tmpl +++ b/internal/shared/otlp/otlpmetric/transform/metricdata.go.tmpl @@ -279,10 +279,7 @@ func Temporality(t metricdata.Temporality) (mpb.AggregationTemporality, error) { // timeUnixNano on the zero Time returns 0. // The result does not depend on the location associated with t. func timeUnixNano(t time.Time) uint64 { - if t.IsZero() { - return 0 - } - return uint64(t.UnixNano()) + return uint64(max(0, t.UnixNano())) // nolint:gosec // Overflow checked. } // Exemplars returns a slice of OTLP Exemplars generated from exemplars. diff --git a/log/keyvalue.go b/log/keyvalue.go index da7f8cb6921..2e1d30c1b88 100644 --- a/log/keyvalue.go +++ b/log/keyvalue.go @@ -76,7 +76,8 @@ func IntValue(v int) Value { return Int64Value(int64(v)) } // Int64Value returns a [Value] for an int64. func Int64Value(v int64) Value { - return Value{num: uint64(v), any: KindInt64} + // This can be later converted back to int64 (overflow not checked). + return Value{num: uint64(v), any: KindInt64} // nolint:gosec } // Float64Value returns a [Value] for a float64. diff --git a/log/keyvalue_test.go b/log/keyvalue_test.go index 9f9a6737d68..8b0788ea1dd 100644 --- a/log/keyvalue_test.go +++ b/log/keyvalue_test.go @@ -47,6 +47,7 @@ func TestValueEqual(t *testing.T) { {}, log.Int64Value(1), log.Int64Value(2), + log.Int64Value(-2), log.Float64Value(3.5), log.Float64Value(3.7), log.BoolValue(true), diff --git a/schema/internal/parser_checks.go b/schema/internal/parser_checks.go index 29485379741..207eeddead5 100644 --- a/schema/internal/parser_checks.go +++ b/schema/internal/parser_checks.go @@ -25,8 +25,15 @@ func CheckFileFormatField(fileFormat string, supportedFormatMajor, supportedForm ) } + if supportedFormatMajor < 0 { + return errors.New("major version should be positive") + } + if supportedFormatMinor < 0 { + return errors.New("major version should be positive") + } + // Check that the major version number in the file is the same as what we expect. - if fileFormatParsed.Major() != uint64(supportedFormatMajor) { + if fileFormatParsed.Major() != uint64(supportedFormatMajor) { // nolint:gosec // Version can't be negative (overflow checked). return fmt.Errorf( "this library cannot parse file formats with major version other than %v", supportedFormatMajor, @@ -35,7 +42,7 @@ func CheckFileFormatField(fileFormat string, supportedFormatMajor, supportedForm // Check that the file minor version number is not greater than // what is requested supports. - if fileFormatParsed.Minor() > uint64(supportedFormatMinor) { + if fileFormatParsed.Minor() > uint64(supportedFormatMinor) { // nolint:gosec // Version can't be negative (overflow checked). supportedFormatMajorMinor := strconv.Itoa(supportedFormatMajor) + "." + strconv.Itoa(supportedFormatMinor) // 1.0 diff --git a/schema/internal/parser_checks_test.go b/schema/internal/parser_checks_test.go index d4c72f943b2..d32cfb84b30 100644 --- a/schema/internal/parser_checks_test.go +++ b/schema/internal/parser_checks_test.go @@ -14,6 +14,8 @@ func TestCheckFileFormatField(t *testing.T) { assert.Error(t, CheckFileFormatField("not a semver", 1, 0)) assert.Error(t, CheckFileFormatField("2.0.0", 1, 0)) assert.Error(t, CheckFileFormatField("1.1.0", 1, 0)) + assert.Error(t, CheckFileFormatField("1.1.0", -1, 0)) + assert.Error(t, CheckFileFormatField("1.1.0", 1, -2)) assert.Error(t, CheckFileFormatField("1.2.0", 1, 1)) diff --git a/sdk/internal/internaltest/text_map_propagator.go b/sdk/internal/internaltest/text_map_propagator.go index d1bd9269917..ee1da7bbccc 100644 --- a/sdk/internal/internaltest/text_map_propagator.go +++ b/sdk/internal/internaltest/text_map_propagator.go @@ -75,8 +75,8 @@ func (p *TextMapPropagator) Inject(ctx context.Context, carrier propagation.Text } // InjectedN tests if p has made n injections to carrier. -func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n int) bool { - if actual := p.stateFromCarrier(carrier).Injections; actual != uint64(n) { +func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n uint64) bool { + if actual := p.stateFromCarrier(carrier).Injections; actual != n { t.Errorf("TextMapPropagator{%q} injected %d times, not %d", p.name, actual, n) return false } diff --git a/sdk/metric/internal/exemplar/value.go b/sdk/metric/internal/exemplar/value.go index 1957d6b1e3a..b1f8637819f 100644 --- a/sdk/metric/internal/exemplar/value.go +++ b/sdk/metric/internal/exemplar/value.go @@ -28,7 +28,8 @@ type Value struct { func NewValue[N int64 | float64](value N) Value { switch v := any(value).(type) { case int64: - return Value{t: Int64ValueType, val: uint64(v)} + // This can be later converted back to int64 (overflow not checked). + return Value{t: Int64ValueType, val: uint64(v)} // nolint:gosec case float64: return Value{t: Float64ValueType, val: math.Float64bits(v)} } diff --git a/sdk/metric/internal/exemplar/value_test.go b/sdk/metric/internal/exemplar/value_test.go index 835879bdc05..7e243e9836a 100644 --- a/sdk/metric/internal/exemplar/value_test.go +++ b/sdk/metric/internal/exemplar/value_test.go @@ -10,8 +10,8 @@ import ( ) func TestValue(t *testing.T) { - const iVal, fVal = int64(43), float64(0.3) - i, f, bad := NewValue[int64](iVal), NewValue[float64](fVal), Value{} + const iVal, fVal, nVal = int64(43), float64(0.3), int64(-42) + i, f, n, bad := NewValue[int64](iVal), NewValue[float64](fVal), NewValue[int64](nVal), Value{} assert.Equal(t, Int64ValueType, i.Type()) assert.Equal(t, iVal, i.Int64()) @@ -21,6 +21,10 @@ func TestValue(t *testing.T) { assert.Equal(t, fVal, f.Float64()) assert.Equal(t, int64(0), f.Int64()) + assert.Equal(t, Int64ValueType, n.Type()) + assert.Equal(t, nVal, n.Int64()) + assert.Equal(t, float64(0), i.Float64()) + assert.Equal(t, UnknownValueType, bad.Type()) assert.Equal(t, float64(0), bad.Float64()) assert.Equal(t, int64(0), bad.Int64())