git-codereview: forbid mailing editor temp files

"mail" command rejects *~, #*# and .#* filenames.

Also it should reject commit messages with non-printable characters,
because these are impossible to spot in the review.

For golang/go#24139

Change-Id: I3544e3c34c5ac9f55a7808264de4535bc455bd0a
Reviewed-on: https://go-review.googlesource.com/101755
Run-TryBot: Yury Smolsky <yury@smolsky.by>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>

3 files changed, 78 insertions, 0 deletions

diff --git a/git-codereview/mail.go b/git-codereview/mail.go
index a63790f..3bea462 100644
--- a/git-codereview/mail.go
+++ b/git-codereview/mail.go
@@ -10,6 +10,8 @@ import (
 	"regexp"
 	"sort"
 	"strings"
+	"unicode"
+	"unicode/utf8"
 )
 
 func cmdMail(args []string) {
@@ -68,6 +70,13 @@ func cmdMail(args []string) {
 		if strings.HasPrefix(c1.Message, "squash!") {
 			dief("%s: CL is a squash! commit", c1.ShortHash)
 		}
+
+		for _, f := range ListFiles(c1) {
+			if strings.HasPrefix(f, ".#") || strings.HasSuffix(f, "~") ||
+				(strings.HasPrefix(f, "#") && strings.HasSuffix(f, "#")) {
+				dief("cannot mail temporary files: %s", f)
+			}
+		}
 	}
 	if !foundCommit {
 		// b.CommitByRev and b.DefaultCommit both return a commit on b.
@@ -79,6 +88,15 @@ func cmdMail(args []string) {
 			"Use '%s change' to include them or '%s mail -f' to force it.", os.Args[0], os.Args[0])
 	}
 
+	if !utf8.ValidString(c.Message) {
+		dief("cannot mail message with invalid UTF-8")
+	}
+	for _, r := range c.Message {
+		if !unicode.IsPrint(r) && !unicode.IsSpace(r) {
+			dief("cannot mail message with non-printable rune %q", r)
+		}
+	}
+
 	// for side effect of dying with a good message if origin is GitHub
 	loadGerritOrigin()
 
diff --git a/git-codereview/mail_test.go b/git-codereview/mail_test.go
index 8680f0e..b1b5201 100644
--- a/git-codereview/mail_test.go
+++ b/git-codereview/mail_test.go
@@ -57,6 +57,60 @@ func TestDoNotMail(t *testing.T) {
 	testPrintedStderr(t, "DO NOT MAIL")
 }
 
+func TestDoNotMailTempFiles(t *testing.T) {
+	// fake auth information to avoid Gerrit error
+	auth.host = "gerrit.fake"
+	auth.user = "not-a-user"
+	defer func() {
+		auth.host = ""
+		auth.user = ""
+	}()
+
+	testFile := func(file string) {
+		gt := newGitTest(t)
+		defer gt.done()
+		gt.work(t)
+		gt.workFile(t, file)
+		testMainDied(t, "mail", "HEAD")
+		testPrintedStderr(t, "cannot mail temporary")
+	}
+
+	testFile("vim-backup.go~")
+	testFile("#emacs-auto-save.go#")
+	testFile(".#emacs-lock.go")
+
+	// Do not mail when a parent of the thing we asked to mail has temporary files.
+	gt := newGitTest(t)
+	defer gt.done()
+	gt.work(t)
+	gt.workFile(t, "backup~")
+	gt.work(t)
+	testMainDied(t, "mail", "HEAD")
+	testPrintedStderr(t, "cannot mail temporary")
+}
+
+func TestMailNonPrintables(t *testing.T) {
+	gt := newGitTest(t)
+	defer gt.done()
+	gt.work(t)
+
+	// fake auth information to avoid Gerrit error
+	auth.host = "gerrit.fake"
+	auth.user = "not-a-user"
+	defer func() {
+		auth.host = ""
+		auth.user = ""
+	}()
+
+	trun(t, gt.client, "git", "commit", "--amend", "-m", "This is my commit.\x10\n\n")
+	testMainDied(t, "mail")
+	testPrintedStderr(t, "message with non-printable")
+
+	// This should be mailed.
+	trun(t, gt.client, "git", "commit", "--amend", "-m", "Printable unicode: \u263A \u0020. Spaces: \v \f \r \t\n\n")
+	testMain(t, "mail", "HEAD")
+}
+
 func TestMailGitHub(t *testing.T) {
 	gt := newGitTest(t)
 	defer gt.done()
diff --git a/git-codereview/util_test.go b/git-codereview/util_test.go
index 133c93a..ab8dced 100644
--- a/git-codereview/util_test.go
+++ b/git-codereview/util_test.go
@@ -90,6 +90,12 @@ func (gt *gitTest) work(t *testing.T) {
 	doWork(t, gt.nwork, gt.client, "file", "23456789")
 }
 
+func (gt *gitTest) workFile(t *testing.T, file string) {
+	// make local change on client in the specific file
+	gt.nwork++
+	doWork(t, gt.nwork, gt.client, file, "23456789")
+}
+
 func (gt *gitTest) serverWork(t *testing.T) {
 	// make change on server
 	// duplicating the sequence of changes in gt.work to simulate them