2024-04-29 Fred Gleason <fredg@paravelsystems.com>

* Fixed a race condition in the PAD XML feed that could cause corrupt
	updates.

Signed-off-by: Fred Gleason <fredg@paravelsystems.com>

ChangeLog

@@ -24732,3 +24732,6 @@
 	* Documented the '[Debugging]' section in the rd.conf(5) man page.
 2024-04-26 Fred Gleason <fredg@paravelsystems.com>
 	* Incremented the package version to 4.2.1int0
+2024-04-29 Fred Gleason <fredg@paravelsystems.com>
+	* Fixed a race condition in the PAD XML feed that could cause corrupt
+	updates.

apis/pypad/api/pypad.py

@@ -941,9 +941,12 @@ class Receiver(object):
         sel=selectors.DefaultSelector()
         sel.register(sock,selectors.EVENT_READ)
         c=bytes()
-        line=bytes()
-        msg=""
+        msg=bytearray()
 
+        # Process updates
+        bracket_count=0
+        escaped=False
+        quoted=False
         while 1<2:
             if len(sel.select(timeout))==0:
                 now=datetime.datetime.now()
@@ -955,28 +958,34 @@ class Receiver(object):
                     timeout=(deadline-now).total_seconds()
             else:
                 c=sock.recv(1)
-                line+=c
-                if c[0]==10:
-                    linebytes=line.decode('utf-8','replace')
-                    msg+=linebytes
-                    if linebytes=='\n':
-                        ok=False
-                        try:
-                            jdata=json.loads(msg)
-                            ok=True
-                        except:
-                            priority=syslog.LOG_WARNING|(int(rd_config.get('Identity','SyslogFacility',fallback=syslog.LOG_USER))<<3)
-                            syslog.syslog(priority,'error parsing JSON: "'+msg+'"')
-                            if rd_config.get('Debugging','KillPypadAfterJsonError',fallback='no').lower()=='yes':
-                                sys.exit(1)
-                        if ok:
-                            if (not self.__active_now_groups and not self.__active_next_groups) or (jdata['padUpdate'] is not None and jdata['padUpdate']['now'] is not None and jdata['padUpdate']['now']['groupName'] in self.__active_now_groups) or (jdata['padUpdate'] is not None and jdata['padUpdate']['next'] is not None and jdata['padUpdate']['next']['groupName'] in self.__active_next_groups):
-                                self.__pypad_Process(Update(jdata,self.__config_parser,rd_config))
-                        msg=""
-                    line=bytes()
-                if self.__timer_interval!=None:
-                    timeout=(deadline-datetime.datetime.now()).total_seconds()
-
+                msg.append(c[0])
+                if (c[0]==92)and(not escaped):   # Matches '\'
+                    escaped=True
+                else:
+                    if (c[0]==34)and(not escaped):   # Matches '"'
+                        quoted=not quoted
+                    else:
+                        if (c[0]==123)and(not quoted):  # Matches '{'
+                            bracket_count+=1
+                        if (c[0]==125)and(not quoted):  # Matches '}'
+                            bracket_count-=1
+                            if bracket_count==0:
+                                ok=False
+                                try:
+                                    jdata=json.loads(msg)
+                                    ok=True
+                                except:
+                                    priority=syslog.LOG_WARNING|(int(rd_config.get('Identity','SyslogFacility',fallback=syslog.LOG_USER))<<3)
+                                    syslog.syslog(priority,'error parsing JSON: "'+msg.decode('utf-8','replace')+'"')
+                                    if rd_config.get('Debugging','KillPypadAfterJsonError',fallback='no').lower()=='yes':
+                                        sys.exit(1)
+                                if ok:
+                                    if (not self.__active_now_groups and not self.__active_next_groups) or (jdata['padUpdate'] is not None and jdata['padUpdate']['now'] is not None and jdata['padUpdate']['now']['groupName'] in self.__active_now_groups) or (jdata['padUpdate'] is not None and jdata['padUpdate']['next'] is not None and jdata['padUpdate']['next']['groupName'] in self.__active_next_groups):
+                                                self.__pypad_Process(Update(jdata,self.__config_parser,rd_config))
+                                msg=bytearray()
+                        if self.__timer_interval!=None:
+                            timeout=(deadline-datetime.datetime.now()).total_seconds()
+                    escaped=False
 
 def SigHandler(signo,stack):
     sys.exit(0)

lib/Makefile.am

