Skip to content

Commit

Permalink
Properly ignore unknown fields when parsing history JSON (#175)
Browse files Browse the repository at this point in the history
Fixes #173
  • Loading branch information
cretz authored Jan 10, 2024
1 parent 0e592ee commit 53a8a3d
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/Temporalio/Common/WorkflowHistory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ public record WorkflowHistory(string Id, IReadOnlyCollection<HistoryEvent> Event
private static readonly JsonSerializerOptions JsonSerializerOptions =
new() { Converters = { JsonCommonTypeConverter.Instance } };

private static readonly JsonParser ProtoJsonParser = new(
JsonParser.Settings.Default.WithIgnoreUnknownFields(true));

/// <summary>
/// Interface for fixes to apply.
/// </summary>
Expand Down Expand Up @@ -71,7 +74,7 @@ private static WorkflowHistory FromJson(string workflowId, Dictionary<string, ob
// serialize the altered, then deserialize again using Proto JSON
HistoryJsonFixer.Fix(historyObj);
var fixedJson = JsonSerializer.Serialize(historyObj);
var history = JsonParser.Default.Parse<History>(fixedJson);
var history = ProtoJsonParser.Parse<History>(fixedJson);
return new(workflowId, history.Events);
}

Expand Down
42 changes: 42 additions & 0 deletions tests/Temporalio.Tests/Common/WorkflowHistoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,46 @@ void Replace(string prevVal, string newVal)
// But that our history parser succeeds
var newHistory = AssertHistoryJson();
}

[Fact]
public void FromJson_UnknownField()
{
// Format some JSON history, then sneak in a new field on an event, then
// try to parse
var history = new History();
history.Events.Add(new HistoryEvent()
{
EventType = EventType.RequestCancelExternalWorkflowExecutionFailed,
RequestCancelExternalWorkflowExecutionFailedEventAttributes = new()
{
Cause = CancelExternalWorkflowExecutionFailedCause.ExternalWorkflowExecutionNotFound,
},
});
history.Events.Add(new HistoryEvent()
{
WorkflowExecutionStartedEventAttributes = new()
{
TaskQueue = new() { Kind = TaskQueueKind.Sticky },
},
});
var historyJson = JsonFormatter.Default.Format(history);

// Alter the first event to have an extra field
var eventStart = "\"events\": [ { ";
var eventStartIndex = historyJson.IndexOf(eventStart);
Assert.True(eventStartIndex > 0);
historyJson = historyJson.Substring(0, eventStartIndex + eventStart.Length) +
"\"someField\": \"someValue\", " +
historyJson.Substring(eventStartIndex + eventStart.Length);

// Confirm parse failure
var exc = Assert.Throws<InvalidProtocolBufferException>(
() => JsonParser.Default.Parse<History>(historyJson));
Assert.Contains("Unknown field: someField", exc.Message);

// But confirm history JSON parsing ignores
var workflowHistory = WorkflowHistory.FromJson("workflow-id", historyJson);
Assert.True(workflowHistory.Events.Count == 2);
Assert.Equal(EventType.RequestCancelExternalWorkflowExecutionFailed, workflowHistory.Events.First().EventType);
}
}

0 comments on commit 53a8a3d

Please sign in to comment.