From aae2bf28db36ab9133829dc33ea6ef886e8373c2 Mon Sep 17 00:00:00 2001
From: Jeremy Harris <jgh146exb@wizmail.org>
Date: Sat, 8 Jul 2023 17:59:20 +0100
Subject: [PATCH] Fix json extract for strings carrying commas.  Bug 3006

---
 doc/doc-txt/ChangeLog        |  5 +++++
 src/src/expand.c             | 11 +++++++++--
 test/aux-fixed/policy.json   |  5 +++--
 test/scripts/0000-Basic/0002 |  7 +++++++
 test/scripts/2750-json/2750  |  3 +++
 test/stdout/0002             |  7 +++++++
 test/stdout/2750             |  3 +++
 7 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 9c073f3e0..a3b43b2f5 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -158,6 +158,11 @@ JH/29 Change format of the internal ID used for message identification. The old
       back to old (losing time-precision and PID information) and remove any
       wait- hints databases.
 
+JH/30 Bug 3006: Fix handling of JSON strings having embedded commas. Previously
+      we treated them as item separators when parsing for a list item, but they
+      need to be protected by the doublequotes.  While there, add handling for
+      backslashes.
+
 Exim version 4.96
 -----------------
 
diff --git a/src/src/expand.c b/src/src/expand.c
index 55c53957e..fea6501fe 100644
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -2384,19 +2384,26 @@ static uschar *
 json_nextinlist(const uschar ** list)
 {
 unsigned array_depth = 0, object_depth = 0;
+BOOL quoted = FALSE;
 const uschar * s = *list, * item;
 
 skip_whitespace(&s);
 
 for (item = s;
-     *s && (*s != ',' || array_depth != 0 || object_depth != 0);
+     *s && (*s != ',' || array_depth != 0 || object_depth != 0 || quoted);
      s++)
-  switch (*s)
+  if (!quoted) switch (*s)
     {
     case '[': array_depth++; break;
     case ']': array_depth--; break;
     case '{': object_depth++; break;
     case '}': object_depth--; break;
+    case '"': quoted = TRUE;
+    }
+  else switch(*s)
+    {
+    case '\\': s++; break;		/* backslash protects one char */
+    case '"':  quoted = FALSE; break;
     }
 *list = *s ? s+1 : s;
 if (item == s) return NULL;
diff --git a/test/aux-fixed/policy.json b/test/aux-fixed/policy.json
index 8f31ec902..f7802c1ad 100644
--- a/test/aux-fixed/policy.json
+++ b/test/aux-fixed/policy.json
@@ -28,7 +28,8 @@
       "mxs": [
         ".yahoodns.net"
       ]
-    }
+    },
+    "key_for_string_with_comma": "Doe, John"
   },
   "policies": {
     "aol.com": {
@@ -2005,4 +2006,4 @@
       ]
     }
   }
-}
\ No newline at end of file
+}
diff --git a/test/scripts/0000-Basic/0002 b/test/scripts/0000-Basic/0002
index 58ec29250..dab982253 100644
--- a/test/scripts/0000-Basic/0002
+++ b/test/scripts/0000-Basic/0002
@@ -982,6 +982,13 @@ expect: <>
 <${extract jsons{nonexistent}{ \{"id": \{"a":101, "b":102\}, "IDs": \{"1":116, "2":943, "3":234\}\} }}>
 expect: <>
 
+# string value with embedded comma
+<${extract jsons{name}{ \{ "id":"1","name":"Doe, John","age":"unknown" \}}}>
+expect <Doe, John>
+# string value with embedded doublequote
+<${extract jsons{name}{ \{ "id":"1","name":"word1 \\\" word2","age":"unknown" \}}}>
+expect <word1 \\\" word2>
+
 ${if forany_json {[1, 2, 3]}{={$item}{1}}{yes}{no}}
 ${if forany_jsons{["A", "B", "C"]}{eq{$item}{B}}{yes}{no}}
 
diff --git a/test/scripts/2750-json/2750 b/test/scripts/2750-json/2750
index f01414b4c..47f5e13cf 100644
--- a/test/scripts/2750-json/2750
+++ b/test/scripts/2750-json/2750
@@ -19,4 +19,7 @@ ${lookup {policy-aliases : outlook : mxs : 1} json {DIR/aux-fixed/policy.json}}
 aggregate output vs. json extract
 ${extract json {mxs} \
 	{${lookup {policy-aliases:outlook} json {DIR/aux-fixed/policy.json}}}}
+
+string with embedded comma
+${lookup {policy-aliases:key_for_string_with_comma} json {DIR/aux-fixed/policy.json}}
 ****
diff --git a/test/stdout/0002 b/test/stdout/0002
index 1da46e7a0..5b9de8e5e 100644
--- a/test/stdout/0002
+++ b/test/stdout/0002
@@ -945,6 +945,13 @@ xyz
 > <>
 > expect: <>
 > 
+> # string value with embedded comma
+> <Doe, John>
+> expect <Doe, John>
+> # string value with embedded doublequote
+> <word1 \" word2>
+> expect <word1 \" word2>
+> 
 > yes
 > yes
 > 
diff --git a/test/stdout/2750 b/test/stdout/2750
index d70041a16..11b85cdcf 100644
--- a/test/stdout/2750
+++ b/test/stdout/2750
@@ -16,3 +16,6 @@
 > aggregate output vs. json extract
 > [".outlook.com", "outlook.com"]
 > 
+> string with embedded comma
+> Doe, John
+> 
-- 
2.30.2