@@ -163,6 +163,7 @@ dist_librd_la_SOURCES = dbversion.h\
                         rdimport_audio.cpp rdimport_audio.h\
                         rdinstancelock.cpp rdinstancelock.h\
                         rdjackclientlistmodel.cpp rdjackclientlistmodel.h\
+                        rdjsonframer.cpp rdjsonframer.h\
                         rdkernelgpio.cpp rdkernelgpio.h\
                         rdlibrary_conf.cpp rdlibrary_conf.h\
                         rdlibrarymodel.cpp rdlibrarymodel.h\
@@ -363,6 +364,7 @@ nodist_librd_la_SOURCES = moc_rdadd_cart.cpp\
                           moc_rdimagepickermodel.cpp\
                           moc_rdimport_audio.cpp\
                           moc_rdjackclientlistmodel.cpp\
+                          moc_rdjsonframer.cpp\
                           moc_rdkernelgpio.cpp\
                           moc_rdlibrarymodel.cpp\
                           moc_rdlineedit.cpp\

lib/librd.pro

@@ -127,6 +127,7 @@ SOURCES += rdimagepickermodel.cpp
 SOURCES += rdimport_audio.cpp
 SOURCES += rdkernelgpio.cpp
 SOURCES += rdjackclientlistmodel.cpp
+SOURCES += rdjsonframer.cpp
 SOURCES += rdlibrary_conf.cpp
 SOURCES += rdlibrarymodel.cpp
 SOURCES += rdlineedit.cpp
@@ -319,6 +320,7 @@ HEADERS += rdimagepickerbox.h
 HEADERS += rdimagepickermodel.h
 HEADERS += rdimport_audio.h
 HEADERS += rdjackclientlistmodel.h
+HEADERS += rdjsonframer.h
 HEADERS += rdkernelgpio.h
 HEADERS += rdlibrary_conf.h
 HEADERS += rdlibrarymodel.h

lib/rdjsonframer.cpp

@@ -0,0 +1,115 @@
+//   rdjsonframer.cpp
+//
+//   Frame an unsynchronized stream of JSON messages
+//
+//   (C) Copyright 2024 Fred Gleason <fredg@paravelsystems.com>
+//
+//   This program is free software; you can redistribute it and/or modify
+//   it under the terms of the GNU Library General Public License 
+//   version 2 as published by the Free Software Foundation.
+//
+//   This program is distributed in the hope that it will be useful,
+//   but WITHOUT ANY WARRANTY; without even the implied warranty of
+//   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+//   GNU General Public License for more details.
+//
+//   You should have received a copy of the GNU General Public
+//   License along with this program; if not, write to the Free Software
+//   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+//
+
+#include "rdjsonframer.h"
+
+RDJsonFramer::RDJsonFramer(QTcpSocket *in_sock,QObject *parent)
+  : QObject(parent)
+{
+  d_escaped=false;
+  d_quoted=false;
+  d_level=0;
+  d_socket=in_sock;
+  connect(d_socket,SIGNAL(readyRead()),this,SLOT(readyReadData()));
+}
+
+
+RDJsonFramer::RDJsonFramer(QObject *parent)
+  : QObject(parent)
+{
+  d_escaped=false;
+  d_quoted=false;
+  d_level=0;
+  d_socket=NULL;
+}
+
+
+RDJsonFramer::~RDJsonFramer()
+{
+  if(d_socket!=NULL) {
+    delete d_socket;
+  }
+}
+
+
+QByteArray RDJsonFramer::currentDocument() const
+{
+  return d_current_document;
+}
+
+
+void RDJsonFramer::write(const QByteArray &data)
+{
+  for(int i=0;i<data.size();i++) {
+    QChar c=data.at(i);
+
+    d_data+=c;
+
+    if((c.cell()=='\\')&&(!d_escaped)) {
+      d_escaped=true;
+    }
+    else {
+      switch(c.cell()) {
+      case '{':
+	if(!d_quoted) {
+	  d_level++;
+	}
+	break;
+
+      case '}':
+	if(!d_quoted) {
+	  if(--d_level==0) {
+	    d_current_document=d_data+"\n";
+	    d_escaped=false;
+	    d_quoted=false;
+	    d_level=0;
+	    d_data.clear();
+	    emit documentReceived(d_current_document);
+	  }
+	}
+	break;
+
+      case '"':
+	if(!d_escaped) {
+	  d_quoted=!d_quoted;
+	}
+	break;
+      }
+      d_escaped=false;
+    }
+  }
+}
+
+
+void RDJsonFramer::reset()
+{
+  d_escaped=false;
+  d_quoted=false;
+  d_level=0;
+  d_data.clear();
+
+  emit documentReset();  
+}
+
+
+void RDJsonFramer::readyReadData()
+{
+  write(d_socket->readAll());
+}

