Skip to content

C# ProtoJSON: Avoid N^2 behavior in nested Any-of-Any where @type is not first. #27032

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.

Sign up for GitHub

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

Closed
copybara-service wants to merge 1 commit into from
+158 −9
Closed

C# ProtoJSON: Avoid N^2 behavior in nested Any-of-Any where @type is not first. #27032

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions csharp/src/Google.Protobuf.Test/JsonParserTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,59 @@ public void Any_Nested()
Assert.AreEqual(message, parser.Parse(json, TestWellKnownTypes.Descriptor));
}

// -----------------------------------------------------------------------------------------
// Regression tests for O(N²) memory growth when @type appears last in nested Any values
// (https://github.com/protocolbuffers/protobuf/pull/26851).
// -----------------------------------------------------------------------------------------

private static string BuildNestedAnyTypeLastJson(int depth)
{
var sb = new System.Text.StringBuilder("{}");
for (int i = 0; i < depth; i++)
{
string inner = sb.ToString();
sb.Clear();
sb.Append("{\"value\":");
sb.Append(inner);
sb.Append(",\"@type\":\"type.googleapis.com/google.protobuf.Any\"}");
}
return sb.ToString();
}

[Test]
public void Any_TypeUrlLast_DeepNesting()
{
var registry = TypeRegistry.FromMessages(Any.Descriptor);
var parser = new JsonParser(new JsonParser.Settings(1000, registry));

var result200 = parser.Parse<Any>(BuildNestedAnyTypeLastJson(200));
Assert.AreEqual("type.googleapis.com/google.protobuf.Any", result200.TypeUrl);

var result400 = parser.Parse<Any>(BuildNestedAnyTypeLastJson(400));
Assert.AreEqual("type.googleapis.com/google.protobuf.Any", result400.TypeUrl);
}

[Test]
public void Any_TypeUrlLast_ManyFields()
{
var registry = TypeRegistry.FromMessages(TestAllTypes.Descriptor);
var parser = new JsonParser(new JsonParser.Settings(10, registry));

const int fieldCount = 10_000;
var sb = new System.Text.StringBuilder();
sb.Append("{\"repeatedInt32\":[");
for (int i = 0; i < fieldCount; i++)
{
if (i > 0) sb.Append(',');
sb.Append(i);
}
sb.Append("],\"@type\":\"type.googleapis.com/protobuf_unittest3.TestAllTypes\"}");

var any = parser.Parse<Any>(sb.ToString());
var unpacked = any.Unpack<TestAllTypes>();
Assert.AreEqual(fieldCount, unpacked.RepeatedInt32.Count);
}

