Runtime error: index out of range

[k33nice]

I've got runtime error

Oct 26 07:45:18: panic: runtime error: index out of range
Oct 26 07:45:18: goroutine 80639 [running]:
Oct 26 07:45:18: github.com/phalaaxx/milter.(*MilterSession).Process(0xc4210278c0, 0xc4210360a0, 0x0, 0x0, 0x0, 0x0)
Oct 26 07:45:18: /go/src/github.com/phalaaxx/milter/session.go:116 +0x48b

It's caused by bounds check in encoding/binary package.

func (bigEndian) Uint16(b []byte) uint16 {
	_ = b[1] // bounds check hint to compiler; see golang.org/issue/14808
	return uint16(b[1]) | uint16(b[0])<<8
}

Perhaps it would be better retrun error if msg.Data length less than needed.

...
		Hostname := ReadCString(msg.Data)
		msg.Data = msg.Data[len(Hostname)+1:]
		// get protocol family
		ProtocolFamily := msg.Data[0]
		msg.Data = msg.Data[1:]
		// get port
		Port := binary.BigEndian.Uint16(msg.Data)
...

[phalaaxx]

Is there any easy way to reproduce this error? How do you make the mail server feed broken data to the milter?
I'm thinking which would be better, to give some kind of error or to silently ignore and let the milter continue to work...?

[k33nice]

I have no success in reproducing it. It's sporadically happens on one of production mail servers, but I have no permissions for debug it deeply. I guess it can be wrong authentication or connection timeout relay on context in maillog... But I don't see strong correlation between panics.
I think maybe do somth like that

Connect(string, string, uint16, net.IP, *Modifier, error) (Response, error)

So users can manualy treat this type of message.

[phalaaxx]

OK, I changed it a bit to return RespTempFail if there are errors during Connect, you can see it in 95fe116. Could you test it like this and see if it fixes the problem for you?
I would prefer to avoid changing the interface because projects that already use milter will have to patch for any such backwards incompatible changes.

[k33nice]

Thanks, I'd updated service, but it seems that the 'if' branch is skipped

Nov 29 20:00:14 e-mail: panic: runtime error: slice bounds out of range
Nov 29 20:00:14 e-mail: goroutine 128005 [running]:
Nov 29 20:00:14 e-mail: github.com/phalaaxx/milter.(*MilterSession).Process(0xc420595a80, 0xc420416be0, 0x0, 0x0, 0x0, 0x0)
Nov 29 20:00:14 e-mail: /go/src/github.com/phalaaxx/milter/session.go:123 +0x13f0
Nov 29 20:00:14 e-mail: github.com/phalaaxx/milter.(*MilterSession).HandleMilterCommands(0xc420595a80)
Nov 29 20:00:14 e-mail: /go/src/github.com/phalaaxx/milter/session.go:236 +0x98
Nov 29 20:00:14 e-mail: created by github.com/phalaaxx/milter.RunServer
Nov 29 20:00:14 e-mail: /go/src/github.com/phalaaxx/milter/server.go:27 +0x4e

[phalaaxx]

My bad, sorry. I'm trying to properly implement it only by reading https://github.com/phalaaxx/milter/blob/master/milter-protocol.txt#L217 and overlooked the next line which tries to advance the msg.Data buffer after reading port number, you can see this patched in 1bbd693.
This may not yet fix the problem for you but if you still get any panics please copy/paste here.
Thanks.

[k33nice]

Yep, for now I have no panics. I'll keep my eye on, but for few past monthes I've had panics only from connect macro. If I catch some again, I'll let you know.
Thanks for fixing it!

[temoon]

Hi all!
I had cough semi bug, but with strings which contains empties on the end, like this: "foo\0bar\0baz\0\0". Technically it should be a slice of ["foo", "bar", "baz", ""], but now DecodeCStrings does ["foo", "bar", "baz"].

There is a patch:

diff --git a/cstrings.go b/cstrings.go
index 1dc610c..c4ec7fe 100644
--- a/cstrings.go
+++ b/cstrings.go
@@ -9,11 +9,12 @@ import (
 const NULL = "\x00"
 
 /* DecodeCStrings splits c style strings into golang slice */
-func DecodeCStrings(data []byte) []string {
+func DecodeCStrings(data []byte) (str []string) {
        if len(data) == 0 {
                return nil
        }
-       return strings.Split(strings.Trim(string(data), NULL), NULL)
+       str = strings.Split(string(data), NULL)
+       return str[:len(str) - 1]
 }
 
 /* ReadCString reads and returs c style string from []byte */

[temoon]

My prev comment here because its hit the topic error from session.go line 149