protoio: Use new protobuf API for text marshalling

This patch makes protoio use the new protobuf API for
marshalling/unmarshalling text protobufs, as well as extends the tests
to cover marshalling failures.

The protobuf text output is not stable/deterministic and some spaces are
added randomly, so some integration tests have to be adjusted to account
for it.

internal/protoio/protoio.go

@@ -9,7 +9,8 @@ import (
 
 	"blitiri.com.ar/go/chasquid/internal/safeio"
 
-	"github.com/golang/protobuf/proto"
+	"google.golang.org/protobuf/encoding/prototext"
+	"google.golang.org/protobuf/proto"
 )
 
 // ReadMessage reads a protocol buffer message from fname, and unmarshalls it
@@ -29,7 +30,7 @@ func ReadTextMessage(fname string, pb proto.Message) error {
 	if err != nil {
 		return err
 	}
-	return proto.UnmarshalText(string(in), pb)
+	return prototext.Unmarshal(in, pb)
 }
 
 // WriteMessage marshals pb and atomically writes it into fname.
@@ -42,11 +43,18 @@ func WriteMessage(fname string, pb proto.Message, perm os.FileMode) error {
 	return safeio.WriteFile(fname, out, perm)
 }
 
+var textOpts = prototext.MarshalOptions{
+	Multiline: true,
+}
+
 // WriteTextMessage marshals pb in text format and atomically writes it into
 // fname.
 func WriteTextMessage(fname string, pb proto.Message, perm os.FileMode) error {
-	out := proto.MarshalTextString(pb)
-	return safeio.WriteFile(fname, []byte(out), perm)
+	out, err := textOpts.Marshal(pb)
+	if err != nil {
+		return err
+	}
+	return safeio.WriteFile(fname, out, perm)
 }
 
 ///////////////////////////////////////////////////////////////

internal/protoio/protoio_test.go

@@ -75,19 +75,28 @@ func TestStore(t *testing.T) {
 	}
 
 	// Add an extraneous file, which ListIDs should ignore.
-	fd, err := os.Create(dir + "/store/" + "somefile")
-	if fd != nil {
-		fd.Close()
-	}
-	if err != nil {
-		t.Errorf("failed to create extraneous file: %v", err)
-	}
+	mustCreate(t, dir+"/store/"+"somefile")
+
+	// Add a file that is not properly query-escaped, and should be ignored.
+	mustCreate(t, dir+"/store/"+"s:somefile%N")
 
 	if ids, err := st.ListIDs(); len(ids) != 1 || ids[0] != "f" || err != nil {
 		t.Errorf("expected [f], got %v - %v", ids, err)
 	}
 }
 
+func mustCreate(t *testing.T, fname string) {
+	t.Helper()
+
+	f, err := os.Create(fname)
+	if f != nil {
+		f.Close()
+	}
+	if err != nil {
+		t.Fatalf("failed to create file %q: %v", fname, err)
+	}
+}
+
 func TestFileErrors(t *testing.T) {
 	dir := testlib.MustTempDir(t)
 	defer testlib.RemoveIfOk(t, dir)
@@ -97,12 +106,37 @@ func TestFileErrors(t *testing.T) {
 		t.Errorf("write to /proc/doesnotexist worked, expected error")
 	}
 
+	if err := WriteTextMessage("/proc/doesnotexist", pb, 0600); err == nil {
+		t.Errorf("text write to /proc/doesnotexist worked, expected error")
+	}
+
 	if err := ReadMessage("/doesnotexist", pb); err == nil {
 		t.Errorf("read from /doesnotexist worked, expected error")
 	}
 
+	if err := ReadTextMessage("/doesnotexist", pb); err == nil {
+		t.Errorf("text read from /doesnotexist worked, expected error")
+	}
+
 	s := &Store{dir: "/doesnotexist"}
 	if ids, err := s.ListIDs(); !(ids == nil && err != nil) {
 		t.Errorf("list /doesnotexist worked (%v, %v), expected error", ids, err)
 	}
 }
+
+func TestMarshalErrors(t *testing.T) {
+	dir := testlib.MustTempDir(t)
+	defer testlib.RemoveIfOk(t, dir)
+
+	// The marshaller enforces that strings are well-formed utf8. So to create
+	// a marshalling error, we use a non-utf8 string.
+	pb := &testpb.M{Content: "\xc3\x28"}
+
+	if err := WriteMessage("f", pb, 0600); err == nil {
+		t.Errorf("write worked, expected error")
+	}
+
+	if err := WriteTextMessage("ft", pb, 0600); err == nil {
+		t.Errorf("text write worked, expected error")
+	}
+}

test/t-09-loop/run.sh

@@ -57,7 +57,7 @@ done
 # Test that A has outgoing domaininfo for srv-b.
 # This is unrelated to the loop itself, but serves as an end-to-end
 # verification that outgoing domaininfo works.
-if ! grep -q "outgoing_sec_level: TLS_INSECURE" ".data-A/domaininfo/s:srv-b";
+if ! grep -q 'outgoing_sec_level:\s*TLS_INSECURE' ".data-A/domaininfo/s:srv-b";
 then
 	fail "A is missing the domaininfo for srv-b"
 fi

test/t-14-tls_tracking/run.sh

@@ -50,13 +50,13 @@ wait_for_file .mail/userb@srv-b
 mail_diff content .mail/userb@srv-b
 
 # A should have a secure outgoing connection to srv-b.
-if ! grep -q "outgoing_sec_level: TLS_SECURE" ".data-A/domaininfo/s:srv-b";
+if ! grep -q 'outgoing_sec_level:\s*TLS_SECURE' ".data-A/domaininfo/s:srv-b";
 then
 	fail "A is missing the domaininfo for srv-b"
 fi
 
 # B should have a secure incoming connection from srv-a.
-if ! grep -q "incoming_sec_level: TLS_CLIENT" ".data-B/domaininfo/s:srv-a";
+if ! grep -q 'incoming_sec_level:\s*TLS_CLIENT' ".data-B/domaininfo/s:srv-a";
 then
 	fail "B is missing the domaininfo for srv-a"
 fi