net/smtp: possible typo in Client.Rcpt

[dtelyukh]

https://github.com/golang/go/blob/master/src/net/smtp/smtp.go#L256
c.cmd(25, "RCPT TO:<%s>", to)

I guess it must be 250, but not 25. Am I right?

[bradfitz]

Yes, looks like it.

Why did nobody (or no tests) notice this before?

Does Rcpt return an error now, but nobody checks the error?

[mattn]

Maybe, no one check the error?

https://github.com/search?utf8=%E2%9C%93&q=%22client.Rcpt%22+language%3AGo&type=Code&ref=searchresults

[bradfitz]

Paging Mr. @campoy for analysis! :)

[mattn]

go/src/net/textproto/reader.go

Lines 196 to 200 in 0104a31

	if 1 <= expectCode && expectCode < 10 && code/100 != expectCode ||
	10 <= expectCode && expectCode < 100 && code/10 != expectCode ||
	100 <= expectCode && expectCode < 1000 && code != expectCode {
	err = &Error{code, message}
	}

https://play.golang.org/p/YTS3qTb_PE

[odeke-em]

Hmm, as alluded by @mattn, the signature of Client.cmd https://github.com/golang/go/blob/master/src/net/smtp/smtp.go#L103

func (c *Client) cmd(expectCode int, format string, args ...interface{}) (int, string, error) {

takes in an expectCode which is used in verifying the status of normalized message.

Perhaps we could improve the readability of that code by defining a table/small function of expect codes and then changing that to something like

+   _, _, err := c.cmd(expectCode(250), "RCPT TO:<%s>", to)
-   _, _, err := c.cmd(25, "RCPT TO:<%s>", to)

or set it to 250

+   _, _, err := c.cmd(250, "RCPT TO:<%s>", to)
-   _, _, err := c.cmd(25, "RCPT TO:<%s>", to)

following suit with other usages:
https://github.com/golang/go/blob/master/src/net/smtp/smtp.go#L155
https://github.com/golang/go/blob/master/src/net/smtp/smtp.go#L184
https://github.com/golang/go/blob/master/src/net/smtp/smtp.go#L220
https://github.com/golang/go/blob/master/src/net/smtp/smtp.go#L248

[mattn]

I guess the code is trying to handle codes like 221/251. But seems not good.

[odeke-em]

So I was curious about the acceptable status codes for RCPT and looked at a few resources on the 25X code intentions so found some hits off Google:

1. https://cr.yp.to/smtp/mail.html by D. J. Bernstein:

2. http://www.greenend.org.uk/rjk/tech/smtpreplies.html by Richard Kettlewell:

It seems like 250 and 251 are the acceptable success codes for RCPT responses. According to https://cr.yp.to/smtp/mail.html, 251 was deprecated, so using 250 is the way to go.

Apologies if my reverse engineering is causing a lot of noise here, but perhaps if we are still in touch with Evan Shaw who created the package with CL df74d8d, we could reach out to them or to someone familiar with the protocol?

[bradfitz]

Thanks, @mattn. I should've read the docs.

@dtelyukh, not a typo. See https://golang.org/pkg/net/textproto/#Reader.ReadResponse ...

For example, if expectCode is 31, an error will be returned if the status is not in the range [310,319].