Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] - devslog try to read unexported field which may cause panic. #30

Closed
violin0622 opened this issue Jan 31, 2024 · 1 comment
Closed

Comments

@violin0622
Copy link
Contributor

violin0622 commented Jan 31, 2024

Describe the bug
when formatting struct attributes, devslog try to read the unexported fields using reflect,
this behaviour cause panic.

To Reproduce
Given the codes below:

// play/ref/main.go
package main

import (
    "fmt"
    "reflect"

    "play/ref/subpack"
)

func main() {
    var s subpack.S

    fmt.Println(`hello world`)
    fmt.Println(reflect.ValueOf(s).Field(0).Interface())
}
// play/ref/subpack/subpack.go
package subpack

type S struct {
    unexported string
}

build and run it, program will panic with:

$ go run .
hello world
panic: reflect.Value.Interface: cannot return value obtained from unexported field or method

goroutine 1 [running]:
reflect.valueInterface({0x100d175c0?, 0x100dd26a0?, 0x1400009cf18?}, 0x1?)
        /opt/homebrew/Cellar/go/1.21.6/libexec/src/reflect/value.go:1501 +0xc8
reflect.Value.Interface(...)
        /opt/homebrew/Cellar/go/1.21.6/libexec/src/reflect/value.go:1490
main.main()
        /Users/violin/Workspace/golang/play/ref/main.go:14 +0xc4
exit status 2

As for devlog, given the codes below:

package main

import (
	"context"
	"log/slog"
	"net"
	"os"

	"github.com/golang-cz/devslog"
	grpclog "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/logging"
	"google.golang.org/grpc"
	"google.golang.org/grpc/reflection"
)

func main() {
	slog.SetDefault(slog.New(devslog.NewHandler(os.Stderr, nil)))

	logger, opts := grpclog.LoggerFunc(func(
		ctx context.Context,
		lvl grpclog.Level,
		msg string, fields ...any,
	) {
		slog.Default().Log(ctx, slog.Level(lvl), msg, fields...)
	}), []grpclog.Option{grpclog.WithLogOnEvents(grpclog.PayloadReceived, grpclog.PayloadSent)}

	svr := grpc.NewServer(
		grpc.ChainUnaryInterceptor(
			grpclog.UnaryServerInterceptor(logger, opts...)),
		grpc.ChainStreamInterceptor(
			grpclog.StreamServerInterceptor(logger, opts...)))
	reflection.Register(svr)
	l, _ := net.Listen(`tcp`, `:8000`)
	svr.Serve(l)
}

when calling grpc, server will panic in devslog:

$ grpcurl -plaintext localhost:8000 list
panic: reflect.Value.Interface: cannot return value obtained from unexported field or method

goroutine 10 [running]:
reflect.valueInterface({0x104ef3ca0?, 0x14000030a50?, 0x19?}, 0x99?)
        /opt/homebrew/Cellar/go/1.21.6/libexec/src/reflect/value.go:1501 +0xc8
reflect.Value.Interface(...)
        /opt/homebrew/Cellar/go/1.21.6/libexec/src/reflect/value.go:1490
github.com/golang-cz/devslog.(*developHandler).formatStruct(0x104ef3ca0?, {0x104f3c148, 0x104ef3ca0}, {0x104ef3ca0?, 0x14000030a50?, 0x104a35e14?}, 0x1)
        /Users/violin/Workspace/golang/play/ref/devslog/devslog.go:503 +0x80
github.com/golang-cz/devslog.(*developHandler).elementType(0x140001ad4d0, {0x104f3c148, 0x104ef3ca0}, {0x104ef3ca0, 0x14000030a50, 0x1b9}, 0x0, 0x10)
        /Users/violin/Workspace/golang/play/ref/devslog/devslog.go:541 +0xff0
github.com/golang-cz/devslog.(*developHandler).formatStruct(0x1051f491c?, {0x104f3c148, 0x104f07fc0}, {0x104f07fc0?, 0x14000030a50?, 0x0?}, 0x0)
        /Users/violin/Workspace/golang/play/ref/devslog/devslog.go:520 +0x714
github.com/golang-cz/devslog.(*developHandler).colorize(0x140001ad4d0, {0x1400011a800, 0x47, 0x400}, {0x14000294000, 0x9, 0x104d84be5?}, 0x0, {0x10527cea0, 0x0, ...})
        /Users/violin/Workspace/golang/play/ref/devslog/devslog.go:336 +0x2174
github.com/golang-cz/devslog.(*developHandler).processAttributes(0x140001ad4d0, {0x1400011a800, 0x47, 0x400}, 0x140001c3048)
        /Users/violin/Workspace/golang/play/ref/devslog/devslog.go:241 +0x290
