From f99b3ae4668929f576a1c6ed5396ef6df30cdbfa Mon Sep 17 00:00:00 2001
From: Anton Tarasenko <dkanus@gmail.com>
Date: Tue, 13 Jul 2021 18:16:46 +0700
Subject: [PATCH] Fix bug with return value of `JSONPointer.Pop(true)`

`JSONPointer`'s `Pop(true)` method returned stored `MutableText` value
instead of expected immutable `Text` copy. This patch fixes that
behavior.
---
 sources/Text/JSON/JSONPointer.uc | 12 ++++++++----
 sources/Text/Tests/TEST_JSON.uc  | 27 +++++++++++++++++++++++++--
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/sources/Text/JSON/JSONPointer.uc b/sources/Text/JSON/JSONPointer.uc
index 93ecc3c..74ec1c1 100644
--- a/sources/Text/JSON/JSONPointer.uc
+++ b/sources/Text/JSON/JSONPointer.uc
@@ -102,7 +102,10 @@ public final function JSONPointer Set(Text pointerAsText)
     }
     hasEscapedSequences = (pointerAsText.IndexOf(T(TJSON_ESCAPE)) >= 0);
     parts = pointerAsText.SplitByCharacter(T(TSLASH).GetCharacter(0));
-    //  First elements of the array will be empty, so throw it away
+    //  First element of the array is expected to be empty, so throw it away;
+    //  If it is not empty - then `pointerAsText` does not start with "/" and
+    //  we will pretend that we have already removed first element, thus
+    //  "fixing" path (e.g. effectively turning "foo/bar" into "/foo/bar").
     if (parts[0].IsEmpty())
     {
         _.memory.Free(parts[0]);
@@ -199,7 +202,7 @@ public final function Text Pop(optional bool doNotRemove)
         result = _.text.FromIntM(components[lastIndex].asNumber);
     }
     else {
-        result = components[lastIndex].asText;
+        result = components[lastIndex].asText.Copy();
     }
     if (!doNotRemove) {
         components.length = components.length - 1;
@@ -229,8 +232,9 @@ public final function int PopNumeric(optional bool doNotRemove)
     }
     lastIndex = components.length - 1;
     result = GetNumericComponent(lastIndex);
-    _.memory.Free(components[lastIndex].asText);
-    if (!doNotRemove) {
+    if (!doNotRemove)
+    {
+        _.memory.Free(components[lastIndex].asText);
         components.length = components.length - 1;
     }
     return result;
diff --git a/sources/Text/Tests/TEST_JSON.uc b/sources/Text/Tests/TEST_JSON.uc
index d26e577..65dc720 100644
--- a/sources/Text/Tests/TEST_JSON.uc
+++ b/sources/Text/Tests/TEST_JSON.uc
@@ -36,6 +36,7 @@ protected static function Test_Pointer()
     SubTest_PointerToText();
     SubTest_PointerPushPop();
     SubTest_PointerNumeric();
+    SubTest_PopWithoutRemoving();
 }
 
 protected static function SubTest_PointerCreate()
@@ -132,8 +133,8 @@ protected static function SubTest_PointerPushPop()
 
 protected static function SubTest_PointerNumeric()
 {
-    local JSONPointer pointer;
-    local string correct, incorrect;
+    local JSONPointer   pointer;
+    local string        correct, incorrect;
     correct = "`GetNumericComponent()`/`PopNumeric()` cannot correctly retrieve"
         @ "`JSONPointer`'s numeric components.";
     incorrect = "`GetNumericComponent()`/`PopNumeric()` do not return negative"
@@ -164,6 +165,28 @@ protected static function SubTest_PointerNumeric()
     TEST_ExpectTrue(pointer.PopNumeric() < 0);
 }
 
+protected static function SubTest_PopWithoutRemoving()
+{
+    local Text          component;
+    local JSONPointer   pointer;
+    Issue("`Pop(true)` removes the value from the pointer.");
+    pointer = __().json.Pointer(P("/just/a/simple/test"));
+    TEST_ExpectTrue(pointer.Pop(true).ToPlainString() == "test");
+    TEST_ExpectTrue(pointer.Pop(true).ToPlainString() == "test");
+
+    Issue("`Pop(true)` returns actually stored value instead of a copy.");
+    pointer.Pop(true).FreeSelf();
+    TEST_ExpectTrue(pointer.Pop(true).ToPlainString() == "test");
+    component = pointer.Pop();
+    TEST_ExpectNotNone(component);
+    TEST_ExpectTrue(component.ToPlainString() == "test");
+    TEST_ExpectTrue(component.IsAllocated());
+
+    Issue("`Pop(true)` breaks after regular `Pop()` call.");
+    TEST_ExpectTrue(pointer.Pop(true).ToPlainString() == "simple");
+    TEST_ExpectTrue(pointer.Pop(true).ToPlainString() == "simple");
+}
+
 protected static function Test_Print()
 {
     Context("Testing printing simple JSON values.");