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>

diff --git a/src/net/smtp/smtp.go b/src/net/smtp/smtp.go
index 48f38894fd5f6fb3302432ddf62d4c179cd5ef88..a408fa53363cba284106374421e1054b8d4f7bea 100644 (file)
--- a/src/net/smtp/smtp.go
+++ b/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 {

diff --git a/src/net/smtp/smtp_test.go b/src/net/smtp/smtp_test.go
index 1b1cc84f36e754353ac4624a8d272b59c36b1011..c48fae6d5ac0995f9e01fc525614fe2b6bcd023f 100644 (file)
--- a/src/net/smtp/smtp_test.go
+++ b/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
 }