lib/rdjsonframer.h

@@ -0,0 +1,58 @@
+//   rdjsonframer.h
+//
+//   Frame an unsynchronized stream of JSON messages
+//
+//   (C) Copyright 2024 Fred Gleason <fredg@paravelsystems.com>
+//
+//   This program is free software; you can redistribute it and/or modify
+//   it under the terms of the GNU Library General Public License 
+//   version 2 as published by the Free Software Foundation.
+//
+//   This program is distributed in the hope that it will be useful,
+//   but WITHOUT ANY WARRANTY; without even the implied warranty of
+//   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+//   GNU General Public License for more details.
+//
+//   You should have received a copy of the GNU General Public
+//   License along with this program; if not, write to the Free Software
+//   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+//
+
+#ifndef RDJSONFRAMER_H
+#define RDJSONFRAMER_H
+
+#include <QByteArray>
+#include <QObject>
+#include <QTcpSocket>
+
+class RDJsonFramer : public QObject
+{
+  Q_OBJECT
+ public:
+  RDJsonFramer(QTcpSocket *in_sock,QObject *parent);
+  RDJsonFramer(QObject *parent);
+  ~RDJsonFramer();
+  QByteArray currentDocument() const;
+  
+ public slots:
+  void write(const QByteArray &data);
+  void reset();
+
+ signals:
+  void documentReceived(const QByteArray &jdoc);
+  void documentReset();
+
+ private slots:
+  void readyReadData();
+
+ private:
+  QByteArray d_data;
+  bool d_escaped;
+  bool d_quoted;
+  int d_level;
+  QByteArray d_current_document;
+  QTcpSocket *d_socket;
+};
+
+
+#endif  // RDJSONFRAMER_H

rdpadd/repeater.cpp

@@ -27,64 +27,6 @@
 
 #include "repeater.h"
 
-MetadataSource::MetadataSource(QTcpSocket *sock)
-{
-  meta_socket=sock;
-  meta_curly_count=0;
-  meta_quoted=false;
-  meta_committed=true;
-}
-
-
-QByteArray MetadataSource::buffer() const
-{
-  return meta_buffer;
-}
-
-
-bool MetadataSource::appendBuffer(const QByteArray &data)
-{
-  if(meta_committed) {
-    meta_buffer.clear();
-  }
-  meta_buffer+=data;
-
-  for(int i=0;i<data.size();i++) {
-    if(data.at(i)=='"') {
-      meta_quoted=!meta_quoted;
-    }
-    if(!meta_quoted) {
-      if(data.at(i)=='{') {
-	meta_curly_count++;
-      }
-      if(data.at(i)=='}') {
-	meta_curly_count--;
-      }
-    }
-  }
-  meta_committed=(meta_curly_count==0);
-  if(meta_committed) {
-    meta_buffer+="\n\n";
-  }
-
-  return meta_committed;
-}
-
-
-bool MetadataSource::isCommitted() const
-{
-  return meta_committed;
-}
-
-
-QTcpSocket *MetadataSource::socket() const
-{
-  return meta_socket;
-}
-
-
-
-
 Repeater::Repeater(const QString &src_unix_addr,uint16_t serv_port,
 		   QObject *parent)
   : QObject(parent)
@@ -110,10 +52,6 @@ Repeater::Repeater(const QString &src_unix_addr,uint16_t serv_port,
   //
   // Source Server
   //
-  pad_source_ready_mapper=new QSignalMapper(this);
-  connect(pad_source_ready_mapper,SIGNAL(mapped(int)),
-	  this,SLOT(sourceReadyReadData(int)));
-
   pad_source_disconnect_mapper=new QSignalMapper(this);
   connect(pad_source_disconnect_mapper,SIGNAL(mapped(int)),
 	  this,SLOT(sourceDisconnected(int)));
@@ -148,7 +86,10 @@ void Repeater::newClientConnectionData()
   pad_client_disconnect_mapper->setMapping(sock,sock->socketDescriptor());
   pad_client_sockets[sock->socketDescriptor()]=sock;
 
-  SendState(sock->socketDescriptor());
+  for(QMap<int,RDJsonFramer *>::const_iterator it=pad_framers.begin();
+      it!=pad_framers.end();it++) {
+    sock->write(it.value()->currentDocument());
+  }
 }
 
 
