From f04a3b28b55e380bcd249cb61b7dda2e6fa6e51e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Eduardo=20Fern=C3=A1ndez?= Date: Tue, 21 Nov 2023 17:15:52 +0000 Subject: [PATCH 1/2] fix: invalid pointer deref accessing results --- extra/pgotel/pgotel.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/extra/pgotel/pgotel.go b/extra/pgotel/pgotel.go index da4f5964..01c78ff3 100644 --- a/extra/pgotel/pgotel.go +++ b/extra/pgotel/pgotel.go @@ -2,6 +2,7 @@ package pgotel import ( "context" + "reflect" "runtime" "strings" @@ -125,7 +126,7 @@ func (h *TracingHook) AfterQuery(ctx context.Context, evt *pg.QueryEvent) error span.RecordError(evt.Err) span.SetStatus(codes.Error, evt.Err.Error()) } - } else if evt.Result != nil { + } else if evt.Result != nil && (reflect.ValueOf(evt.Result).Kind() != reflect.Ptr || !reflect.ValueOf(evt.Result).IsNil()) { numRow := evt.Result.RowsAffected() if numRow == 0 { numRow = evt.Result.RowsReturned() From 3288511bf391d8d04746ca9afd5c9b9fd9591cbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Eduardo=20Fern=C3=A1ndez?= Date: Tue, 21 Nov 2023 18:22:38 +0000 Subject: [PATCH 2/2] add test --- extra/pgotel/pgotel.go | 6 +++- extra/pgotel/pgotel_test.go | 62 +++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 extra/pgotel/pgotel_test.go diff --git a/extra/pgotel/pgotel.go b/extra/pgotel/pgotel.go index 01c78ff3..dda8fe12 100644 --- a/extra/pgotel/pgotel.go +++ b/extra/pgotel/pgotel.go @@ -126,7 +126,7 @@ func (h *TracingHook) AfterQuery(ctx context.Context, evt *pg.QueryEvent) error span.RecordError(evt.Err) span.SetStatus(codes.Error, evt.Err.Error()) } - } else if evt.Result != nil && (reflect.ValueOf(evt.Result).Kind() != reflect.Ptr || !reflect.ValueOf(evt.Result).IsNil()) { + } else if hasResults(evt) { numRow := evt.Result.RowsAffected() if numRow == 0 { numRow = evt.Result.RowsReturned() @@ -139,6 +139,10 @@ func (h *TracingHook) AfterQuery(ctx context.Context, evt *pg.QueryEvent) error return nil } +func hasResults(evt *pg.QueryEvent) bool { + return evt.Result != nil && (reflect.ValueOf(evt.Result).Kind() != reflect.Ptr || !reflect.ValueOf(evt.Result).IsNil()) +} + func funcFileLine(pkg string) (string, string, int) { const depth = 16 var pcs [depth]uintptr diff --git a/extra/pgotel/pgotel_test.go b/extra/pgotel/pgotel_test.go new file mode 100644 index 00000000..943d4c92 --- /dev/null +++ b/extra/pgotel/pgotel_test.go @@ -0,0 +1,62 @@ +package pgotel + +import ( + "testing" + + "github.com/go-pg/pg/v10" + "github.com/go-pg/pg/v10/orm" +) + +// mockResult is a mock implementation of the Result interface +type mockResult struct { + model orm.Model + rowsAffected int + rowsReturned int +} + +func (m mockResult) Model() orm.Model { + return m.model +} + +func (m mockResult) RowsAffected() int { + return m.rowsAffected +} + +func (m mockResult) RowsReturned() int { + return m.rowsReturned +} + +func TestHasResults(t *testing.T) { + // Define test cases + testCases := []struct { + name string + event *pg.QueryEvent + expected bool + }{ + { + name: "Nil Result", + event: &pg.QueryEvent{Result: nil}, + expected: false, + }, + { + name: "Nil Pointer Result", + event: &pg.QueryEvent{Result: (*mockResult)(nil)}, + expected: false, + }, + { + name: "Non-Nil Result", + event: &pg.QueryEvent{Result: mockResult{rowsAffected: 1, rowsReturned: 1}}, + expected: true, + }, + } + + // Run test cases + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := hasResults(tc.event) + if result != tc.expected { + t.Errorf("hasResults(%v) = %v, want %v", tc.event, result, tc.expected) + } + }) + } +}