net/smtp: make Client.Auth trim final space if Auth.Start toServer is empty

Users can implement the smtp.Auth interface and return zero bytes in
the "toServer []byte" return value from the Auth.Start method. People
apparently do this to implement the SMTP "LOGIN" method.

But we were then sending "AUTH LOGIN \r\n" to the server, which some
servers apparently choke on. So, trim it when the toServer value is
empty.

Fixes #17794

Change-Id: I83662dba9e0f61b1c5000396c096cf7110f78361
Reviewed-on: https://go-review.googlesource.com/33143
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

src/net/smtp/smtp.go

@@ -19,6 +19,7 @@ import (
 	"crypto/tls"
 	"encoding/base64"
 	"errors"
+	"fmt"
 	"io"
 	"net"
 	"net/textproto"
@@ -200,7 +201,7 @@ func (c *Client) Auth(a Auth) error {
 	}
 	resp64 := make([]byte, encoding.EncodedLen(len(resp)))
 	encoding.Encode(resp64, resp)
-	code, msg64, err := c.cmd(0, "AUTH %s %s", mech, resp64)
+	code, msg64, err := c.cmd(0, strings.TrimSpace(fmt.Sprintf("AUTH %s %s", mech, resp64)))
 	for err == nil {
 		var msg []byte
 		switch code {

src/net/smtp/smtp_test.go

@@ -94,6 +94,46 @@ func TestAuthPlain(t *testing.T) {
 	}
 }
 
+// Issue 17794: don't send a trailing space on AUTH command when there's no password.
+func TestClientAuthTrimSpace(t *testing.T) {
+	server := "220 hello world\r\n" +
+		"200 some more"
+	var wrote bytes.Buffer
+	var fake faker
+	fake.ReadWriter = struct {
+		io.Reader
+		io.Writer
+	}{
+		strings.NewReader(server),
+		&wrote,
+	}
+	c, err := NewClient(fake, "fake.host")
+	if err != nil {
+		t.Fatalf("NewClient: %v", err)
+	}
+	c.tls = true
+	c.didHello = true
+	c.Auth(toServerEmptyAuth{})
+	c.Close()
+	if got, want := wrote.String(), "AUTH FOOAUTH\r\n*\r\nQUIT\r\n"; got != want {
+		t.Errorf("wrote %q; want %q", got, want)
+	}
+}
+
+// toServerEmptyAuth is an implementation of Auth that only implements
+// the Start method, and returns "FOOAUTH", nil, nil. Notably, it returns
+// zero bytes for "toServer" so we can test that we don't send spaces at
+// the end of the line. See TestClientAuthTrimSpace.
+type toServerEmptyAuth struct{}
+
+func (toServerEmptyAuth) Start(server *ServerInfo) (proto string, toServer []byte, err error) {
+	return "FOOAUTH", nil, nil
+}
+
+func (toServerEmptyAuth) Next(fromServer []byte, more bool) (toServer []byte, err error) {
+	panic("unexpected call")
+}
+
 type faker struct {
 	io.ReadWriter
 }