@@ -174,35 +115,21 @@ void Repeater::newSourceConnectionData()
 	    (const char *)pad_source_server->errorString().toUtf8());
     exit(1);
   }
-  connect(sock,SIGNAL(readyRead()),pad_source_ready_mapper,SLOT(map()));
-  pad_source_ready_mapper->setMapping(sock,sock->socketDescriptor());
-
   connect(sock,SIGNAL(disconnected()),pad_source_disconnect_mapper,SLOT(map()));
   pad_source_disconnect_mapper->setMapping(sock,sock->socketDescriptor());
 
-  pad_sources[sock->socketDescriptor()]=new MetadataSource(sock);
-}
-
-
-void Repeater::sourceReadyReadData(int id)
-{
-  if(pad_sources[id]!=NULL) {
-    if(pad_sources[id]->appendBuffer(pad_sources[id]->socket()->readAll())) {
-      for(QMap<int,QTcpSocket *>::const_iterator it=pad_client_sockets.begin();
-	  it!=pad_client_sockets.end();it++) {
-	it.value()->write(pad_sources[id]->buffer());
-      }
-    }
-  }
+  RDJsonFramer *framer=new RDJsonFramer(sock,this);
+  connect(framer,SIGNAL(documentReceived(const QByteArray &)),
+	  this,SLOT(sendUpdate(const QByteArray &)));
+  pad_framers[sock->socketDescriptor()]=framer;
 }
 
 
 void Repeater::sourceDisconnected(int id)
 {
-  if(pad_sources.value(id)!=NULL) {
-    pad_sources.value(id)->socket()->deleteLater();
-    delete pad_sources.value(id);
-    pad_sources.remove(id);
+  if(pad_framers.value(id)!=NULL) {
+    pad_framers.value(id)->deleteLater();
+    pad_framers.remove(id);
   }
   else {
     fprintf(stderr,"unknown source connection %d attempted to close\n",id);
@@ -210,12 +137,10 @@ void Repeater::sourceDisconnected(int id)
 }
 
 
-void Repeater::SendState(int id)
+void Repeater::sendUpdate(const QByteArray &jdoc)
 {
-  for(QMap<int,MetadataSource *>::const_iterator it=pad_sources.begin();
-      it!=pad_sources.end();it++) {
-    if(it.value()->isCommitted()&&(!it.value()->buffer().trimmed().isEmpty())) {
-      pad_client_sockets.value(id)->write(it.value()->buffer());
-    }
+  for(QMap<int,QTcpSocket *>::const_iterator it=pad_client_sockets.begin();
+      it!=pad_client_sockets.end();it++) {
+    it.value()->write(jdoc);
   }
 }

rdpadd/repeater.h

@@ -27,30 +27,11 @@
 #include <QTcpServer>
 #include <QTcpSocket>
 
+#include <rdjsonframer.h>
 #include <rdunixserver.h>
 
 #include "repeater.h"
 
-class MetadataSource
-{
- public:
-  MetadataSource(QTcpSocket *sock);
-  QByteArray buffer() const;
-  bool appendBuffer(const QByteArray &data);
-  bool isCommitted() const;
-  QTcpSocket *socket() const;
-
- private:
-  QByteArray meta_buffer;
-  int meta_curly_count;
-  bool meta_quoted;
-  bool meta_committed;
-  QTcpSocket *meta_socket;
-};
-
-
-
-
 class Repeater : public QObject
 {
   Q_OBJECT
@@ -63,20 +44,18 @@ class Repeater : public QObject
   void newClientConnectionData();
   void clientDisconnected(int id);
   void newSourceConnectionData();
-  void sourceReadyReadData(int id);
   void sourceDisconnected(int id);
+  void sendUpdate(const QByteArray &jdoc);
 
  private:
-  void SendState(int id);
   uint16_t pad_server_port;
   QString pad_source_unix_address;
   QSignalMapper *pad_client_disconnect_mapper;
   QTcpServer *pad_client_server;
   QMap<int,QTcpSocket *> pad_client_sockets;
-  QSignalMapper *pad_source_ready_mapper;
   QSignalMapper *pad_source_disconnect_mapper;
   RDUnixServer *pad_source_server;
-  QMap<int,MetadataSource *> pad_sources;
+  QMap<int,RDJsonFramer *> pad_framers;
 };