Implement arguments check before deserialization

martinhaefner · Dec 31, 2017 · d3ebbf1 · d3ebbf1
1 parent 95b72a2
commit d3ebbf1
Show file tree

Hide file tree

Showing 19 changed files with 121 additions and 100 deletions.
diff --git a/include/simppl/bool.h b/include/simppl/bool.h
@@ -17,6 +17,7 @@ template<typename T>
 struct Codec;
 
 
+// FIXME implement in .cpp
 template<>
 struct Codec<bool>
 {
@@ -32,9 +33,8 @@ struct Codec<bool>
    void decode(DBusMessageIter& iter, bool& t)
    {
       dbus_bool_t b;
-      dbus_message_iter_get_basic(&iter, &b);
-      dbus_message_iter_next(&iter);
-
+      simppl_dbus_message_iter_get_basic(&iter, &b, DBUS_TYPE_BOOLEAN);
+
       t = b;
    }
 

diff --git a/include/simppl/buffer.h b/include/simppl/buffer.h
@@ -85,6 +85,7 @@ struct FixedSizeBuffer
 };
 
 
+// FIXME make slim implementation in src file
 template<size_t len>
 struct Codec<FixedSizeBuffer<len>>
 {
@@ -103,7 +104,7 @@ struct Codec<FixedSizeBuffer<len>>
    void decode(DBusMessageIter& iter, FixedSizeBuffer<len>& b)
    {
       DBusMessageIter _iter;
-      dbus_message_iter_recurse(&iter, &_iter);
+      simppl_dbus_message_iter_recurse(&iter, &_iter, DBUS_TYPE_ARRAY);
 
       unsigned char* buf; 
       int _len = len;

diff --git a/include/simppl/map.h b/include/simppl/map.h
@@ -38,9 +38,9 @@ struct Codec<std::map<KeyT, ValueT>>
    void decode(DBusMessageIter& iter, std::map<KeyT, ValueT>& m)
    {
       m.clear();
-
+      
       DBusMessageIter _iter;
-      dbus_message_iter_recurse(&iter, &_iter);
+      simppl_dbus_message_iter_recurse(&iter, &_iter, DBUS_TYPE_ARRAY);
 
       while(dbus_message_iter_get_arg_type(&_iter) != 0)
       {

diff --git a/include/simppl/pair.h b/include/simppl/pair.h
@@ -34,7 +34,7 @@ struct Codec<std::pair<KeyT, ValueT>>
    void decode(DBusMessageIter& iter, std::pair<KeyT, ValueT>& p)
    {
       DBusMessageIter item_iterator;
-      dbus_message_iter_recurse(&iter, &item_iterator);
+      simppl_dbus_message_iter_recurse(&iter, &item_iterator, DBUS_TYPE_DICT_ENTRY);
 
       Codec<KeyT>::decode(item_iterator, p.first);
       Codec<ValueT>::decode(item_iterator, p.second);

diff --git a/include/simppl/pod.h b/include/simppl/pod.h
@@ -69,8 +69,7 @@ struct CodecImpl<T, Pod>
    static inline
    void decode(DBusMessageIter& iter, T& t)
    {
-      dbus_message_iter_get_basic(&iter, &t);
-      dbus_message_iter_next(&iter);
+      simppl_dbus_message_iter_get_basic(&iter, &t, dbus_type_code);
    }
 
 

diff --git a/include/simppl/property.h b/include/simppl/property.h
@@ -73,7 +73,7 @@ struct PropertyCodec
    void decode(DBusMessageIter& iter, T& t)
    {
       DBusMessageIter _iter;
-      dbus_message_iter_recurse(&iter, &_iter);
+      simppl_dbus_message_iter_recurse(&iter, &_iter, DBUS_TYPE_VARIANT);
 
       Codec<T>::decode(_iter, t);
 

diff --git a/include/simppl/serialization.h b/include/simppl/serialization.h
@@ -7,9 +7,15 @@
 #include <dbus/dbus.h>
 
 #include "simppl/typelist.h"
-#include "simppl/bool.h"
 
 
+/// throwing exception if expected_type is not met.
+void simppl_dbus_message_iter_recurse(DBusMessageIter* iter, DBusMessageIter* nested, int expected_type);
+
+/// throwing exception if expected_type is not met; advancing iterator
+void simppl_dbus_message_iter_get_basic(DBusMessageIter* iter, void* p, int expected_type);
+
+
 namespace simppl
 {
 
@@ -122,12 +128,17 @@ void decode(DBusMessageIter& iter, T1& t1, T&... t)
 }
 
 
+class DecoderError : public std::exception
+{
+};
+
+
 }   // namespace dbus
 
 }   // namespace simppl
 
 
-
+#include "simppl/bool.h"
 #include "simppl/pod.h"
 
 #endif   // SIMPPL_SERIALIZATION_H
diff --git a/include/simppl/serverside.h b/include/simppl/serverside.h
@@ -42,14 +42,6 @@ struct ServerMethodBase
       eval_(this, msg);
    }
 
-#if SIMPPL_SIGNATURE_CHECK
-   void get_signature(std::ostream& os) const
-   {
-      sig_(os);
-   }
-
-   const char* get_signature() const;
-#endif
 
 #if SIMPPL_HAVE_INTROSPECTION
    virtual void introspect(std::ostream& os) const = 0;
@@ -65,11 +57,6 @@ struct ServerMethodBase
    ~ServerMethodBase();
 
    eval_type eval_;
-
-#if SIMPPL_SIGNATURE_CHECK
-   mutable std::string signature_;
-   sig_type sig_;
-#endif
 };
 
 
@@ -151,18 +138,8 @@ struct ServerMethod : ServerMethodBase
     : ServerMethodBase(name, iface)
     {
         eval_ = __eval;
-#if SIMPPL_SIGNATURE_CHECK
-        sig_ = __sig;
-#endif
     }
 
-#if SIMPPL_SIGNATURE_CHECK
-   static
-   void __sig(std::ostream& os)
-   {
-      ForEach<typename args_type_generator::list_type>::template eval<detail::make_type_signature>(os);
-   }
-#endif
 
    template<typename... T>
    inline

diff --git a/include/simppl/struct.h b/include/simppl/struct.h
@@ -176,7 +176,7 @@ template<typename StructT, typename SelectorT>
 void StructSerializationHelper<StructT, SelectorT>::decode(DBusMessageIter& iter, const StructT& st)
 {
    DBusMessageIter _iter;
-   dbus_message_iter_recurse(&iter, &_iter);
+   simppl_dbus_message_iter_recurse(&iter, &_iter, DBUS_TYPE_STRUCT);
 
    s_type& tuple = *(s_type*)&st;
    tuple.decode(_iter);

diff --git a/include/simppl/tuple.h b/include/simppl/tuple.h
@@ -53,7 +53,7 @@ struct TupleDeserializer // : noncopable
     , flattened_(flattened)
    {
       if (!flattened)
-         dbus_message_iter_recurse(&orig_, &iter_);
+         simppl_dbus_message_iter_recurse(&orig_, &iter_, DBUS_TYPE_STRUCT);
    }
 
    ~TupleDeserializer()

diff --git a/include/simppl/variant.h b/include/simppl/variant.h
@@ -464,7 +464,7 @@ struct Codec<Variant<T...>>
    void decode(DBusMessageIter& orig, Variant<T...>& v)
    {
       DBusMessageIter iter;
-      dbus_message_iter_recurse(&orig, &iter);
+      simppl_dbus_message_iter_recurse(&orig, &iter, DBUS_TYPE_VARIANT);
 
       if (!detail::try_deserialize(iter, v, dbus_message_iter_get_signature(&iter)))
          assert(false);

diff --git a/include/simppl/vector.h b/include/simppl/vector.h
@@ -42,7 +42,7 @@ struct Codec<std::vector<T>>
       v.clear();
 
       DBusMessageIter iter;
-      dbus_message_iter_recurse(&s, &iter);
+      simppl_dbus_message_iter_recurse(&s, &iter, DBUS_TYPE_ARRAY);
 
       while(dbus_message_iter_get_arg_type(&iter) != 0)
       {

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -15,6 +15,7 @@ add_library(simppl SHARED
    objectpath.cpp
    filedescriptor.cpp
    clientside.cpp
+   serialization.cpp
 )
 
 target_link_libraries(simppl dbus-1)

diff --git a/src/filedescriptor.cpp b/src/filedescriptor.cpp
@@ -100,7 +100,7 @@ void FileDescriptorCodec::encode(DBusMessageIter& iter, const FileDescriptor& fd
 void FileDescriptorCodec::decode(DBusMessageIter& iter, FileDescriptor& fd)
 {
    int _fd;
-   dbus_message_iter_get_basic(&iter, &_fd);
+   simppl_dbus_message_iter_get_basic(&iter, &_fd, DBUS_TYPE_UNIX_FD);
    fd = FileDescriptor(_fd);
 }
 

diff --git a/src/objectpath.cpp b/src/objectpath.cpp
@@ -18,12 +18,10 @@ void ObjectPathCodec::encode(DBusMessageIter& iter, const ObjectPath& p)
 
 /*static*/ 
 void ObjectPathCodec::decode(DBusMessageIter& iter, ObjectPath& p)
-{
-   char* c_str = nullptr;
+{   
+   char* c_str = nullptr;  
+   simppl_dbus_message_iter_get_basic(&iter, &c_str, DBUS_TYPE_OBJECT_PATH);
 
-   dbus_message_iter_get_basic(&iter, &c_str);
-   dbus_message_iter_next(&iter);
-
    if (c_str)
    {
       p.path.assign(c_str);

diff --git a/src/serialization.cpp b/src/serialization.cpp
@@ -0,0 +1,20 @@
+#include "simppl/serialization.h"
+
+
+void simppl_dbus_message_iter_recurse(DBusMessageIter* iter, DBusMessageIter* nested, int expected_type)
+{
+   if (dbus_message_iter_get_arg_type(iter) != expected_type)
+      throw simppl::dbus::DecoderError();
+
+   dbus_message_iter_recurse(iter, nested);
+}
+
+
+void simppl_dbus_message_iter_get_basic(DBusMessageIter* iter, void* p, int expected_type)
+{
+   if (dbus_message_iter_get_arg_type(iter) != expected_type)
+      throw simppl::dbus::DecoderError();
+
+   dbus_message_iter_get_basic(iter, p);
+   dbus_message_iter_next(iter);
+}
diff --git a/src/skeletonbase.cpp b/src/skeletonbase.cpp
@@ -262,52 +262,63 @@ DBusHandlerResult SkeletonBase::handle_request(DBusMessage* msg)
           std::string interface;
           std::string attribute;
 
-          DBusMessageIter iter;
-          dbus_message_iter_init(msg, &iter);
-
-          decode(iter, interface, attribute);
-
-          while(p)
+          try
           {
-             if (attribute == p->name_)
-             {
-                if (method[0] == 'G')
-                {
-                   message_ptr_t response = make_message(dbus_message_new_method_return(msg));
-                   p->eval(response.get());
+             DBusMessageIter iter;
+             dbus_message_iter_init(msg, &iter);
+
+             decode(iter, interface, attribute);
 
-                   dbus_connection_send(disp_->conn_, response.get(), nullptr);
-                }
-                else
+             while(p)
+             {
+                if (attribute == p->name_)
                 {
-                   message_ptr_t response = make_message(nullptr);
-
-                   try
+                   if (method[0] == 'G')
                    {
-                      p->evalSet(iter);
-
-                      response = make_message(dbus_message_new_method_return(msg));
-                   }
-                   catch(simppl::dbus::Error& err)
-                   {
-                      response = err.make_reply_for(*msg);
+                      message_ptr_t response = make_message(dbus_message_new_method_return(msg));
+                      p->eval(response.get());
+
+                      dbus_connection_send(disp_->conn_, response.get(), nullptr);
                    }
-                   catch(...)
+                   else
                    {
-                      simppl::dbus::Error e("simppl.dbus.UnhandledException");
-                      response = e.make_reply_for(*msg);
+                      message_ptr_t response = make_message(nullptr);
+
+                      try
+                      {
+                         p->evalSet(iter);
+
+                         response = make_message(dbus_message_new_method_return(msg));
+                      }
+                      catch(simppl::dbus::Error& err)
+                      {
+                         response = err.make_reply_for(*msg);
+                      }
+                      catch(...)
+                      {
+                         simppl::dbus::Error e("simppl.dbus.UnhandledException");
+                         response = e.make_reply_for(*msg);
+                      }
+
+                      dbus_connection_send(disp_->conn_, response.get(), nullptr);
                    }
-                   
-                   dbus_connection_send(disp_->conn_, response.get(), nullptr);
+
+                   return DBUS_HANDLER_RESULT_HANDLED;
                 }
 
-                return DBUS_HANDLER_RESULT_HANDLED;
+                p = p->next_;
              }
 
-             p = p->next_;
+             std::cerr << "attribute '" << attribute << "' unknown" << std::endl;
+          }
+          catch(DecoderError&)
+          {
+             simppl::dbus::Error err(DBUS_ERROR_INVALID_ARGS);
+             auto r = err.make_reply_for(*msg);
+             dbus_connection_send(disp_->conn_, r.get(), nullptr);
+
+             return DBUS_HANDLER_RESULT_HANDLED;
           }
-
-          std::cerr << "attribute '" << attribute << "' unknown" << std::endl;
        }
    }
    else
@@ -317,24 +328,28 @@ DBusHandlerResult SkeletonBase::handle_request(DBusMessage* msg)
       {
          if (!strcmp(method, pm->name_))
          {
-#if SIMPPL_SIGNATURE_CHECK
-            if (strcmp(pm->get_signature(), dbus_message_get_signature(msg)))
+            current_request_.set(pm, msg);
+
+            try
             {
-               std::cerr << "Shit, wrong arguments" << std::endl;
-               return DBUS_HANDLER_RESULT_HANDLED;
+               pm->eval(msg);
+            }
+            catch(DecoderError&)
+            {
+               simppl::dbus::Error err(DBUS_ERROR_INVALID_ARGS);
+               auto r = err.make_reply_for(*msg);
+               dbus_connection_send(disp_->conn_, r.get(), nullptr);
+            }
+            catch(...)
+            {
+               simppl::dbus::Error e("simppl.dbus.UnhandledException");
+               auto r = e.make_reply_for(*msg);
+               dbus_connection_send(disp_->conn_, r.get(), nullptr);
             }
-#endif
-
-            current_request_.set(pm, msg);
-            pm->eval(msg);
 
             // current_request_ is only valid if no response handler was called
             if (current_request_)
-            {
-               // in that case the request must not have a reponse
-               //assert(!current_request_.requestor_->hasResponse());
                current_request_.clear();
-            }
 
             return DBUS_HANDLER_RESULT_HANDLED;
          }