nspcc-dev/neofs-api-go

Fork 0

mirror of https://github.com/nspcc-dev/neofs-api-go.git synced 2026-03-01 04:28:56 +00:00

Stable marshaller produces different wire for nil structure #75

New issue

Closed

opened 2025-12-28 18:12:27 +00:00 by sami · 5 comments

sami commented

2025-12-28 18:12:27 +00:00

Owner

Originally created by @alexvanin on GitHub (Apr 25, 2022).

Originally assigned to: @fyrchik on GitHub.

Here is the example where stable marshaler produces different wire data compare to default proto.Marshaler.

var nilAttribute *object.Attribute
attrs := []*object.Attribute{nilAttribute}

hdr := new(object.Header)
hdr.SetAttributes(attrs)

o := new(object.Object)
o.SetHeader(hdr)

val, err := o.StableMarshal(nil)
require.NoError(t, err)

val2, err := proto.Marshal(o.ToGRPCMessage().(proto.Message))
require.NoError(t, err)


// expected: []byte{0x1a, 0x0}
// actual  : []byte{0x1a, 0x2, 0x52, 0x0}
require.Equal(t, val, val2)

This can lead to unexpected signature check failures, because client side calls StableMarshal() to generate signature, and server side calls proto.Unmarshal() + StableMarshal(). They produce different wire output which leads to signature check failure.

We mostly avoid nil structures, but clarification of expected behavior will be appreciated.

Originally created by @alexvanin on GitHub (Apr 25, 2022). Originally assigned to: @fyrchik on GitHub. Here is the example where stable marshaler produces different wire data compare to default `proto.Marshaler`. ```go var nilAttribute *object.Attribute attrs := []*object.Attribute{nilAttribute} hdr := new(object.Header) hdr.SetAttributes(attrs) o := new(object.Object) o.SetHeader(hdr) val, err := o.StableMarshal(nil) require.NoError(t, err) val2, err := proto.Marshal(o.ToGRPCMessage().(proto.Message)) require.NoError(t, err) // expected: []byte{0x1a, 0x0} // actual : []byte{0x1a, 0x2, 0x52, 0x0} require.Equal(t, val, val2) ``` This can lead to unexpected signature check failures, because client side calls `StableMarshal()` to generate signature, and server side calls `proto.Unmarshal() + StableMarshal()`. They produce different wire output which leads to signature check failure. We mostly avoid `nil` structures, but clarification of expected behavior will be appreciated.

sami

2025-12-28 18:12:27 +00:00

closed this issue
added the
discussion

U2
labels

sami commented

2025-12-28 18:12:28 +00:00

Author

Owner

@alexvanin commented on GitHub (Apr 25, 2022):

A bit of clarification.

(1) Client side sets nil structure to repeated list of attributes, uses StableMarshal() and generates message based on []byte{0x1a, 0x0} value.

(2) Then client sends structure to the server over gRPC which uses proto.Marshal with []byte{0x1a, 0x2, 0x52, 0x0} wire.

(3) Server receives []byte{0x1a, 0x2, 0x52, 0x0} and unmarshals it to a list with empty attribute value

(4) Server calls StableMarshal() and generates message based on []byte{0x1a, 0x2, 0x52, 0x0} value

(5) Signature check fails.

Can be fixed by https://github.com/nspcc-dev/neofs-api-go/issues/384 probably, but this is a long-term issue.

@alexvanin commented on GitHub (Apr 25, 2022): A bit of clarification. (1) Client side sets `nil` structure to repeated list of attributes, uses `StableMarshal()` and generates message based on `[]byte{0x1a, 0x0}` value. (2) Then client sends structure to the server over `gRPC` which uses `proto.Marshal` with `[]byte{0x1a, 0x2, 0x52, 0x0}` wire. (3) Server receives `[]byte{0x1a, 0x2, 0x52, 0x0}` and unmarshals it to a list with empty attribute value (4) Server calls `StableMarshal()` and generates message based on `[]byte{0x1a, 0x2, 0x52, 0x0}` value (5) Signature check fails. Can be fixed by https://github.com/nspcc-dev/neofs-api-go/issues/384 probably, but this is a long-term issue.

sami commented

2025-12-28 18:12:28 +00:00

Author

Owner

@alexvanin commented on GitHub (Apr 26, 2022):

This is not the case for object.Attribute after https://github.com/nspcc-dev/neofs-api-go/pull/377 but we still have some pointer slices here and there.

@alexvanin commented on GitHub (Apr 26, 2022): This is not the case for `object.Attribute` after https://github.com/nspcc-dev/neofs-api-go/pull/377 but we still have some pointer slices here and there.

sami commented

2025-12-28 18:12:28 +00:00

Author

Owner

@fyrchik commented on GitHub (Apr 26, 2022):

I believe, we have slices of pointers only in grpc sub-packages or in conversion functions.

If we are talking about the spec only, what about disallowing to have nil in slices completely? List of attributes, ID's etc. should always be filled.

@fyrchik commented on GitHub (Apr 26, 2022): I believe, we have slices of pointers only in `grpc` sub-packages or in conversion functions. If we are talking about the spec only, what about disallowing to have `nil` in slices completely? List of attributes, ID's etc. should always be filled.

sami commented

2025-12-28 18:12:28 +00:00

Author

Owner

@alexvanin commented on GitHub (Apr 26, 2022):

If we are talking about the spec only, what about disallowing to have nil in slices completely? List of attributes, ID's etc. should always be filled.

Seems good enough. So we can say that properties of stable marshaling are:

encode fields in wire in non-descending order,
consider nil messages as non-existing in repeated fields.

If everyone agree with that, the issue can be closed.

@alexvanin commented on GitHub (Apr 26, 2022): > If we are talking about the spec only, what about disallowing to have nil in slices completely? List of attributes, ID's etc. should always be filled. Seems good enough. So we can say that properties of stable marshaling are: - encode fields in wire in non-descending order, - consider `nil` messages as non-existing in `repeated` fields. If everyone agree with that, the issue can be closed.

sami commented

2025-12-28 18:12:28 +00:00

Author

Owner

@cthulhu-rider commented on GitHub (May 16, 2022):