Skip to content

Commit

Permalink
Fix NPE due to reslice only initializing new slice elements (#327)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lahsivjar
lahsivjar committed Jul 24, 2024
1 parent cee5ac3 commit eaed618
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 46 deletions.
2 changes: 1 addition & 1 deletion input/elasticapm/internal/modeldecoder/modeldecoderutil/http.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func HTTPHeadersToModelpb(h http.Header, out []*modelpb.HTTPHeader) []*modelpb.H
if len(h) == 0 {
return nil
}
out = Reslice(out, len(h), modelpb.HTTPHeaderFromVTPool)
out = ResliceAndPopulateNil(out, len(h), modelpb.HTTPHeaderFromVTPool)
i := 0
for k, v := range h {
out[i].Key = k
Expand Down
2 changes: 1 addition & 1 deletion input/elasticapm/internal/modeldecoder/modeldecoderutil/keyvaluepb.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func ToKv(m map[string]any, out []*modelpb.KeyValue) []*modelpb.KeyValue {
return nil
}

out = Reslice(out, len(m), modelpb.KeyValueFromVTPool)
out = ResliceAndPopulateNil(out, len(m), modelpb.KeyValueFromVTPool)

i := 0
for k, v := range m {
Expand Down
40 changes: 21 additions & 19 deletions input/elasticapm/internal/modeldecoder/modeldecoderutil/slice.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,30 @@

package modeldecoderutil

// Reslice increases the slice's capacity, if necessary, to match n.
// If specified, the newFn function is used to create the elements
// to populate the additional space appended to the slice.
func Reslice[Slice ~[]model, model any](slice Slice, n int, newFn func() model) Slice {
if diff := n - cap(slice); diff > 0 {
// start of the extra space
idx := cap(slice)
import (
"slices"
)

// Grow the slice
// Note: append gives no guarantee on the capacity of the resulting slice
// and might overallocate as long as there's enough space for n elements.
slice = append([]model(slice)[:cap(slice)], make([]model, diff)...)
if newFn != nil {
// extend the slice to its capacity
slice = slice[:cap(slice)]
// Reslice returns a slice with length n. It will reallocate the
// slice if the capacity is not enough.
func Reslice[Slice ~[]model, model any](slice Slice, n int) Slice {
if n > cap(slice) {
slice = slices.Grow(slice, n-len(slice))
}
return slice[:n]
}

// populate the extra space
for ; idx < len(slice); idx++ {
slice[idx] = newFn()
// ResliceAndPopulateNil ensures a slice of pointers has atleast
// capacity for n elements and populates any non-nil elements
// in the resulting slice with the value returned from newFn.
func ResliceAndPopulateNil[Slice ~[]*model, model any](slice Slice, n int, newFn func() *model) Slice {
slice = Reslice(slice, n)
if newFn != nil {
for i := 0; i < len(slice); i++ {
if slice[i] == nil {
slice[i] = newFn()
}
}
}

return slice[:n]
return slice
}
29 changes: 22 additions & 7 deletions input/elasticapm/internal/modeldecoder/modeldecoderutil/slice_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,33 @@ func TestReslice(t *testing.T) {
var s []*modelpb.APMEvent

originalSize := 10
s = Reslice(s, originalSize, modelpb.APMEventFromVTPool)
s = Reslice(s, originalSize)
assert.Equal(t, originalSize, len(s))

downsize := 4
s = Reslice(s, downsize)
assert.Equal(t, downsize, len(s))

upsize := 21
s = Reslice(s, upsize)
assert.Equal(t, upsize, len(s))
}

func TestResliceAndPopulateNil(t *testing.T) {
var s []*modelpb.APMEvent

originalSize := 10
s = ResliceAndPopulateNil(s, originalSize, modelpb.APMEventFromVTPool)
validateBackingArray(t, s, originalSize)
assert.Equal(t, originalSize, len(s))

downsize := 4
s = Reslice(s, downsize, nil)
s = ResliceAndPopulateNil(s, downsize, nil)
validateBackingArray(t, s, downsize)
assert.Equal(t, downsize, len(s))

upsize := 21
s = Reslice(s, upsize, modelpb.APMEventFromVTPool)
s = ResliceAndPopulateNil(s, upsize, modelpb.APMEventFromVTPool)
validateBackingArray(t, s, upsize)
assert.Equal(t, upsize, len(s))
}
Expand All @@ -49,9 +65,8 @@ func validateBackingArray(t *testing.T, out []*modelpb.APMEvent, expectedLen int
// validate length
assert.Equal(t, expectedLen, len(out))

// validate backing array is fully populated
backing := out[:cap(out)]
for i := 0; i < cap(backing); i++ {
assert.NotNil(t, backing[i])
// validate all elements of backing array are non-nil
for i := 0; i < len(out); i++ {
assert.NotNil(t, out[i])
}
}
28 changes: 22 additions & 6 deletions input/elasticapm/internal/modeldecoder/rumv3/decoder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,11 @@ func mapToErrorModel(from *errorEvent, event *modelpb.APMEvent) {
log.ParamMessage = from.Log.ParamMessage.Val
}
if len(from.Log.Stacktrace) > 0 {
log.Stacktrace = modeldecoderutil.Reslice(log.Stacktrace, len(from.Log.Stacktrace), modelpb.StacktraceFrameFromVTPool)
log.Stacktrace = modeldecoderutil.ResliceAndPopulateNil(
log.Stacktrace,
len(from.Log.Stacktrace),
modelpb.StacktraceFrameFromVTPool,
)
mapToStracktraceModel(from.Log.Stacktrace, log.Stacktrace)
}
out.Log = log
Expand Down Expand Up @@ -291,7 +295,11 @@ func mapToExceptionModel(from *errorException, out *modelpb.Exception) {
out.Code = modeldecoderutil.ExceptionCodeString(from.Code.Val)
}
if len(from.Cause) > 0 {
out.Cause = modeldecoderutil.Reslice(out.Cause, len(from.Cause), modelpb.ExceptionFromVTPool)
out.Cause = modeldecoderutil.ResliceAndPopulateNil(
out.Cause,
len(from.Cause),
modelpb.ExceptionFromVTPool,
)
for i := 0; i < len(from.Cause); i++ {
mapToExceptionModel(&from.Cause[i], out.Cause[i])
}
Expand All @@ -307,7 +315,11 @@ func mapToExceptionModel(from *errorException, out *modelpb.Exception) {
out.Module = from.Module.Val
}
if len(from.Stacktrace) > 0 {
out.Stacktrace = modeldecoderutil.Reslice(out.Stacktrace, len(from.Stacktrace), modelpb.StacktraceFrameFromVTPool)
out.Stacktrace = modeldecoderutil.ResliceAndPopulateNil(
out.Stacktrace,
len(from.Stacktrace),
modelpb.StacktraceFrameFromVTPool,
)
mapToStracktraceModel(from.Stacktrace, out.Stacktrace)
}
if from.Type.IsSet() {
Expand Down Expand Up @@ -676,7 +688,11 @@ func mapToSpanModel(from *span, event *modelpb.APMEvent) {
out.RepresentativeCount = 1 / from.SampleRate.Val
}
if len(from.Stacktrace) > 0 {
out.Stacktrace = modeldecoderutil.Reslice(out.Stacktrace, len(from.Stacktrace), modelpb.StacktraceFrameFromVTPool)
out.Stacktrace = modeldecoderutil.ResliceAndPopulateNil(
out.Stacktrace,
len(from.Stacktrace),
modelpb.StacktraceFrameFromVTPool,
)
mapToStracktraceModel(from.Stacktrace, out.Stacktrace)
}
if from.Sync.IsSet() {
Expand Down Expand Up @@ -720,11 +736,11 @@ func mapToStracktraceModel(from []stacktraceFrame, out []*modelpb.StacktraceFram
fr.Module = eventFrame.Module.Val
}
if len(eventFrame.PostContext) > 0 {
fr.PostContext = modeldecoderutil.Reslice(fr.PostContext, len(eventFrame.PostContext), nil)
fr.PostContext = modeldecoderutil.Reslice(fr.PostContext, len(eventFrame.PostContext))
copy(fr.PostContext, eventFrame.PostContext)
}
if len(eventFrame.PreContext) > 0 {
fr.PreContext = modeldecoderutil.Reslice(fr.PreContext, len(eventFrame.PreContext), nil)
fr.PreContext = modeldecoderutil.Reslice(fr.PreContext, len(eventFrame.PreContext))
copy(fr.PreContext, eventFrame.PreContext)
}
}
Expand Down
52 changes: 40 additions & 12 deletions input/elasticapm/internal/modeldecoder/v2/decoder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,11 @@ func mapToErrorModel(from *errorEvent, event *modelpb.APMEvent) {
log.ParamMessage = from.Log.ParamMessage.Val
}
if len(from.Log.Stacktrace) > 0 {
log.Stacktrace = modeldecoderutil.Reslice(log.Stacktrace, len(from.Log.Stacktrace), modelpb.StacktraceFrameFromVTPool)
log.Stacktrace = modeldecoderutil.ResliceAndPopulateNil(
log.Stacktrace,
len(from.Log.Stacktrace),
modelpb.StacktraceFrameFromVTPool,
)
mapToStracktraceModel(from.Log.Stacktrace, log.Stacktrace)
}
out.Log = log
Expand Down Expand Up @@ -521,7 +525,11 @@ func mapToExceptionModel(from *errorException, out *modelpb.Exception) {
out.Code = modeldecoderutil.ExceptionCodeString(from.Code.Val)
}
if len(from.Cause) > 0 {
out.Cause = modeldecoderutil.Reslice(out.Cause, len(from.Cause), modelpb.ExceptionFromVTPool)
out.Cause = modeldecoderutil.ResliceAndPopulateNil(
out.Cause,
len(from.Cause),
modelpb.ExceptionFromVTPool,
)
for i := 0; i < len(from.Cause); i++ {
mapToExceptionModel(&from.Cause[i], out.Cause[i])
}
Expand All @@ -537,7 +545,11 @@ func mapToExceptionModel(from *errorException, out *modelpb.Exception) {
out.Module = from.Module.Val
}
if len(from.Stacktrace) > 0 {
out.Stacktrace = modeldecoderutil.Reslice(out.Stacktrace, len(from.Stacktrace), modelpb.StacktraceFrameFromVTPool)
out.Stacktrace = modeldecoderutil.ResliceAndPopulateNil(
out.Stacktrace,
len(from.Stacktrace),
modelpb.StacktraceFrameFromVTPool,
)
mapToStracktraceModel(from.Stacktrace, out.Stacktrace)
}
if from.Type.IsSet() {
Expand Down Expand Up @@ -809,18 +821,22 @@ func mapToMetricsetModel(from *metricset, event *modelpb.APMEvent) bool {
}

if len(from.Samples) > 0 {
event.Metricset.Samples = modeldecoderutil.Reslice(event.Metricset.Samples, len(from.Samples), modelpb.MetricsetSampleFromVTPool)
event.Metricset.Samples = modeldecoderutil.ResliceAndPopulateNil(
event.Metricset.Samples,
len(from.Samples),
modelpb.MetricsetSampleFromVTPool,
)
i := 0
for name, sample := range from.Samples {
ms := event.Metricset.Samples[i]
if len(sample.Counts) != 0 || len(sample.Values) != 0 {
ms.Histogram = modelpb.HistogramFromVTPool()
if n := len(sample.Values); n > 0 {
ms.Histogram.Values = modeldecoderutil.Reslice(ms.Histogram.Values, n, nil)
ms.Histogram.Values = modeldecoderutil.Reslice(ms.Histogram.Values, n)
copy(ms.Histogram.Values, sample.Values)
}
if n := len(sample.Counts); n > 0 {
ms.Histogram.Counts = modeldecoderutil.Reslice(ms.Histogram.Counts, n, nil)
ms.Histogram.Counts = modeldecoderutil.Reslice(ms.Histogram.Counts, n)
copy(ms.Histogram.Counts, sample.Counts)
}
}
Expand Down Expand Up @@ -1086,7 +1102,7 @@ func mapToSpanModel(from *span, event *modelpb.APMEvent) {
out.Composite = composite
}
if len(from.ChildIDs) > 0 {
event.ChildIds = modeldecoderutil.Reslice(event.ChildIds, len(from.ChildIDs), nil)
event.ChildIds = modeldecoderutil.Reslice(event.ChildIds, len(from.ChildIDs))
copy(event.ChildIds, from.ChildIDs)
}
if from.Context.Database.IsSet() {
Expand Down Expand Up @@ -1278,7 +1294,11 @@ func mapToSpanModel(from *span, event *modelpb.APMEvent) {
out.RepresentativeCount = 1
}
if len(from.Stacktrace) > 0 {
out.Stacktrace = modeldecoderutil.Reslice(out.Stacktrace, len(from.Stacktrace), modelpb.StacktraceFrameFromVTPool)
out.Stacktrace = modeldecoderutil.ResliceAndPopulateNil(
out.Stacktrace,
len(from.Stacktrace),
modelpb.StacktraceFrameFromVTPool,
)
mapToStracktraceModel(from.Stacktrace, out.Stacktrace)
}
if from.Sync.IsSet() {
Expand All @@ -1305,7 +1325,11 @@ func mapToSpanModel(from *span, event *modelpb.APMEvent) {
mapOTelAttributesSpan(from.OTel, event)
}
if len(from.Links) > 0 {
out.Links = modeldecoderutil.Reslice(out.Links, len(from.Links), modelpb.SpanLinkFromVTPool)
out.Links = modeldecoderutil.ResliceAndPopulateNil(
out.Links,
len(from.Links),
modelpb.SpanLinkFromVTPool,
)
mapSpanLinks(from.Links, out.Links)
}
if out.Type == "" {
Expand Down Expand Up @@ -1347,11 +1371,11 @@ func mapToStracktraceModel(from []stacktraceFrame, out []*modelpb.StacktraceFram
fr.Module = eventFrame.Module.Val
}
if len(eventFrame.PostContext) > 0 {
fr.PostContext = modeldecoderutil.Reslice(fr.PostContext, len(eventFrame.PostContext), nil)
fr.PostContext = modeldecoderutil.Reslice(fr.PostContext, len(eventFrame.PostContext))
copy(fr.PostContext, eventFrame.PostContext)
}
if len(eventFrame.PreContext) > 0 {
fr.PreContext = modeldecoderutil.Reslice(fr.PreContext, len(eventFrame.PreContext), nil)
fr.PreContext = modeldecoderutil.Reslice(fr.PreContext, len(eventFrame.PreContext))
copy(fr.PreContext, eventFrame.PreContext)
}
if len(eventFrame.Vars) > 0 {
Expand Down Expand Up @@ -1575,7 +1599,11 @@ func mapToTransactionModel(from *transaction, event *modelpb.APMEvent) {
if event.Span == nil {
event.Span = modelpb.SpanFromVTPool()
}
event.Span.Links = modeldecoderutil.Reslice(event.Span.Links, len(from.Links), modelpb.SpanLinkFromVTPool)
event.Span.Links = modeldecoderutil.ResliceAndPopulateNil(
event.Span.Links,
len(from.Links),
modelpb.SpanLinkFromVTPool,
)
mapSpanLinks(from.Links, event.Span.Links)
}
if out.Type == "" {
Expand Down

0 comments on commit eaed618

Please sign in to comment.