criteo / haproxy-spoe-go

An implementation of the SPOP protocol in Go. https://www.haproxy.org/download/2.0/doc/SPOE.txt
Apache License 2.0
28 stars 14 forks source link

too short buffer for encoding in encodeBytes() #1

Closed bedis closed 5 years ago

bedis commented 5 years ago

It seems there is a bug in the function encodeBytes(): if my action Value string length is longer than the message arg, then the function detects the buffer to store that data is too short and does not encode it. Here is my use case: I have an Agent which can do DNS resolution and when I do reverse lookup, the host name is usually way longer than the IP address. In such case, I can see that the response frame contains an empty value. Of course max-frame-size is big enough: 16380 in my case

ShimmerGlass commented 5 years ago

Fixed in master !

bedis commented 5 years ago

Cool thx.

Note that with latest commit, I do have this error now:

INFO[0000] spoe: listening on [::]:9000                 
INFO[0004] spoe: hello from 127.0.0.1:37503: map[capabilities:pipelining engine-id:0082BA75-E79D-4766-86D3-1AE84AB4A366 max-frame-size:16380 supported-versions:2.0] 
panic: runtime error: slice bounds out of range

goroutine 5 [running]:
github.com/criteo/haproxy-spoe-go.(*conn).handleHello(0xc000060240, 0x100000065, 0x0, 0x0, 0xc0000b0007, 0x74, 0x3ff5, 0x0, 0x0, 0x0, ...)
        /home/baptiste/GO/src/github.com/criteo/haproxy-spoe-go/hello.go:81 +0x12a9
github.com/criteo/haproxy-spoe-go.(*conn).run(0xc000060240, 0xc0000765d0, 0x0, 0x0)
        /home/baptiste/GO/src/github.com/criteo/haproxy-spoe-go/conn.go:37 +0x23c
github.com/criteo/haproxy-spoe-go.(*Agent).Serve.func1(0x579a00, 0xc000010040, 0xc0000765d0)
        /home/baptiste/GO/src/github.com/criteo/haproxy-spoe-go/spoe.go:72 +0x1ac
created by github.com/criteo/haproxy-spoe-go.(*Agent).Serve
        /home/baptiste/GO/src/github.com/criteo/haproxy-spoe-go/spoe.go:66 +0x230
exit status 2
ShimmerGlass commented 5 years ago

Oops, yes inded i borke something afterwards. Fixed now

bedis commented 5 years ago

I confirm this now work on my side too. Thanks a lot. I can enlarge HAProxy feature set now!