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

Panic inside io.PutVarUint #3766

Open
cthulhu-rider opened this issue Dec 18, 2024 · 3 comments
Open

Panic inside io.PutVarUint #3766

cthulhu-rider opened this issue Dec 18, 2024 · 3 comments
Labels
bug Something isn't working I4 No visible changes S4 Routine U3 Regular
Milestone

Comments

@cthulhu-rider
Copy link
Contributor

Current Behavior

panic: runtime error: index out of range [8] with length 5 [recovered]
	panic: runtime error: index out of range [8] with length 5

goroutine 19 [running]:
testing.tRunner.func1.2({0x5dd0c0, 0xc0000f6180})
	/usr/local/go/src/testing/testing.go:1631 +0x24a
testing.tRunner.func1()
	/usr/local/go/src/testing/testing.go:1634 +0x377
panic({0x5dd0c0?, 0xc0000f6180?})
	/usr/local/go/src/runtime/panic.go:770 +0x132
github.com/nspcc-dev/neo-go/pkg/io.PutVarUint(...)
	/home/ll/projects/neo/go/pkg/io/binaryWriter.go:109
github.com/nspcc-dev/neo-go/pkg/io_test.TestPutVarUint(0xc0000d8b60?)

Expected Behavior

no panic when buffer has enough len, only otherwise (similar to https://pkg.go.dev/encoding/binary#PutUvarint)

Possible Solution

do not do

_ = data[8]

Steps to Reproduce

func TestPutVarUint(t *testing.T) {
	const n = uint64(123456787890)
	io.PutVarUint(make([]byte, io.GetVarSize(n)), n)
}

Regression

no

Your Environment

  • Version of the product used: v0.107.2
@cthulhu-rider cthulhu-rider added the bug Something isn't working label Dec 18, 2024
@roman-khimov
Copy link
Member

This one was added specifically as canary (17a3f17). At the same time users of this API can expect some symmetry between GetVarSize() and PutVarUint(). And it'll fail later if buffer is too short.

@roman-khimov roman-khimov added this to the v0.108.0 milestone Dec 18, 2024
@roman-khimov roman-khimov added U3 Regular S4 Routine I4 No visible changes labels Dec 18, 2024
@cthulhu-rider
Copy link
Contributor Author

io.GetVarSize(uint64(math.MaxInt + 1)) returns 1 which is also unexpected to me. Caused by cast

return getVarIntSize(int(v.Uint()))

@cthulhu-rider
Copy link
Contributor Author

At the same time users of this API can expect some symmetry between GetVarSize() and PutVarUint().

exactly. But this is unexpected even within oneself:

func TestPutVarUint(t *testing.T) {
	const n = uint64(123)
	b := make([]byte, 32)
	ln := io.PutVarUint(b, n)
	io.PutVarUint(b[:ln], n)
}
panic: runtime error: index out of range [8] with length 1 [recovered]
	panic: runtime error: index out of range [8] with length 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working I4 No visible changes S4 Routine U3 Regular
Projects
None yet
Development

No branches or pull requests

2 participants