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

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

Open
jokerttu opened this issue Feb 15, 2023 · 1 comment

Comments

@jokerttu
Copy link

jokerttu commented Feb 15, 2023

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

Related to this PR: #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.

@stephenh
Copy link
Owner

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

#762 (comment)

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants