HPCC-18818 Fix xml/json serialization issues

The intermediate property tree writer had a bug that caused
child datasets to have extra spurious nesting per row.
Use of the property tree also caused the field order to be lost.
Switch to using direct xml/json writers.

Signed-off-by: Jake Smith <jake.smith@lexisnexisrisk.com>

common/remote/sockfile.cpp

@@ -5495,20 +5495,17 @@ public:
 
         const char *outputFmtStr = requestTree->queryProp("format");
         int cursorHandle = requestTree->getPropInt("cursor");
-        Owned<IPropertyTree> responseTree; // Used if xml/json
         OutputFormat outputFormat;
-        if (!outputFmtStr || strieq("xml", outputFmtStr))
+        if (nullptr == outputFmtStr)
+            outputFormat = outFmt_Xml; // default
+        else if (strieq("xml", outputFmtStr))
             outputFormat = outFmt_Xml;
         else if (strieq("json", outputFmtStr))
             outputFormat = outFmt_Json;
-        else
+        else if (strieq("binary", outputFmtStr))
             outputFormat = outFmt_Binary;
-        if (outFmt_Binary != outputFormat)
-            responseTree.setown(createPTree("Response"));
-
-        MemoryBuffer cursorMb;
-        if (requestTree->getPropBin("cursorBin", cursorMb))
-            cursorMb.setEndian(__BIG_ENDIAN);
+        else
+            throw MakeStringException(0, "Unrecognised output format: %s", outputFmtStr);
 
         Owned<IRemoteActivity> outputActivity;
         OpenFileInfo fileInfo;
@@ -5531,13 +5528,23 @@ public:
         else // known handle, continuation
             outputActivity.set(fileInfo.activity);
 
-        if (outputActivity && cursorMb.length()) // use handle if one provided
+        if (outputActivity && requestTree->hasProp("cursorBin")) // use handle if one provided
+        {
+            MemoryBuffer cursorMb;
+            cursorMb.setEndian(__BIG_ENDIAN);
+            JBASE64_Decode(requestTree->queryProp("cursorBin"), cursorMb);
             outputActivity->restoreCursor(cursorMb);
+        }
 
-        if (outFmt_Binary != outputFormat)
-            responseTree->setPropInt("cursor", cursorHandle);
-        else
+        Owned<IXmlWriterExt> responseWriter; // for xml or json response
+        if (outFmt_Binary == outputFormat)
             reply.append(cursorHandle);
+        else // outFmt_Xml || outFmt_Json
+        {
+            responseWriter.setown(createIXmlWriterExt(0, 0, nullptr, outFmt_Xml == outputFormat ? WTStandard : WTJSON));
+            responseWriter->outputBeginNested("Response", true);
+            responseWriter->outputUInt(cursorHandle, sizeof(cursorHandle), "cursor");
+        }
         if (cursorHandle)
         {
             IOutputMetaData *out = outputActivity->queryOutputMeta();
@@ -5558,10 +5565,13 @@ public:
                     reply.append(rowSz, row);
                 }
                 dataLenMarker.write();
+                DelayedSizeMarker cursorLenMarker(reply); // cursor length
+                if (!eoi)
+                    outputActivity->serializeCursor(reply);
+                cursorLenMarker.write();
             }
             else
             {
-                CPropertyTreeWriter iptWriter;
                 for (unsigned __int64 i=0; i<defaultDaFSNumRecs; i++)
                 {
                     size32_t rowSz;
@@ -5571,47 +5581,26 @@ public:
                         eoi = true;
                         break;
                     }
-                    IPropertyTree *rowNode = responseTree->addPropTree("Row");
-                    iptWriter.setRoot(*rowNode);
-                    out->toXML((const byte *)row, iptWriter);
+                    responseWriter->outputBeginNested("Row", true);
+                    out->toXML((const byte *)row, *responseWriter);
+                    responseWriter->outputEndNested("Row");
                 }
-            }
-            if (outFmt_Binary != outputFormat)
-            {
                 if (!eoi)
                 {
                     MemoryBuffer cursorMb;
                     cursorMb.setEndian(__BIG_ENDIAN);
                     outputActivity->serializeCursor(cursorMb);
-                    responseTree->setPropBin("cursorBin", cursorMb.length(), cursorMb.toByteArray());
+                    StringBuffer cursorBinStr;
+                    JBASE64_Encode(cursorMb.toByteArray(), cursorMb.length(), cursorBinStr);
+                    responseWriter->outputString(cursorBinStr.length(), cursorBinStr.str(), "cursorBin");
                 }
             }
-            else
-            {
-                DelayedSizeMarker cursorLenMarker(reply); // cursor length
-                if (!eoi)
-                    outputActivity->serializeCursor(reply);
-                cursorLenMarker.write();
-            }
         }
-        switch (outputFormat)
+        if (outFmt_Binary != outputFormat)
         {
-            case outFmt_Xml:
-            {
-                StringBuffer responseXmlStr;
-                toXML(responseTree, responseXmlStr);
-                reply.append(responseXmlStr.length(), responseXmlStr.str());
-                break;
-            }
-            case outFmt_Json:
-            {
-                StringBuffer responseJsonStr;
-                toJSON(responseTree, responseJsonStr);
-                reply.append(responseJsonStr.length(), responseJsonStr.str());
-                break;
-            }
-            default:
-                break;
+            responseWriter->outputEndNested("Response");
+            PROGLOG("Response: %s", responseWriter->str());
+            reply.append(responseWriter->length(), responseWriter->str());
         }
         return true;
     }

