author | Alberto Bertogli
<albertito@blitiri.com.ar> 2020-06-30 09:23:09 UTC |
committer | Alberto Bertogli
<albertito@blitiri.com.ar> 2020-06-30 10:14:52 UTC |
parent | 4116c046bc733159dde3bcf19fdb5ea6c1c84d20 |
internal/protoio/protoio.go | +12 | -4 |
internal/protoio/protoio_test.go | +41 | -7 |
test/t-09-loop/run.sh | +1 | -1 |
test/t-14-tls_tracking/run.sh | +2 | -2 |
diff --git a/internal/protoio/protoio.go b/internal/protoio/protoio.go index f066955..fb73b87 100644 --- a/internal/protoio/protoio.go +++ b/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) } /////////////////////////////////////////////////////////////// diff --git a/internal/protoio/protoio_test.go b/internal/protoio/protoio_test.go index dd25229..cc58d35 100644 --- a/internal/protoio/protoio_test.go +++ b/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") + } +} diff --git a/test/t-09-loop/run.sh b/test/t-09-loop/run.sh index 7845a40..5b8a5e1 100755 --- a/test/t-09-loop/run.sh +++ b/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 diff --git a/test/t-14-tls_tracking/run.sh b/test/t-14-tls_tracking/run.sh index f291cf7..ce181e0 100755 --- a/test/t-14-tls_tracking/run.sh +++ b/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