Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client: Detect invalid response message format #661

Open
cthulhu-rider opened this issue Dec 10, 2024 · 0 comments
Open

client: Detect invalid response message format #661

cthulhu-rider opened this issue Dec 10, 2024 · 0 comments
Labels
client Issue related to the client enhancement Improving existing functionality I4 No visible changes S3 Minimally significant U4 Nothing urgent

Comments

@cthulhu-rider
Copy link
Contributor

Is your feature request related to a problem? Please describe.

currently, when Client receives protobuf message incompatible with the response structure, it returns invalid response signature. This happens because in this case proto decoder:

  • returns no error;
  • leaves decoded response untouched (empty)
  • sets unknown fields in the decoded response

Describe the solution you'd like

when Client receives response message having known fields with broken format (e.g. wrong type), return clear error on this

note: some fields can be just unsupported (old client, new server). I did not check is it easy to distinguish broken fields from unsupported ones or not, so be careful: unsupported fields must not be denied in general for forward compatibility

Describe alternatives you've considered

do nothing. Technically, there is definitely no valid response signature. But nobody will know what the actual problem is

Additional context

Client testing. Following snippet can also be useful:

func TestInvalidResponseFormat(t *testing.T) {
	m := timestamppb.Now()
	b, err := proto.Marshal(m)
	require.NoError(t, err)
	var resp protoobject.HeadResponse_Body
	require.Error(t, proto.Unmarshal(b, &resp), resp)
}
=== RUN   TestInvalidResponseFormat
        	Error:      	An error is expected but got nil.
        	Test:       	TestInvalidResponseFormat
        	Messages:   	{state:{NoUnkeyedLiterals:{} DoNotCompare:[] DoNotCopy:[] atomicMessageInfo:0xc0002222a0} sizeCache:0 unknownFields:[8 185 193 225 186 6 16 142 250 191 211 1] Head:<nil>}
@cthulhu-rider cthulhu-rider added enhancement Improving existing functionality client Issue related to the client labels Dec 10, 2024
@roman-khimov roman-khimov modified the milestone: v1.0.0-rc13 Dec 10, 2024
@roman-khimov roman-khimov added U4 Nothing urgent S3 Minimally significant I4 No visible changes labels Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Issue related to the client enhancement Improving existing functionality I4 No visible changes S3 Minimally significant U4 Nothing urgent
Projects
None yet
Development

No branches or pull requests

2 participants