[Test]
public void DataAfterObject()
{
Expand Down
12 changes: 8 additions & 4 deletions csharp/src/Google.Protobuf/JsonParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ private void MergeAny(IMessage message, JsonTokenizer tokenizer)
{
// Record the token stream until we see the @type property. At that point, we can take the value, consult
// the type registry for the relevant message, and replay the stream, omitting the @type property.
var tokens = new List<JsonToken>();
tokenizer.StartRecording();

var token = tokenizer.Next();
if (token.Type != JsonToken.TokenType.StartObject)
Expand All @@ -529,7 +529,6 @@ private void MergeAny(IMessage message, JsonTokenizer tokenizer)
token.StringValue != JsonFormatter.AnyTypeUrlField ||
tokenizer.ObjectDepth != typeUrlObjectDepth)
{
tokens.Add(token);
token = tokenizer.Next();

// If we get to the end of the object and haven't seen a type URL, just return.
Expand All @@ -538,11 +537,15 @@ private void MergeAny(IMessage message, JsonTokenizer tokenizer)
// other properties but no type URL.
if (tokenizer.ObjectDepth < typeUrlObjectDepth)
{
tokenizer.StopRecording();
return;
}
}

// Don't add the @type property or its value to the recorded token list
tokenizer.StopRecording();
tokenizer.DiscardLastToken();

token = tokenizer.Next();
if (token.Type != JsonToken.TokenType.StringValue)
{
Expand All @@ -567,8 +570,9 @@ private void MergeAny(IMessage message, JsonTokenizer tokenizer)
}

// Now replay the token stream we've already read and anything that remains of the object, just parsing it
// as normal. Our original tokenizer should end up at the end of the object.
var replay = JsonTokenizer.FromReplayedTokens(tokens, tokenizer);
// as normal.
JsonTokenizer replay = tokenizer.GetReplayTokenizer(tokenizer);

var body = descriptor.Parser.CreateTemplate();
if (descriptor.IsWellKnownType)
{
Expand Down
102 changes: 97 additions & 5 deletions csharp/src/Google.Protobuf/JsonTokenizer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace Google.Protobuf
/// </remarks>
internal abstract class JsonTokenizer
{
private JsonToken bufferedToken;
protected JsonToken bufferedToken;

/// <summary>
/// Creates a tokenizer that reads from the given text reader.
Expand Down Expand Up @@ -58,6 +58,11 @@ internal static JsonTokenizer FromReplayedTokens(IList<JsonToken> tokens, JsonTo
/// </summary>
internal int RecursionDepth { get; set; }

internal abstract void StartRecording();
internal abstract void StopRecording();
internal abstract JsonTokenizer GetReplayTokenizer(JsonTokenizer continuation);
internal virtual void DiscardLastToken() { }

/// <summary>
/// Returns the depth of the stack, purely in objects (not collections).
/// Informally, this is the number of remaining unclosed '{' characters we have.
Expand Down Expand Up @@ -91,7 +96,7 @@ internal void PushBack(JsonToken token)
/// <returns>The next token in the stream. This is never null.</returns>
/// <exception cref="InvalidOperationException">This method is called after an EndDocument token has been returned</exception>
/// <exception cref="InvalidJsonException">The input text does not comply with RFC 7159</exception>
internal JsonToken Next()
internal virtual JsonToken Next()
{
JsonToken tokenToReturn;
if (bufferedToken != null)
Expand Down Expand Up @@ -153,22 +158,71 @@ internal void SkipValue()
/// <summary>
/// Tokenizer which first exhausts a list of tokens, then consults another tokenizer.
/// </summary>
private class JsonReplayTokenizer : JsonTokenizer
internal class JsonReplayTokenizer : JsonTokenizer
{
private readonly IList<JsonToken> tokens;
private readonly JsonTokenizer nextTokenizer;
private int nextTokenIndex;
private readonly int endIndex;

internal JsonReplayTokenizer(IList<JsonToken> tokens, JsonTokenizer nextTokenizer)
{
this.tokens = tokens;
this.nextTokenizer = nextTokenizer;
this.endIndex = -1;
}

internal JsonReplayTokenizer(IList<JsonToken> tokens, int startIndex, int endIndex, JsonTokenizer nextTokenizer)
{
this.tokens = tokens;
this.nextTokenIndex = startIndex;
this.endIndex = endIndex;
this.nextTokenizer = nextTokenizer;
}

private int recordStartIndex;
private int recordCount;
private bool recording;

internal override void StartRecording()
{
recordStartIndex = bufferedToken != null ? nextTokenIndex - 1 : nextTokenIndex;
recordCount = 0;
recording = true;
}

internal override JsonToken Next()
{
var token = base.Next();
if (recording)
{
recordCount++;
}
return token;
}

internal override void StopRecording()
{
recording = false;
}

internal override JsonTokenizer GetReplayTokenizer(JsonTokenizer continuation)
{
return new JsonReplayTokenizer(tokens, recordStartIndex, recordStartIndex + recordCount, continuation);
}

internal override void DiscardLastToken()
{
if (recording && recordCount > 0)
{
recordCount--;
}
}

// FIXME: Object depth not maintained...
protected override JsonToken NextImpl()
{
if (nextTokenIndex >= tokens.Count)
int limit = endIndex < 0 ? tokens.Count : endIndex;
if (nextTokenIndex >= limit)
{
return nextTokenizer.Next();
}
Expand All @@ -187,6 +241,44 @@ private sealed class JsonTextTokenizer : JsonTokenizer
private readonly Stack<ContainerType> containerStack = new Stack<ContainerType>();
private readonly PushBackReader reader;
private State state;
private List<JsonToken> recordedTokens;
private bool recording;

internal override void StartRecording()
{
recordedTokens = new List<JsonToken>();
recording = true;
}

internal override JsonToken Next()
{
var token = base.Next();
if (recording)
{
recordedTokens.Add(token);
}
return token;
}

internal override void StopRecording()
{
recording = false;
}

internal override JsonTokenizer GetReplayTokenizer(JsonTokenizer continuation)
{
var result = FromReplayedTokens(recordedTokens, continuation);
recordedTokens = null;
return result;
}

internal override void DiscardLastToken()
{
if (recordedTokens != null && recordedTokens.Count > 0)
{
recordedTokens.RemoveAt(recordedTokens.Count - 1);
}
}

internal JsonTextTokenizer(TextReader reader)
{
Expand Down
Loading