Fix LDAP diagnostics message used as format string (#208)

* Add GetLDAPError() tests

This patch adds a couple of tests for the `GetLDAPError()` function.

* Fix LDAP diagnostics message used as format string

The GetLDAPError()-function passes the `diagnosticMessage` error field
as the first parameter to `fmt.Errorf()`. If this message happens to
contain a `%`-character, Go will try to interpret it.

This doesn't directly lead to an error, but results in error messages
containing format string error codes. E.g.:

The error message "Detailed error message %" will result in the error
"Detailed error message %!(NOVERB)".

This patch fixes this by inserting a format string as the first
argument to `fmt.Errorf()`.

Author: olavmrk

error.go

@@ -207,7 +207,7 @@ func GetLDAPError(packet *ber.Packet) error {
 				return nil
 			}
 			return &Error{ResultCode: resultCode, MatchedDN: response.Children[1].Value.(string),
-				Err: fmt.Errorf(response.Children[2].Value.(string))}
+				Err: fmt.Errorf("%s", response.Children[2].Value.(string))}
 		}
 	}
 

error_test.go

@@ -58,6 +58,45 @@ func TestConnReadErr(t *testing.T) {
 	}
 }
 
+// TestGetLDAPError tests parsing of result with a error response.
+func TestGetLDAPError(t *testing.T) {
+	diagnosticMessage := "Detailed error message"
+	bindResponse := ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "Bind Response")
+	bindResponse.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(LDAPResultInvalidCredentials), "resultCode"))
+	bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "dc=example,dc=org", "matchedDN"))
+	bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, diagnosticMessage, "diagnosticMessage"))
+	packet := ber.NewSequence("LDAPMessage")
+	packet.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "messageID"))
+	packet.AppendChild(bindResponse)
+	err := GetLDAPError(packet)
+	if err == nil {
+		t.Errorf("Did not get error response")
+	}
+
+	ldapError := err.(*Error)
+	if ldapError.ResultCode != LDAPResultInvalidCredentials {
+		t.Errorf("Got incorrect error code in LDAP error; got %v, expected %v", ldapError.ResultCode, LDAPResultInvalidCredentials)
+	}
+	if ldapError.Err.Error() != diagnosticMessage {
+		t.Errorf("Got incorrect error message in LDAP error; got %v, expected %v", ldapError.Err.Error(), diagnosticMessage)
+	}
+}
+
+// TestGetLDAPErrorSuccess tests parsing of a result with no error (resultCode == 0).
+func TestGetLDAPErrorSuccess(t *testing.T) {
+	bindResponse := ber.Encode(ber.ClassApplication, ber.TypeConstructed, ApplicationBindResponse, nil, "Bind Response")
+	bindResponse.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "resultCode"))
+	bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "", "matchedDN"))
+	bindResponse.AppendChild(ber.NewString(ber.ClassUniversal, ber.TypePrimitive, ber.TagOctetString, "", "diagnosticMessage"))
+	packet := ber.NewSequence("LDAPMessage")
+	packet.AppendChild(ber.Encode(ber.ClassUniversal, ber.TypePrimitive, ber.TagInteger, int64(0), "messageID"))
+	packet.AppendChild(bindResponse)
+	err := GetLDAPError(packet)
+	if err != nil {
+		t.Errorf("Successful responses should not produce an error, but got: %v", err)
+	}
+}
+
 // signalErrConn is a helpful type used with TestConnReadErr. It implements the
 // net.Conn interface to be used as a connection for the test. Most methods are
 // no-ops but the Read() method blocks until it receives a signal which it