do more checking to prevent processing of bad messages

product/ERP5SyncML/Engine/EngineMixin.py

@@ -33,7 +33,8 @@ from Products.PluggableAuthService.interfaces.plugins import \
 
 from Products.ERP5SyncML.XMLSyncUtils import resolveSyncmlStatusCode, decode
 from Products.ERP5SyncML.SyncMLMessage import SyncMLResponse
-from Products.ERP5SyncML.SyncMLConstant import NULL_ANCHOR, ACTIVITY_PRIORITY
+from Products.ERP5SyncML.SyncMLConstant import NULL_ANCHOR, ACTIVITY_PRIORITY, \
+    SynchronizationError
 
 syncml_logger = getLogger('ERP5SyncML')
 
@@ -64,6 +65,9 @@ class EngineMixin(object):
     sync_status_counter = 0
     for status in syncml_request.status_list:
       if status["command"] == "SyncHdr":  # Check for authentication
+        if domain.getSynchronizationState() != "initializing":
+          raise SynchronizationError(
+            "Authentication header found although it is already done")
         if status['status_code'] == \
             resolveSyncmlStatusCode('missing_credentials'):
           # Server challenged an authentication

product/ERP5SyncML/Engine/SynchronousEngine.py

@@ -28,7 +28,7 @@
 from logging import getLogger
 
 from Products.ERP5SyncML.Engine.EngineMixin import EngineMixin
-
+from Products.ERP5SyncML.SyncMLConstant import SynchronizationError
 
 syncml_logger = getLogger('ERP5SyncML')
 
@@ -57,7 +57,13 @@ class SyncMLSynchronousEngine(EngineMixin):
     syncml_response = self._generateBaseResponse(subscription)
 
     # Read & apply status about databases & synchronizations
+    try:
       self._readStatusList(syncml_request, subscription, syncml_response)
+    except SynchronizationError:
+      # Looks like we process an already received message
+      syncml_logger.error("%s does no process packet due to error"
+                          % (subscription.getRelativeUrl()))
+      return
 
     if syncml_request.isFinal and \
       subscription.getSynchronizationState() == "initializing":
@@ -148,11 +154,14 @@ class SyncMLSynchronousEngine(EngineMixin):
       # Apply status about object send & synchronized if any
       self._readStatusList(syncml_request, subscriber, syncml_response, True)
 
-      if syncml_request.isFinal and \
-        subscriber.getSynchronizationState() == \
-        "sending_modifications":
+      if syncml_request.isFinal:
+        if subscriber.getSynchronizationState() == \
+              "waiting_notifications":
           # We got the last notifications from clients
           subscriber.finish()
+        elif subscriber.getSynchronizationState() != \
+              "processing_sync_requests":
+          raise SynchronizationError("Got final request although not waiting for it")
 
       # XXX We compute gid list so that we do not get issue with catalog
       # XXX This is a hack, if real datasynchronization is implemented
@@ -202,6 +211,7 @@ class SyncMLSynchronousEngine(EngineMixin):
         if finished:
           subscriber._getDeletedData(syncml_response=syncml_response)
           syncml_response.addFinal()
+          subscriber.waitNotifications()
           # Do not go into finished here as we must wait for
           # notifications from client
       if subscriber.getSynchronizationState() == "finished":

product/ERP5SyncML/SyncMLConstant.py

@@ -99,3 +99,5 @@ REPLACE_ACTION = 'Replace'
 ACTIVITY_PRIORITY = 5
 
 
+class SynchronizationError(Exception):
+    pass

product/ERP5SyncML/Tool/SynchronizationTool.py

@@ -34,7 +34,8 @@ from AccessControl import ClassSecurityInfo
 from Products.ERP5Type.Tool.BaseTool import BaseTool
 from Products.ERP5Type import Permissions
 from Products.ERP5Type.Globals import InitializeClass
-from Products.ERP5SyncML.SyncMLConstant import ACTIVITY_PRIORITY
+from Products.ERP5SyncML.SyncMLConstant import ACTIVITY_PRIORITY, \
+    SynchronizationError
 from Products.ERP5SyncML.SyncMLMessage import SyncMLResponse, SyncMLRequest
 from Products.ERP5SyncML.Engine.SynchronousEngine import SyncMLSynchronousEngine
 from Products.ERP5SyncML.Engine.AsynchronousEngine import SyncMLAsynchronousEngine
@@ -319,6 +320,13 @@ class SynchronizationTool(BaseTool):
       assert len(database_alert_list) <= 1, "Multi-databases sync no supported"
       if len(database_alert_list):
         # We are initializing the synchronization
+        if subscriber and subscriber.getSynchronizationState() not in \
+              ("not_running", "finished"):
+          syncml_logger.error(
+            'Trying to start a synchronization on server side : %s although synchronisation is already running'
+            % (subscriber.getPath(),))
+          # Prevent initilisation if sync already running
+          return
         syncml_response = engine.processServerInitialization(
           publication=publication,
           syncml_request=syncml_request,
@@ -329,7 +337,10 @@ class SynchronizationTool(BaseTool):
           raise ValueError("First synchronization message must contains alert command")
         else:
           # Let engine manage the synchronization
+          try:
             return engine.processServerSynchronization(subscriber, syncml_request)
+          except SynchronizationError:
+            return
     else:
       # This must be implemented following the syncml protocol, not with this hack
       raise NotImplementedError("Starting sync process from server is forbidden")