rtl/eclrtl/rtlformat.cpp

@@ -934,14 +934,12 @@ void CPropertyTreeWriter::outputBeginDataset(const char *dsname, bool nestChildr
         return;
     StringBuffer dsNameUtf8;
     outputXmlUtf8(rtlUtf8Length(strlen(dsname), dsname), dsname, nullptr, dsNameUtf8);
-    target = target->addPropTree(dsNameUtf8);
-    nestedLevels.append(*target);
+    target->addProp("@name", dsNameUtf8);
 }
 
 void CPropertyTreeWriter::outputEndDataset(const char *dsname)
 {
     outputEndNested("Dataset");
-    target = &nestedLevels.popGet();
 }
 
 void CPropertyTreeWriter::outputBeginNested(const char *fieldname, bool nestChildren)
@@ -958,8 +956,8 @@ void CPropertyTreeWriter::outputBeginNested(const char *fieldname, bool nestChil
         return;
     }
 
-    target = target->addPropTree(fieldname);
     nestedLevels.append(*target);
+    target = target->addPropTree(fieldname);
 }
 
 void CPropertyTreeWriter::outputEndNested(const char *fieldname)

tools/testsocket/testsocket.cpp

@@ -191,7 +191,7 @@ int readResults(ISocket * socket, bool readBlocked, bool useHTTP, StringBuffer &
     if (readBlocked)
         socket->set_block_mode(BF_SYNC_TRANSFER_PULL,0,60*1000);
 
-    MemoryBuffer remoteReadCursorMb;
+    StringBuffer remoteReadCursor;
     unsigned len;
     bool is_status;
     bool isBlockedResult;
@@ -343,12 +343,19 @@ int readResults(ISocket * socket, bool readBlocked, bool useHTTP, StringBuffer &
             {
                 response = (const char *)mb.readDirect(len);
                 responseTree.setown(createPTreeFromXMLString(len, response));
+                assertex(responseTree);
             }
             else if (strieq("json", outputFmtStr))
             {
                 response = (const char *)mb.readDirect(len);
-                responseTree.setown(createPTreeFromJSONString(len, response));
+                // JCSMORE - json string coming back from IXmlWriterExt is always rootless the moment, so workaround it by supplying ptr_noRoot to reader
+                // writer should be fixed.
+                Owned<IPropertyTree> tree = createPTreeFromJSONString(len, response, ipt_none, (PTreeReaderOptions)(ptr_ignoreWhiteSpace|ptr_noRoot));
+                responseTree.setown(tree->getPropTree("Response"));
+                assertex(responseTree);
             }
+            else if (!strieq("binary", outputFmtStr))
+                throw MakeStringException(0, "Unknown output format: %s", outputFmtStr);
             unsigned cursorHandle;
             if (responseTree)
                 cursorHandle = responseTree->getPropInt("cursor");
@@ -366,7 +373,7 @@ int readResults(ISocket * socket, bool readBlocked, bool useHTTP, StringBuffer &
                         fputs(response, stdout);
                         fflush(stdout);
                     }
-                    if (!responseTree->getPropBin("cursorBin", remoteReadCursorMb.clear()))
+                    if (!responseTree->getProp("cursorBin", remoteReadCursor))
                         break;
                 }
                 else
@@ -384,7 +391,7 @@ int readResults(ISocket * socket, bool readBlocked, bool useHTTP, StringBuffer &
                     if (!cursorLen)
                         break;
                     const void *cursor = mb.readDirect(cursorLen);
-                    memcpy(remoteReadCursorMb.clear().reserveTruncate(cursorLen), cursor, cursorLen);
+                    JBASE64_Encode(cursor, cursorLen, remoteReadCursor);
                 }
 
                 if (remoteStreamForceResend)
@@ -395,13 +402,16 @@ int readResults(ISocket * socket, bool readBlocked, bool useHTTP, StringBuffer &
 
                 // Only the handle is needed for continuation, but this tests the behaviour of some clients which may send cursor per request (e.g. to refresh)
                 if (remoteStreamSendCursor)
-                    requestTree->setPropBin("cursorBin", remoteReadCursorMb.length(), remoteReadCursorMb.toByteArray());
+                    requestTree->setProp("cursorBin", remoteReadCursor);
 
                 requestTree->setProp("format", outputFmtStr);
                 StringBuffer requestStr;
                 toJSON(requestTree, requestStr);
 #ifdef _DEBUG
+                fputs("\nNext request:", stdout);
                 fputs(requestStr, stdout);
+                fputs("\n", stdout);
+                fflush(stdout);
 #endif
 
                 sendlen = requestStr.length();
@@ -425,7 +435,7 @@ int readResults(ISocket * socket, bool readBlocked, bool useHTTP, StringBuffer &
             if (retrySend)
             {
                 PROGLOG("Retry send for handle: %u", cursorHandle);
-                requestTree->setPropBin("cursorBin", remoteReadCursorMb.length(), remoteReadCursorMb.toByteArray());
+                requestTree->setProp("cursorBin", remoteReadCursor);
                 StringBuffer requestStr;
                 toJSON(requestTree, requestStr);