Skip to content

panic on non nil "Result" with nil value in "AfterQuery" hook #1942

@server-may-cry

Description

@server-may-cry

Query hook receives non nil Result with nil value. This leads to a panic:

runtime error: invalid memory address or nil pointer dereference

Expected Behavior

Code like this don't trigger a nil pointer panic as it have a e.Result != nil check.

func (h hook) AfterQuery(ctx context.Context, e *pg.QueryEvent) error {
	if e.Err != nil {
		h.log("error", e.Err)
	}
	if e.Result != nil {
		h.log("returned", e.Result.RowsReturned())
		h.log("affected", e.Result.RowsAffected())
	}
	return nil
}

Current Behavior

Part of panic stack trace

	/usr/local/go/src/runtime/panic.go:1038 +0x215
github.com/go-pg/pg/v10.(*result).RowsReturned(0xdc4c80)
	/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/result.go:52
<REDACTED>.AfterQuery({{0xdae1c0, 0xc00011e110}}, {0xdc4c80, 0xc000b5b7d0}, 0xc0004a1900)
	<REDACTED>.go:110 +0xf2
github.com/go-pg/pg/v10.(*baseDB).afterQueryFromIndex(0xc0000af0e0, {0xdc4c80, 0xc000b5b7d0}, 0x0, 0xdd8890)
	/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/hook.go:130 +0x75
github.com/go-pg/pg/v10.(*baseDB).afterQuery(0xc000146d00, {0xdc4c80, 0xc000b5b7d0}, 0xc0009c8005, {0xdbce20, 0x0}, {0xdae1c0, 0xc00011e110})
	/go/pkg/mod/github.com/go-pg/pg/v10@v10.10.6/hook.go:125 +0xaa

Possible Solution

Explicitly set nil for Result when *result is nil.
Related article https://codefibershq.com/blog/golang-why-nil-is-not-always-nil

Steps to Reproduce

  1. Add a query hook as provided above
  2. Execute SQL which fails
  3. nil pointer panic is triggered

Here is an example of how nil becomes not nil https://goplay.tools/snippet/DVV1XNz5icn

Context (Environment)

Detailed Description

On first line res is a nil. At the last line it assigned a value from db.simpleQuery

pg/base.go

Lines 236 to 246 in 51b8018

var res Result
var lastErr error
for attempt := 0; attempt <= db.opt.MaxRetries; attempt++ {
if attempt > 0 {
if err := internal.Sleep(ctx, db.retryBackoff(attempt-1)); err != nil {
return nil, err
}
}
lastErr = db.withConn(ctx, func(ctx context.Context, cn *pool.Conn) error {
res, err = db.simpleQuery(ctx, cn, wb)

In db.simpleQuery result might get returned with nil value of type *result, but in db.simpleQuery it will be wrapped into Result and will pass != nil check

pg/base.go

Lines 544 to 553 in 51b8018

var res *result
if err := cn.WithReader(c, db.opt.ReadTimeout, func(rd *pool.ReaderContext) error {
var err error
res, err = readSimpleQuery(rd)
return err
}); err != nil {
return nil, err
}
return res, nil

Note: Same issue exists in other parts of a code.

Possible Implementation

Workaround of this issue is to change a hook to look like this:

func (h hook) AfterQuery(ctx context.Context, e *pg.QueryEvent) error {
	if e.Err != nil {
		h.log("error", e.Err)
	} else if e.Result != nil {
		h.log("returned", e.Result.RowsReturned())
		h.log("affected", e.Result.RowsAffected())
	}
	return nil
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions