From 8e81deb4ab9cb28cf11099934dc2d2dc27e89340 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Wed, 16 Sep 2026 22:23:36 +0100 Subject: [PATCH 1/2] Improve buf curl error message on trailing input --- CHANGELOG.md | 1 + private/buf/bufcurl/invoker.go | 25 ++++++---- private/buf/bufcurl/invoker_test.go | 64 ++++++++++++++++++------- private/buf/bufcurl/testdata/test.proto | 5 ++ 4 files changed, 70 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76c945721b..46fae62e69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Add `--stdin-filepath` flag to `buf format`, which reads a single `.proto` file from stdin and writes the formatted result to stdout. The path is not read from disk, and is only used to report parse errors and diffs. +- Improve the `buf curl` error message for methods that accept a single request message. ## [v1.73.0] - 2026-09-11 diff --git a/private/buf/bufcurl/invoker.go b/private/buf/bufcurl/invoker.go index 46a8581f49..7c0ae8a7f6 100644 --- a/private/buf/bufcurl/invoker.go +++ b/private/buf/bufcurl/invoker.go @@ -124,10 +124,8 @@ func (inv *invoker) handleUnary(ctx context.Context, dataSource string, data io. if err := provider.next(msg); err != nil { return err } - // make sure input does not contain a second message - dummy := dynamicpb.NewMessage(inv.md.Input()) - if err := provider.next(dummy); err != io.EOF { - return fmt.Errorf("method %s is a unary RPC, but input contained more than one request message", inv.md.Name()) + if err := inv.verifySingleRequest(provider); err != nil { + return err } req := connect.NewRequest(msg) @@ -180,10 +178,8 @@ func (inv *invoker) handleServerStream(ctx context.Context, dataSource string, d if err := provider.next(msg); err != nil { return err } - // make sure input does not contain a second message - dummy := dynamicpb.NewMessage(inv.md.Input()) - if err := provider.next(dummy); err != io.EOF { - return fmt.Errorf("method %s is a unary RPC, but input contained more than one request message", inv.md.Name()) + if err := inv.verifySingleRequest(provider); err != nil { + return err } req := connect.NewRequest(msg) @@ -261,6 +257,19 @@ func isCancelled(err error) bool { return false } +// verifySingleRequest validates the messageProvider has no more messages. +func (inv *invoker) verifySingleRequest(provider messageProvider) error { + dummy := dynamicpb.NewMessage(inv.md.Input()) + switch err := provider.next(dummy); { + case errors.Is(err, io.EOF): + return nil + case err != nil: + return fmt.Errorf("method %s accepts only a single request message, and the input after the first message could not be parsed: %w", inv.md.Name(), err) + default: + return fmt.Errorf("method %s accepts only a single request message, but input contained more than one", inv.md.Name()) + } +} + func (inv *invoker) handleResponse(data []byte, msg *dynamicpb.Message) error { if msg == nil { msg = dynamicpb.NewMessage(inv.md.Output()) diff --git a/private/buf/bufcurl/invoker_test.go b/private/buf/bufcurl/invoker_test.go index 63f8b5794e..cb67a9ae49 100644 --- a/private/buf/bufcurl/invoker_test.go +++ b/private/buf/bufcurl/invoker_test.go @@ -16,6 +16,7 @@ package bufcurl import ( "os" + "strings" "testing" "github.com/bufbuild/buf/private/buf/buftesting" @@ -32,23 +33,7 @@ import ( func TestCountUnrecognized(t *testing.T) { t.Parallel() - results, _, err := incremental.Run(t.Context(), incremental.New(), queries.FDS{ - Opener: &source.Openers{&source.FS{FS: os.DirFS("./testdata")}, buftesting.WKTOpener()}, - Session: new(ir.Session), - Workspace: source.NewWorkspace("test.proto"), - }) - require.NoError(t, err) - require.Len(t, results, 1) - require.NoError(t, results[0].Fatal) - // fdp stores option values (e.g. MessageOptions.map_entry) as unknown bytes; - // a wire round-trip materializes them as typed fields so the resolver - // recognizes map fields as maps. Mirrors build_image.go's resolverForFDS. - fdsBytes, err := protoencoding.NewWireMarshaler().Marshal(results[0].Value) - require.NoError(t, err) - fds := new(descriptorpb.FileDescriptorSet) - require.NoError(t, protoencoding.NewWireUnmarshaler(nil).Unmarshal(fdsBytes, fds)) - resolver, err := protoencoding.NewResolver(fds.File...) - require.NoError(t, err) + resolver := newTestResolver(t) msgType, err := resolver.FindMessageByName("foo.bar.Message") require.NoError(t, err) msg := msgType.New() @@ -86,3 +71,48 @@ func TestCountUnrecognized(t *testing.T) { unrecognized := countUnrecognized(msg) assert.Equal(t, expectedUnrecognized, unrecognized) } + +func TestVerifySingleRequest(t *testing.T) { + t.Parallel() + resolver := newTestResolver(t) + // Download is server-streaming: it still accepts only a single request, and + // must not be described as unary. + descriptor, err := resolver.FindDescriptorByName("foo.bar.Service.Download") + require.NoError(t, err) + methodDescriptor, ok := descriptor.(protoreflect.MethodDescriptor) + require.True(t, ok) + inv := &invoker{md: methodDescriptor, res: resolver} + verify := func(remainingData string) error { + return inv.verifySingleRequest(newMessageProvider("source", strings.NewReader(remainingData), resolver)) + } + + assert.NoError(t, verify("")) + assert.NoError(t, verify("\n \n")) + assert.EqualError(t, verify("}"), + "method Download accepts only a single request message, and the input after the first message could not be parsed: source at offset 0: invalid character '}' looking for beginning of value") + assert.EqualError(t, verify(`{"s":"two"}`), + "method Download accepts only a single request message, but input contained more than one") +} + +// newTestResolver compiles ./testdata/test.proto into a resolver. +func newTestResolver(t *testing.T) protoencoding.Resolver { + t.Helper() + results, _, err := incremental.Run(t.Context(), incremental.New(), queries.FDS{ + Opener: &source.Openers{&source.FS{FS: os.DirFS("./testdata")}, buftesting.WKTOpener()}, + Session: new(ir.Session), + Workspace: source.NewWorkspace("test.proto"), + }) + require.NoError(t, err) + require.Len(t, results, 1) + require.NoError(t, results[0].Fatal) + // fdp stores option values (e.g. MessageOptions.map_entry) as unknown bytes; + // a wire round-trip materializes them as typed fields so the resolver + // recognizes map fields as maps. Mirrors build_image.go's resolverForFDS. + fdsBytes, err := protoencoding.NewWireMarshaler().Marshal(results[0].Value) + require.NoError(t, err) + fds := new(descriptorpb.FileDescriptorSet) + require.NoError(t, protoencoding.NewWireUnmarshaler(nil).Unmarshal(fdsBytes, fds)) + resolver, err := protoencoding.NewResolver(fds.File...) + require.NoError(t, err) + return resolver +} diff --git a/private/buf/bufcurl/testdata/test.proto b/private/buf/bufcurl/testdata/test.proto index c63bcd532e..c518e4529f 100644 --- a/private/buf/bufcurl/testdata/test.proto +++ b/private/buf/bufcurl/testdata/test.proto @@ -82,3 +82,8 @@ enum Enum { A = 0; B = 1; } + +service Service { + rpc Unary(Message) returns (Message); + rpc Download(Message) returns (stream Message); +} From d0986d72a3386776f2158d371544568045f84b1e Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Thu, 17 Sep 2026 10:05:39 +0100 Subject: [PATCH 2/2] Cover unary testcases --- private/buf/bufcurl/invoker_test.go | 37 +++++++++++++++++++---------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/private/buf/bufcurl/invoker_test.go b/private/buf/bufcurl/invoker_test.go index cb67a9ae49..774e37db07 100644 --- a/private/buf/bufcurl/invoker_test.go +++ b/private/buf/bufcurl/invoker_test.go @@ -75,22 +75,33 @@ func TestCountUnrecognized(t *testing.T) { func TestVerifySingleRequest(t *testing.T) { t.Parallel() resolver := newTestResolver(t) - // Download is server-streaming: it still accepts only a single request, and - // must not be described as unary. - descriptor, err := resolver.FindDescriptorByName("foo.bar.Service.Download") - require.NoError(t, err) - methodDescriptor, ok := descriptor.(protoreflect.MethodDescriptor) - require.True(t, ok) - inv := &invoker{md: methodDescriptor, res: resolver} - verify := func(remainingData string) error { - return inv.verifySingleRequest(newMessageProvider("source", strings.NewReader(remainingData), resolver)) + newVerify := func(methodName string) func(string) error { + descriptor, err := resolver.FindDescriptorByName(protoreflect.FullName("foo.bar.Service." + methodName)) + require.NoError(t, err) + methodDescriptor, ok := descriptor.(protoreflect.MethodDescriptor) + require.True(t, ok) + inv := &invoker{md: methodDescriptor, res: resolver} + return func(remainingData string) error { + return inv.verifySingleRequest(newMessageProvider("source", strings.NewReader(remainingData), resolver)) + } } - assert.NoError(t, verify("")) - assert.NoError(t, verify("\n \n")) - assert.EqualError(t, verify("}"), + verifyUnary := newVerify("Unary") + assert.NoError(t, verifyUnary("")) + assert.NoError(t, verifyUnary("\n \n")) + assert.EqualError(t, verifyUnary("}"), + "method Unary accepts only a single request message, and the input after the first message could not be parsed: source at offset 0: invalid character '}' looking for beginning of value") + assert.EqualError(t, verifyUnary(`{"s":"two"}`), + "method Unary accepts only a single request message, but input contained more than one") + + // Download is server-streaming: it still accepts only a single request, and + // must not be described as unary. + verifyDownload := newVerify("Download") + assert.NoError(t, verifyDownload("")) + assert.NoError(t, verifyDownload("\n \n")) + assert.EqualError(t, verifyDownload("}"), "method Download accepts only a single request message, and the input after the first message could not be parsed: source at offset 0: invalid character '}' looking for beginning of value") - assert.EqualError(t, verify(`{"s":"two"}`), + assert.EqualError(t, verifyDownload(`{"s":"two"}`), "method Download accepts only a single request message, but input contained more than one") }