github.com/golang-cz/devslog.(*developHandler).Handle(_, {_, _}, {{0xc1669326378f4058, 0x373936, 0x105247ec0}, {0x104d8d46a, 0x10}, 0x0, 0x104d84be5, ...})
        /Users/violin/Workspace/golang/play/ref/devslog/devslog.go:157 +0x2b0
log/slog.(*Logger).log(0x140000214a0, {0x104f35138, 0x140002881b0}, 0x0, {0x104d8d46a, 0x10}, {0x1400014b340, 0x12, 0x1c})
        /opt/homebrew/Cellar/go/1.21.6/libexec/src/log/slog/logger.go:225 +0x15c
log/slog.(*Logger).Log(...)
        /opt/homebrew/Cellar/go/1.21.6/libexec/src/log/slog/logger.go:158
main.main.func1({0x104f35138?, 0x140002881b0?}, 0x1c?, {0x104d8d46a?, 0x140001c3578?}, {0x1400014b340?, 0x4?, 0x4?})
        /Users/violin/Workspace/golang/play/ref/main.go:23 +0xb0
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/logging.LoggerFunc.Log(0x140001fa1c0?, {0x104f35138?, 0x140002881b0?}, 0x140001c3578?, {0x104d8d46a?, 0x104a27c0c?}, {0x1400014b340?, 0x104a274e4?, 0x104a3be90?})
        /Users/violin/.go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.1/interceptors/logging/logging.go:202 +0x54
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/logging.(*reporter).PostMsgReceive(0x14000119d60, {0x104f07fc0?, 0x14000030a50}, {0x0, 0x0}, 0x0?)
        /Users/violin/.go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.1/interceptors/logging/interceptors.go:119 +0x550
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors.(*monitoredServerStream).RecvMsg(0x14000288210, {0x104f07fc0, 0x14000030a50})
        /Users/violin/.go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.1/interceptors/server.go:63 +0x9c
google.golang.org/grpc/reflection/grpc_reflection_v1.(*serverReflectionServerReflectionInfoServer).Recv(0x140000219f0)
        /Users/violin/.go/pkg/mod/google.golang.org/grpc@v1.60.1/reflection/grpc_reflection_v1/reflection_grpc.pb.go:142 +0x58
google.golang.org/grpc/reflection.(*serverReflectionServer).ServerReflectionInfo(0x140001af980, {0x104f38288, 0x140000219f0})
        /Users/violin/.go/pkg/mod/google.golang.org/grpc@v1.60.1/reflection/serverreflection.go:259 +0x74
google.golang.org/grpc/reflection/grpc_reflection_v1._ServerReflection_ServerReflectionInfo_Handler({0x104ed71e0?, 0x140001af980}, {0x104f35f28?, 0x14000288210})
        /Users/violin/.go/pkg/mod/google.golang.org/grpc@v1.60.1/reflection/grpc_reflection_v1/reflection_grpc.pb.go:123 +0x98
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/logging.StreamServerInterceptor.StreamServerInterceptor.func2({0x104ed71e0, 0x140001af980}, {0x104f36000, 0x1400028a000}, 0x104e67c80?, 0x104f2e6e8)
        /Users/violin/.go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.1/interceptors/server.go:35 +0x250
google.golang.org/grpc.(*Server).processStreamingRPC(0x14000181000, {0x104f35138, 0x140002880c0}, {0x104f38510, 0x140001b29c0}, 0x14000001680, 0x140001afaa0, 0x105235cc0, 0x0)
        /Users/violin/.go/pkg/mod/google.golang.org/grpc@v1.60.1/server.go:1673 +0xec8
google.golang.org/grpc.(*Server).handleStream(0x14000181000, {0x104f38510, 0x140001b29c0}, 0x14000001680)
        /Users/violin/.go/pkg/mod/google.golang.org/grpc@v1.60.1/server.go:1787 +0xc18
google.golang.org/grpc.(*Server).serveStreams.func2.1()
        /Users/violin/.go/pkg/mod/google.golang.org/grpc@v1.60.1/server.go:1016 +0x5c
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 9
        /Users/violin/.go/pkg/mod/google.golang.org/grpc@v1.60.1/server.go:1027 +0x138

This is because devlog is trying to read XXXRequest.state.atomicMessageInfo.GoReflectType.t, which is type of reflect.Type

Expected behavior
formatStruct() method should ignore and skip the unexported fields in struct, for reading an unexported field of other package is dangerous and useless.

@david-littlefarmer
Copy link
Collaborator

Resolved with #31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants