Server: catch Python exceptions from plugins

Make sure Python exceptions are converted into QgsServerException and catched by the main service executor. Without this patch, the server process is terminated (aborted).
qgis · Aug 19, 2019 · 4cc7a0f · 4cc7a0f
1 parent 037bdef
commit 4cc7a0f
Show file tree

Hide file tree

Showing 8 changed files with 84 additions and 11 deletions.
diff --git a/python/server/auto_generated/qgsserverresponse.sip.in b/python/server/auto_generated/qgsserverresponse.sip.in
@@ -109,17 +109,18 @@ Write server exception
 Returns the underlying QIODevice
 %End
 
-    virtual void finish() = 0;
+    virtual void finish() throw( QgsServerException ) /VirtualErrorHandler=server_exception_handler/;
 %Docstring
-Finish the response,  ending the transaction
+Finish the response, ending the transaction. The default implementation does nothing.
 %End
 
-    virtual void flush() = 0;
+    virtual void flush() throw( QgsServerException ) /VirtualErrorHandler=server_exception_handler/;
 %Docstring
 Flushes the current output buffer to the network
 
 'flush()' may be called multiple times. For HTTP transactions
 headers will be written on the first call to 'flush()'.
+The default implementation does nothing.
 %End
 
     virtual void clear() = 0;

diff --git a/python/server/server.sip.in b/python/server/server.sip.in
@@ -27,3 +27,17 @@ ${DEFAULTDOCSTRINGSIGNATURE}
     }
     throw QgsServerApiBadRequestException( strVal );
 %End
+
+
+%VirtualErrorHandler server_exception_handler
+    PyObject *exception, *value, *traceback;
+    PyErr_Fetch(&exception, &value, &traceback);
+    SIP_RELEASE_GIL( sipGILState );
+    QString strVal = "Server internal error";
+    if ( value && PyUnicode_Check(value) )
+    {
+      Py_ssize_t size;
+      strVal = QString::fromUtf8( PyUnicode_AsUTF8AndSize(value, &size) );
+    }
+    throw QgsServerException( strVal );
+%End
diff --git a/src/server/qgsfilterresponsedecorator.cpp b/src/server/qgsfilterresponsedecorator.cpp
@@ -19,6 +19,7 @@
 
 #include "qgsconfig.h"
 #include "qgsfilterresponsedecorator.h"
+#include "qgsserverexception.h"
 
 QgsFilterResponseDecorator::QgsFilterResponseDecorator( QgsServerFiltersMap filters, QgsServerResponse &response )
   : mFilters( filters )

diff --git a/src/server/qgsfilterresponsedecorator.h b/src/server/qgsfilterresponsedecorator.h
@@ -24,6 +24,7 @@
 
 #include "qgsserverresponse.h"
 #include "qgsserverfilter.h"
+#include "qgsserverexception.h"
 
 /**
  * \ingroup server
@@ -45,7 +46,7 @@ class QgsFilterResponseDecorator: public QgsServerResponse
     /**
      * Call filters requestReady() method
      */
-    void start();
+    void start() SIP_THROW( QgsServerException ) SIP_VIRTUALERRORHANDLER( server_exception_handler );
 
     // QgsServerResponse overrides
 

diff --git a/src/server/qgsserver.cpp b/src/server/qgsserver.cpp
@@ -345,7 +345,17 @@ void QgsServer::handleRequest( QgsServerRequest &request, QgsServerResponse &res
   }
 
   // Call  requestReady() method (if enabled)
-  responseDecorator.start();
+  // This may also throw exceptions if there are errors in python plugins code
+  try
+  {
+    responseDecorator.start();
+  }
+  catch ( QgsException &ex )
+  {
+    // Internal server error
+    response.sendError( 500, QStringLiteral( "Internal Server Error" ) );
+    QgsMessageLog::logMessage( ex.what(), QStringLiteral( "Server" ), Qgis::Critical );
+  }
 
   // Plugins may have set exceptions
   if ( !requestHandler.exceptionRaised() )
@@ -418,8 +428,20 @@ void QgsServer::handleRequest( QgsServerRequest &request, QgsServerResponse &res
       QgsMessageLog::logMessage( ex.what(), QStringLiteral( "Server" ), Qgis::Critical );
     }
   }
+
   // Terminate the response
-  responseDecorator.finish();
+  // This may also throw exceptions if there are errors in python plugins code
+  try
+  {
+    responseDecorator.finish();
+  }
+  catch ( QgsException &ex )
+  {
+    // Internal server error
+    response.sendError( 500, QStringLiteral( "Internal Server Error" ) );
+    QgsMessageLog::logMessage( ex.what(), QStringLiteral( "Server" ), Qgis::Critical );
+  }
+
 
   // We are done using requestHandler in plugins, make sure we don't access
   // to a deleted request handler from Python bindings

diff --git a/src/server/qgsserverresponse.cpp b/src/server/qgsserverresponse.cpp
@@ -65,6 +65,16 @@ qint64 QgsServerResponse::write( const char *data )
   return 0;
 }
 
+void QgsServerResponse::finish()
+{
+
+}
+
+void QgsServerResponse::flush()
+{
+
+}
+
 qint64 QgsServerResponse::write( const std::string data )
 {
   return write( data.c_str() );

diff --git a/src/server/qgsserverresponse.h b/src/server/qgsserverresponse.h
@@ -21,6 +21,7 @@
 
 #include "qgis_server.h"
 #include "qgis_sip.h"
+#include "qgsserverexception.h"
 
 #include <QString>
 #include <QIODevice>
@@ -159,17 +160,18 @@ class SERVER_EXPORT QgsServerResponse
     virtual QIODevice *io() = 0;
 
     /**
-     * Finish the response,  ending the transaction
+     * Finish the response, ending the transaction. The default implementation does nothing.
      */
-    virtual void finish() = 0;
+    virtual void finish() SIP_THROW( QgsServerException ) SIP_VIRTUALERRORHANDLER( server_exception_handler );
 
     /**
      * Flushes the current output buffer to the network
      *
      * 'flush()' may be called multiple times. For HTTP transactions
      * headers will be written on the first call to 'flush()'.
+     * The default implementation does nothing.
      */
-    virtual void flush() = 0;
+    virtual void flush() SIP_THROW( QgsServerException ) SIP_VIRTUALERRORHANDLER( server_exception_handler );
 
     /**
      * Reset all headers and content for this response

diff --git a/tests/src/python/test_qgsserver_plugins.py b/tests/src/python/test_qgsserver_plugins.py
@@ -161,15 +161,15 @@ def responseComplete(self):
         self.assertTrue(filter2 in serverIface.filters()[100])
         self.assertEqual(filter1, serverIface.filters()[101][0])
         self.assertEqual(filter2, serverIface.filters()[200][0])
-        header, body = [_v for _v in self._execute_request('?service=simple')]
+        header, body = self._execute_request('?service=simple')
         response = header + body
         expected = b'Content-Length: 62\nContent-type: text/plain\n\nHello from SimpleServer!Hello from Filter1!Hello from Filter2!'
         self.assertEqual(response, expected)
 
         # Now, re-run with body setter
         filter5 = Filter5(serverIface)
         serverIface.registerFilter(filter5, 500)
-        header, body = [_v for _v in self._execute_request('?service=simple')]
+        header, body = self._execute_request('?service=simple')
         response = header + body
         expected = b'Content-Length: 19\nContent-type: text/plain\n\nnew body, new life!'
         self.assertEqual(response, expected)
@@ -220,6 +220,28 @@ def requestReady(self):
         # Check config file path
         self.assertEqual(configFilePath2, project.fileName())
 
+    def test_exceptions(self):
+        """Test that plugin filter Python exceptions can be caught"""
+
+        try:
+            from qgis.server import QgsServerFilter
+        except ImportError:
+            print("QGIS Server plugins are not compiled. Skipping test")
+            return
+
+        class FilterBroken(QgsServerFilter):
+
+            def responseComplete(self):
+                raise Exception("There was something very wrong!")
+
+        serverIface = self.server.serverInterface()
+        filter1 = FilterBroken(serverIface)
+        filters = {100: [filter1]}
+        serverIface.setFilters(filters)
+        header, body = self._execute_request('')
+        self.assertEqual(body, b'Internal Server Error')
+        serverIface.setFilters({})
+
 
 if __name__ == '__main__':
     unittest.main()