Skip to content

Commit 9d855d3

Browse files
committed
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`.
1 parent 12ac4ec commit 9d855d3

File tree

3 files changed

+72
-3
lines changed

3 files changed

+72
-3
lines changed
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/*
2+
* Copyright 2024 Diffson Project
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package diffson.circe
18+
19+
import diffson.jsonpatch.TestObjectDiff
20+
import io.circe.Json
21+
22+
class CirceTestObjectDiff extends TestObjectDiff[Json]

core/src/main/scala/diffson/jsonpatch/JsonDiff.scala

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,14 @@ class JsonDiff[Json](diffArray: Boolean, rememberOld: Boolean)(implicit J: Jsony
4848
case (fld, value1) :: fields1 =>
4949
fields2.get(fld) match {
5050
case Some(value2) =>
51-
fieldsDiff(fields1, fields2 - fld, path).flatMap(d => diff(value1, value2, path / fld).map(_ ++ d))
51+
Eval
52+
.defer(fieldsDiff(fields1, fields2 - fld, path))
53+
.flatMap(d => diff(value1, value2, path / fld).map(_ ++ d))
5254
case None =>
5355
// field is not in the second object, delete it
54-
fieldsDiff(fields1, fields2, path).map(
55-
_.prepend(Remove(path / fld, if (rememberOld) Some(value1) else None)))
56+
Eval
57+
.defer(fieldsDiff(fields1, fields2, path))
58+
.map(_.prepend(Remove(path / fld, if (rememberOld) Some(value1) else None)))
5659
}
5760
case Nil =>
5861
Eval.now(Chain.fromSeq(fields2.toList).map { case (fld, value) => Add(path / fld, value) })
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* Copyright 2024 Diffson Project
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package diffson
18+
package jsonpatch
19+
20+
import org.scalatest.flatspec.AnyFlatSpec
21+
import org.scalatest.matchers.should.Matchers
22+
import diffson.jsonpatch.JsonDiff
23+
import diffson.lcs.Patience
24+
25+
abstract class TestObjectDiff[J](implicit J: Jsony[J]) extends AnyFlatSpec with Matchers {
26+
27+
implicit val lcsalg: Patience[J] = new Patience[J]
28+
29+
val diff = new JsonDiff[J](false, false)
30+
31+
"a wide object diffed with an empty one" should "not cause stack overflows" in {
32+
val json1 = J.makeObject((1 to 10000).map(i => s"key$i" -> J.Null).toMap)
33+
val json2 = J.makeObject(Map.empty)
34+
35+
diff.diff(json1, json2)
36+
}
37+
38+
"a wide object diffed with itself" should "not cause stack overflows" in {
39+
val json1 = J.makeObject((1 to 10000).map(i => s"key$i" -> J.Null).toMap)
40+
41+
diff.diff(json1, json1)
42+
}
43+
44+
}

0 commit comments

Comments
 (0)