From 9d855d3a48e0c97de11c6b01d62f611cab4067f4 Mon Sep 17 00:00:00 2001 From: Lucas Satabin Date: Thu, 25 Sep 2025 15:12:55 +0200 Subject: [PATCH] Fix stack overflow when diffing wide objects The recursive call is sadly not in tail position, so it does use stack when comparing sibling fields. This change suspends the recursive call within an `Eval`, which turns it into a stack safe version. Without the change, the two added tests fail with a `StackOverflow`. --- .../diffson/circe/CirceTestObjectDiff.scala | 22 ++++++++++ .../scala/diffson/jsonpatch/JsonDiff.scala | 9 ++-- .../main/scala/diffson/TestObjectDiff.scala | 44 +++++++++++++++++++ 3 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 circe/shared/src/test/scala/diffson/circe/CirceTestObjectDiff.scala create mode 100644 testkit/shared/src/main/scala/diffson/TestObjectDiff.scala diff --git a/circe/shared/src/test/scala/diffson/circe/CirceTestObjectDiff.scala b/circe/shared/src/test/scala/diffson/circe/CirceTestObjectDiff.scala new file mode 100644 index 0000000..a6648d3 --- /dev/null +++ b/circe/shared/src/test/scala/diffson/circe/CirceTestObjectDiff.scala @@ -0,0 +1,22 @@ +/* + * Copyright 2024 Diffson Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package diffson.circe + +import diffson.jsonpatch.TestObjectDiff +import io.circe.Json + +class CirceTestObjectDiff extends TestObjectDiff[Json] diff --git a/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala b/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala index 2153a33..5e251c9 100644 --- a/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala +++ b/core/src/main/scala/diffson/jsonpatch/JsonDiff.scala @@ -48,11 +48,14 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony case (fld, value1) :: fields1 => fields2.get(fld) match { case Some(value2) => - fieldsDiff(fields1, fields2 - fld, path).flatMap(d => diff(value1, value2, path / fld).map(_ ++ d)) + Eval + .defer(fieldsDiff(fields1, fields2 - fld, path)) + .flatMap(d => diff(value1, value2, path / fld).map(_ ++ d)) case None => // field is not in the second object, delete it - fieldsDiff(fields1, fields2, path).map( - _.prepend(Remove(path / fld, if (rememberOld) Some(value1) else None))) + Eval + .defer(fieldsDiff(fields1, fields2, path)) + .map(_.prepend(Remove(path / fld, if (rememberOld) Some(value1) else None))) } case Nil => Eval.now(Chain.fromSeq(fields2.toList).map { case (fld, value) => Add(path / fld, value) }) diff --git a/testkit/shared/src/main/scala/diffson/TestObjectDiff.scala b/testkit/shared/src/main/scala/diffson/TestObjectDiff.scala new file mode 100644 index 0000000..3a28a56 --- /dev/null +++ b/testkit/shared/src/main/scala/diffson/TestObjectDiff.scala @@ -0,0 +1,44 @@ +/* + * Copyright 2024 Diffson Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package diffson +package jsonpatch + +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers +import diffson.jsonpatch.JsonDiff +import diffson.lcs.Patience + +abstract class TestObjectDiff[J](implicit J: Jsony[J]) extends AnyFlatSpec with Matchers { + + implicit val lcsalg: Patience[J] = new Patience[J] + + val diff = new JsonDiff[J](false, false) + + "a wide object diffed with an empty one" should "not cause stack overflows" in { + val json1 = J.makeObject((1 to 10000).map(i => s"key$i" -> J.Null).toMap) + val json2 = J.makeObject(Map.empty) + + diff.diff(json1, json2) + } + + "a wide object diffed with itself" should "not cause stack overflows" in { + val json1 = J.makeObject((1 to 10000).map(i => s"key$i" -> J.Null).toMap) + + diff.diff(json1, json1) + } + +}