tliron / glsp

Language Server Protocol SDK for Go
Apache License 2.0
165 stars 22 forks source link

fix potential stack overflow when unmarshaling MarkedString #19

Closed slimsag closed 1 year ago

slimsag commented 1 year ago

👋 I work at Sourcegraph and was looking at using your (awesome) library for an experiment, and was encountering some stack overflows (sorry for not providing a test case / reproduction):

    runtime: goroutine stack exceeds 1000000000-byte limit
    runtime: sp=0x14027ab63c0 stack=[0x14027ab6000, 0x14047ab6000]
    fatal error: stack overflow

    runtime stack:
    runtime.throw({0x1043d86ea?, 0x10fbe5880?})
        /Users/stephen@sourcegraph.com/.asdf/installs/golang/1.19.8/go/src/runtime/panic.go:1047 +0x40 fp=0x16f38ed50 sp=0x16f38ed20 pc=0x100f09c30
    runtime.newstack()
        /Users/stephen@sourcegraph.com/.asdf/installs/golang/1.19.8/go/src/runtime/stack.go:1103 +0x464 fp=0x16f38ef00 sp=0x16f38ed50 pc=0x100f246d4
    runtime.morestack()
        /Users/stephen@sourcegraph.com/.asdf/installs/golang/1.19.8/go/src/runtime/asm_arm64.s:316 +0x70 fp=0x16f38ef00 sp=0x16f38ef00 pc=0x100f3bf80

    goroutine 4046 [running]:
    encoding/json.indirect({0x10c9c8540?, 0x140164677c0?, 0x16?}, 0x0)
        /Users/stephen@sourcegraph.com/.asdf/installs/golang/1.19.8/go/src/encoding/json/decode.go:426 +0x488 fp=0x14027ab63c0 sp=0x14027ab63c0 pc=0x100fe12f8
    encoding/json.(*decodeState).array(0x14016479680, {0x10c9c8540?, 0x140164677c0?, 0x100fef5ec?})
        /Users/stephen@sourcegraph.com/.asdf/installs/golang/1.19.8/go/src/encoding/json/decode.go:503 +0x48 fp=0x14027ab6490 sp=0x14027ab63c0 pc=0x100fe1358
    encoding/json.(*decodeState).value(0x14016479680, {0x10c9c8540?, 0x140164677c0?, 0x100fee38c?})
        /Users/stephen@sourcegraph.com/.asdf/installs/golang/1.19.8/go/src/encoding/json/decode.go:364 +0x70 fp=0x14027ab6500 sp=0x14027ab6490 pc=0x100fe0c80
    encoding/json.(*decodeState).unmarshal(0x14016479680, {0x10c9c8540?, 0x140164677c0?})
        /Users/stephen@sourcegraph.com/.asdf/installs/golang/1.19.8/go/src/encoding/json/decode.go:181 +0x204 fp=0x14027ab6570 sp=0x14027ab6500 pc=0x100fe05a4
    encoding/json.Unmarshal({0x14002b80000, 0xa8, 0xb0}, {0x10c9c8540, 0x140164677c0})
        /Users/stephen@sourcegraph.com/.asdf/installs/golang/1.19.8/go/src/encoding/json/decode.go:108 +0xf4 fp=0x14027ab65b0 sp=0x14027ab6570 pc=0x100fe00e4
    github.com/tliron/glsp/protocol_3_16.MarkedString.UnmarshalJSON({{0x100016628?, 0xa7?}}, {0x14002b80000, 0xa8, 0xb0})
        /Users/stephen@sourcegraph.com/.asdf/installs/golang/1.19.8/packages/pkg/mod/github.com/tliron/glsp@v0.1.2-0.20230522173456-0418a2f36157/protocol_3_16/language-features.go:714 +0x50 fp=0x14027ab65f0 sp=0x14027ab65b0 pc=0x101960de0

It seemed the fix was rather simple as MarkedStringStruct was already declared - just not used - to workaround a potential recursive UnmarshalJSON operation, so I figured I'd send the fix in case you want it :)

tliron commented 1 year ago

I sure do want it. :)

To my defense, writing this library involved a lot of brainless boilerplate that an AI could probably have done with fewer mistakes. Thanks for catching this one and for submitting a fix!

tliron commented 1 year ago

Also, let me say it's an honor to get a PR from the "zig/mach guy". It's a very sensible language. I dabble in game dev and have a nice little voxel engine I wrote in C (I hate C++). I have been considering a rewrite in Rust, but maybe Zig would make more sense. If you have an inclination to convince me or any reading materials I am wide open.