ts-proto icon indicating copy to clipboard operation
ts-proto copied to clipboard

Maximum call stack size exceeded while encoding google.protobuf.Struct type

Open jokerttu opened this issue 2 years ago • 2 comments

Maximum call stack size exceeded error while encoding .google.protobuf.Struct data.

Related to this PR: https://github.com/stephenh/ts-proto/pull/762

For following proto structure:

message Metadata {
  map<string, google.protobuf.Struct> filter_metadata = 1;
}

Generated .pb.ts type for filter_metadata:

export interface Metadata {
  filter_metadata: { [key: string]: { [key: string]: any } | undefined };
}

Encoders:

Metadata::

  ...
    Object.entries(message.filter_metadata).forEach(([key, value]) => {
      if (value !== undefined) {
        Metadata_FilterMetadataEntry.encode({ key: key as any, value }, writer.uint32(10).fork()).ldelim();
      }
    });
  ...

Metadata_FilterMetadataEntry:

export const Metadata_FilterMetadataEntry = {
  encode(message: Metadata_FilterMetadataEntry, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer {
    if (message.key !== "") {
      writer.uint32(10).string(message.key);
    }
    if (message.value !== undefined) {
      Struct.encode(Struct.wrap(message.value), writer.uint32(18).fork()).ldelim();
    }
    return writer;
  },
  ...

Struct:

export const Struct = {
  encode(message: Struct, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer {
    Object.entries(message.fields).forEach(([key, value]) => {
      if (value !== undefined) {
        Struct_FieldsEntry.encode({ key: key as any, value }, writer.uint32(10).fork()).ldelim();
      }
    });
    return writer;
  },
  ...

Struct_FieldsEntry:

export const Struct_FieldsEntry = {
  encode(message: Struct_FieldsEntry, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer {
    if (message.key !== "") {
      writer.uint32(10).string(message.key);
    }
    if (message.value !== undefined) {
      Value.encode(Value.wrap(message.value), writer.uint32(18).fork()).ldelim();
    }
    return writer;
  },
  ...

Value:

export const Value = {
  encode(message: Value, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer {
  ...
    if (message.struct_value !== undefined) {
      Struct.encode(Struct.wrap(message.struct_value), writer.uint32(42).fork()).ldelim();
    }
  ...

Problem can be replicated with following data structure:

  filter_metadata: {
    'key1': { key2: 'data' },
  },

for map<string, google.protobuf.Struct> filter_metadata = 1;

Encoding this structure causes stack overflow as data is wrapped multiple times in encoding process with Struct.wrap wrapping already wrapped data again and again causing recursive loop. This generates following structure; this example is after only few iterations:

{
  key: "fields",
  value: {
    struct_value: {
      fields: {
        string_value: {
          struct_value: {
            fields: {
              string_value: {
                string_value: "data",
              },
            },
          },
        },
      },
    },
  },
}

Hotfixed this in generated proto.pd.ts file by changing Struct_FieldsEntry encode function from

Value.encode(Value.wrap(message.value), writer.uint32(18).fork()).ldelim();

to

Value.encode(message.value, writer.uint32(18).fork()).ldelim();

as message should be in Struct format in this step already.

Most probably this could break some other use cases (with deeply nested values maybe), but at least this fixed encoding with data structure above.

jokerttu avatar Feb 15 '23 12:02 jokerttu

Ah shoot; thanks for the report @jokerttu , and yeah it's probably related to the deep-or-shallow wrap/unwrap discussion from this thread:

https://github.com/stephenh/ts-proto/pull/762#discussion_r1091292423

cc @ZimGil if you'd like to take a look, @jokerttu would also be great if you want to dig in; I can put it on my todo list but it might be a little while before I have time to look in to it.

Thanks!

stephenh avatar Feb 15 '23 14:02 stephenh