Improved encoding error handling

export.go

@@ -3,6 +3,7 @@ package dbus
 import (
 	"errors"
 	"fmt"
+	"os"
 	"reflect"
 	"strings"
 )
@@ -209,28 +210,23 @@ func (conn *Conn) handleCall(msg *Message) {
 		}
 		reply.Headers[FieldSignature] = MakeVariant(SignatureOf(reply.Body...))
 
-		conn.sendMessageAndIfClosed(reply, nil)
+		if err := reply.IsValid(); err != nil {
+			fmt.Fprintf(os.Stderr, "dbus: dropping invalid reply to %s.%s on obj %s: %s\n", ifaceName, name, path, err)
+		} else {
+			conn.sendMessageAndIfClosed(reply, nil)
+		}
 	}
 }
 
 // Emit emits the given signal on the message bus. The name parameter must be
 // formatted as "interface.member", e.g., "org.freedesktop.DBus.NameLost".
 func (conn *Conn) Emit(path ObjectPath, name string, values ...interface{}) error {
-	if !path.IsValid() {
-		return errors.New("dbus: invalid object path")
-	}
 	i := strings.LastIndex(name, ".")
 	if i == -1 {
 		return errors.New("dbus: invalid method name")
 	}
 	iface := name[:i]
 	member := name[i+1:]
-	if !isValidMember(member) {
-		return errors.New("dbus: invalid method name")
-	}
-	if !isValidInterface(iface) {
-		return errors.New("dbus: invalid interface name")
-	}
 	msg := new(Message)
 	msg.Type = TypeSignal
 	msg.Headers = make(map[HeaderField]Variant)
@@ -241,6 +237,9 @@ func (conn *Conn) Emit(path ObjectPath, name string, values ...interface{}) erro
 	if len(values) > 0 {
 		msg.Headers[FieldSignature] = MakeVariant(SignatureOf(values...))
 	}
+	if err := msg.IsValid(); err != nil {
+		return err
+	}
 
 	var closed bool
 	conn.sendMessageAndIfClosed(msg, func() {

message.go

@@ -208,7 +208,7 @@ func DecodeMessageWithFDs(rd io.Reader, fds []int) (msg *Message, err error) {
 // The possibly returned error can be an error of the underlying reader, an
 // InvalidMessageError or a FormatError.
 func DecodeMessage(rd io.Reader) (msg *Message, err error) {
-	return DecodeMessageWithFDs(rd, make([]int, 0));
+	return DecodeMessageWithFDs(rd, make([]int, 0))
 }
 
 type nullwriter struct{}
@@ -227,8 +227,8 @@ func (msg *Message) CountFds() (int, error) {
 }
 
 func (msg *Message) EncodeToWithFDs(out io.Writer, order binary.ByteOrder) (fds []int, err error) {
-	if err := msg.IsValid(); err != nil {
-		return make([]int, 0), err
+	if err := msg.validateHeader(); err != nil {
+		return nil, err
 	}
 	var vs [7]interface{}
 	switch order {
@@ -237,7 +237,7 @@ func (msg *Message) EncodeToWithFDs(out io.Writer, order binary.ByteOrder) (fds
 	case binary.BigEndian:
 		vs[0] = byte('B')
 	default:
-		return make([]int, 0), errors.New("dbus: invalid byte order")
+		return nil, errors.New("dbus: invalid byte order")
 	}
 	body := new(bytes.Buffer)
 	fds = make([]int, 0)
@@ -284,8 +284,13 @@ func (msg *Message) EncodeTo(out io.Writer, order binary.ByteOrder) (err error)
 }
 
 // IsValid checks whether msg is a valid message and returns an
-// InvalidMessageError if it is not.
+// InvalidMessageError or FormatError if it is not.
 func (msg *Message) IsValid() error {
+	var b bytes.Buffer
+	return msg.EncodeTo(&b, nativeEndian)
+}
+
+func (msg *Message) validateHeader() error {
 	if msg.Flags & ^(FlagNoAutoStart|FlagNoReplyExpected|FlagAllowInteractiveAuthorization) != 0 {
 		return InvalidMessageError("invalid flags")
 	}
@@ -330,6 +335,7 @@ func (msg *Message) IsValid() error {
 			return InvalidMessageError("missing signature")
 		}
 	}
+
 	return nil
 }
 

message_test.go

@@ -0,0 +1,111 @@
+package dbus
+
+import "testing"
+
+func TestMessage_validateHeader(t *testing.T) {
+	var tcs = []struct {
+		msg Message
+		err error
+	}{
+		{
+			msg: Message{
+				Flags: 0xFF,
+			},
+			err: InvalidMessageError("invalid flags"),
+		},
+		{
+			msg: Message{
+				Type: 0xFF,
+			},
+			err: InvalidMessageError("invalid message type"),
+		},
+		{
+			msg: Message{
+				Type: TypeMethodCall,
+				Headers: map[HeaderField]Variant{
+					0xFF: MakeVariant("foo"),
+				},
+			},
+			err: InvalidMessageError("invalid header"),
+		},
+		{
+			msg: Message{
+				Type: TypeMethodCall,
+				Headers: map[HeaderField]Variant{
+					FieldPath: MakeVariant(42),
+				},
+			},
+			err: InvalidMessageError("invalid type of header field"),
+		},
+		{
+			msg: Message{
+				Type: TypeMethodCall,
+			},
+			err: InvalidMessageError("missing required header"),
+		},
+		{
+			msg: Message{
+				Type: TypeMethodCall,
+				Headers: map[HeaderField]Variant{
+					FieldPath:   MakeVariant(ObjectPath("break")),
+					FieldMember: MakeVariant("foo.bar"),
+				},
+			},
+			err: InvalidMessageError("invalid path name"),
+		},
+		{
+			msg: Message{
+				Type: TypeMethodCall,
+				Headers: map[HeaderField]Variant{
+					FieldPath:   MakeVariant(ObjectPath("/")),
+					FieldMember: MakeVariant("2"),
+				},
+			},
+			err: InvalidMessageError("invalid member name"),
+		},
+		{
+			msg: Message{
+				Type: TypeMethodCall,
+				Headers: map[HeaderField]Variant{
+					FieldPath:      MakeVariant(ObjectPath("/")),
+					FieldMember:    MakeVariant("foo.bar"),
+					FieldInterface: MakeVariant("break"),
+				},
+			},
+			err: InvalidMessageError("invalid interface name"),
+		},
+		{
+			msg: Message{
+				Type: TypeError,
+				Headers: map[HeaderField]Variant{
+					FieldReplySerial: MakeVariant(uint32(0)),
+					FieldErrorName:   MakeVariant("break"),
+				},
+			},
+			err: InvalidMessageError("invalid error name"),
+		},
+		{
+
+			msg: Message{
+				Type: TypeError,
+				Headers: map[HeaderField]Variant{
+					FieldReplySerial: MakeVariant(uint32(0)),
+					FieldErrorName:   MakeVariant("error.name"),
+				},
+				Body: []interface{}{
+					"break",
+				},
+			},
+			err: InvalidMessageError("missing signature"),
+		},
+	}
+
+	for _, tc := range tcs {
+		t.Run(tc.err.Error(), func(t *testing.T) {
+			err := tc.msg.validateHeader()
+			if err != tc.err {
+				t.Errorf("expected error %q, got %q", tc.err, err)
+			}
+		})
+	}
+}