Merge pull request #175 from LGSInnovations/setProperty-strict-encoding

Shouqun · web-flow · commit 341d4770c358 · 2017-09-13T07:10:39.000-05:00
Set property strict encoding
diff --git a/lib/interface.js b/lib/interface.js
@@ -103,12 +103,14 @@ Interface.prototype.setProperty = function(propertyName, value, callback) {
 		return;
 	}
 
+	var propSig = self.object['property'][propertyName].type;
+
 	self.bus.callMethod(self.bus.connection,
 		self.serviceName,
 		self.objectPath,
 		'org.freedesktop.DBus.Properties',
 		'Set',
-		'ssv',
+		{ type: 'ssv', concrete_type: 'ss' + propSig },
 		-1,
 		[ self.interfaceName, propertyName, value ],
 		function(err) {
diff --git a/src/dbus.cc b/src/dbus.cc
@@ -196,7 +196,7 @@ namespace NodeDBus {
 		// Preparing method arguments
 		if (info[7]->IsObject()) {
 			DBusMessageIter iter;
-			DBusSignatureIter siter;
+			DBusSignatureIter siter, concrete_siter;
 
 			Local<Array> argument_arr = Local<Array>::Cast(info[7]);
 			if (argument_arr->Length() > 0) {
@@ -205,29 +205,71 @@ namespace NodeDBus {
 				dbus_message_iter_init_append(message, &iter); 
 
 				// Initializing signature
-				char *sig = strdup(*String::Utf8Value(info[5]->ToString()));
+				char *sig = NULL, *concrete_sig = NULL;
+				if (info[5]->IsObject())
+				{
+					Local<Object> obj(info[5]->ToObject());
+					
+					Local<Value> typeKey(Nan::New("type").ToLocalChecked());
+					Nan::MaybeLocal<Value> mb_sig(Nan::Get(obj, typeKey));
+					sig = strdup(*String::Utf8Value(mb_sig.ToLocalChecked()->ToString()));
+					
+					Local<Value> concreteTypeKey(Nan::New("concrete_type").ToLocalChecked());
+					Nan::MaybeLocal<Value> mb_concrete_sig(Nan::Get(obj, concreteTypeKey));
+					Local<Value> concrete_sig_value;
+					if (mb_concrete_sig.ToLocal(&concrete_sig_value))
+					{
+						concrete_sig = strdup(*String::Utf8Value(concrete_sig_value->ToString()));
+					}
+				}
+				else
+				{
+					sig = strdup(*String::Utf8Value(info[5]->ToString()));
+				}
+				
+				if (concrete_sig == NULL) {
+					concrete_sig = strdup(sig);
+				}
+
 				if (!dbus_signature_validate(sig, &error)) {
 					return Nan::ThrowError(error.message);
 				}
 
+				if (!dbus_signature_validate(concrete_sig, &error)) {
+					return Nan::ThrowError(error.message);
+				}
+
 				// Getting all signatures
 				dbus_signature_iter_init(&siter, sig);
+				dbus_signature_iter_init(&concrete_siter, concrete_sig);
 				for (unsigned int i = 0; i < argument_arr->Length(); ++i) {
 					char *arg_sig = dbus_signature_iter_get_signature(&siter);
+					char *arg_concrete_sig = dbus_signature_iter_get_signature(&concrete_siter);
+					
+					DBusSignatureIter item_siter, item_concrete_siter;
 					Local<Value> arg = argument_arr->Get(i);
 
-					if (!Encoder::EncodeObject(arg, &iter, arg_sig)) {
+					dbus_signature_iter_init(&item_siter, arg_sig);
+					dbus_signature_iter_init(&item_concrete_siter, arg_concrete_sig);
+					
+					if (!Encoder::EncodeObject(arg, &iter, &item_siter, &item_concrete_siter)) {
 						dbus_free(arg_sig);
+						dbus_free(arg_concrete_sig);
 						break;
 					}
 
 					dbus_free(arg_sig);
+					dbus_free(arg_concrete_sig);
 
 					if (!dbus_signature_iter_next(&siter))
 						break;
+					
+					if (!dbus_signature_iter_next(&concrete_siter))
+						break;
 				}
 
 				dbus_free(sig);
+				dbus_free(concrete_sig);
 			}
 		}
 
diff --git a/src/encoder.cc b/src/encoder.cc
@@ -151,17 +151,23 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
 		return "";
 	}
 
-	bool EncodeObject(Local<Value> value, DBusMessageIter *iter, const char *signature)
+	bool EncodeObject(Local<Value> value, DBusMessageIter *iter,
+		const DBusSignatureIter *siter,
+		const DBusSignatureIter *concreteSiter)
 	{
 		// printf("EncodeObject %s\n",signature);
 		// printf("%p", value);
 		Nan::HandleScope scope;
-		DBusSignatureIter siter;
 		int type;
+		
+		if (concreteSiter == NULL)
+		{
+			// this is safe because we never modify siter
+			concreteSiter = siter;
+		}
 
 		// Get type of current value
-		dbus_signature_iter_init(&siter, signature);
-		type = dbus_signature_iter_get_current_type(&siter);
+		type = dbus_signature_iter_get_current_type(siter);
 
 		switch(type) {
 		case DBUS_TYPE_INVALID:
@@ -315,11 +321,12 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
 			}
 
 			DBusMessageIter subIter;
-			DBusSignatureIter arraySiter;
+			DBusSignatureIter arraySiter, arrayConcreteSiter;
 			char *array_sig = NULL;
 
 			// Getting signature of array object
-			dbus_signature_iter_recurse(&siter, &arraySiter);
+			dbus_signature_iter_recurse(siter, &arraySiter);
+			dbus_signature_iter_recurse(concreteSiter, &arrayConcreteSiter);
 			array_sig = dbus_signature_iter_get_signature(&arraySiter);
 
 			// Open array container to process elements in there
@@ -328,27 +335,24 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
 				printf("Can't open container for Array type\n");
 				return false; 
 			}
+			dbus_free(array_sig);
 
 			// It's a dictionary
-			if (dbus_signature_iter_get_element_type(&siter) == DBUS_TYPE_DICT_ENTRY) {
-
-				dbus_free(array_sig);
-
+			if (dbus_signature_iter_get_element_type(siter) == DBUS_TYPE_DICT_ENTRY) {
 				Local<Object> value_object = value->ToObject();
-				DBusSignatureIter dictSubSiter;
-
-				// Getting sub-signature object
-				dbus_signature_iter_recurse(&arraySiter, &dictSubSiter);
-				dbus_signature_iter_next(&dictSubSiter);
-				char *sig = dbus_signature_iter_get_signature(&dictSubSiter);
 
 				// process each elements
 				Local<Array> prop_names = value_object->GetPropertyNames();
 				unsigned int len = prop_names->Length();
 
 				bool failed = false;
 				for (unsigned int i = 0; i < len; ++i) {
+					DBusSignatureIter dictSubSiter, dictSubConcreteSiter;
 					DBusMessageIter dict_iter;
+					
+					// Getting sub-signature object
+					dbus_signature_iter_recurse(&arraySiter, &dictSubSiter);
+					dbus_signature_iter_recurse(&arrayConcreteSiter, &dictSubConcreteSiter);
 
 					// Open dict entry container
 					if (!dbus_message_iter_open_container(&subIter, DBUS_TYPE_DICT_ENTRY, NULL, &dict_iter)) {
@@ -362,13 +366,17 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
 					Local<Value> prop_value = value_object->Get(prop_key);
 
 					// Append the key
-					char *prop_key_str = strdup(*String::Utf8Value(prop_key->ToString()));
-					// printf("key: %s\n", prop_key_str);
-					dbus_message_iter_append_basic(&dict_iter, DBUS_TYPE_STRING, &prop_key_str);
-					dbus_free(prop_key_str);
+					if (!EncodeObject(prop_key, &dict_iter, &dictSubSiter, &dictSubConcreteSiter)) {
+						dbus_message_iter_close_container(&subIter, &dict_iter); 
+						printf("Failed to encode element of dictionary\n");
+						failed = true;
+						break;
+					}
 
 					// Append the value
-					if (!EncodeObject(prop_value, &dict_iter, sig)) {
+					dbus_signature_iter_next(&dictSubSiter);
+					dbus_signature_iter_next(&dictSubConcreteSiter);
+					if (!EncodeObject(prop_value, &dict_iter, &dictSubSiter, &dictSubConcreteSiter)) {
 						dbus_message_iter_close_container(&subIter, &dict_iter); 
 						printf("Failed to encode element of dictionary\n");
 						failed = true;
@@ -378,7 +386,6 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
 					dbus_message_iter_close_container(&subIter, &dict_iter); 
 				}
 
-				dbus_free(sig);
 				dbus_message_iter_close_container(iter, &subIter);
 
 				if (failed) 
@@ -396,42 +403,54 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
 			Local<Array> arrayData = Local<Array>::Cast(value);
 			for (unsigned int i = 0; i < arrayData->Length(); ++i) {
 				Local<Value> arrayItem = arrayData->Get(i);
-				if (!EncodeObject(arrayItem, &subIter, array_sig))
+				if (!EncodeObject(arrayItem, &subIter, &arraySiter, &arrayConcreteSiter))
 					break;
 			}
 
 			dbus_message_iter_close_container(iter, &subIter);
-			dbus_free(array_sig);
 
 			break;
 		}
 
 		case DBUS_TYPE_VARIANT:
 		{
 			DBusMessageIter subIter;
-
-			string str_sig = GetSignatureFromV8Type(value);
-			const char *var_sig = str_sig.c_str();
+			DBusSignatureIter subSiter;
+			
+			char *var_sig;
+			if (dbus_signature_iter_get_current_type(concreteSiter) == DBUS_TYPE_VARIANT)
+			{
+				string str_sig = GetSignatureFromV8Type(value);
+				var_sig = strdup(str_sig.c_str());
+			}
+			else
+			{
+				var_sig = dbus_signature_iter_get_signature(concreteSiter);
+			}
 
 			if (!dbus_message_iter_open_container(iter, DBUS_TYPE_VARIANT, var_sig, &subIter)) {
+				dbus_free(var_sig);
 				printf("Can't open container for VARIANT type\n");
 				return false;
 			}
 
-			if (!EncodeObject(value, &subIter, var_sig)) { 
+			dbus_signature_iter_init(&subSiter, var_sig);
+			if (!EncodeObject(value, &subIter, &subSiter, &subSiter)) { 
 				dbus_message_iter_close_container(iter, &subIter);
+				dbus_free(var_sig);
 				return false;
 			}
 
 			dbus_message_iter_close_container(iter, &subIter);
+			dbus_free(var_sig);
 
 			break;
 		}
 		case DBUS_TYPE_STRUCT:
 		{
 			// printf("struct\n");
 			DBusMessageIter subIter;
-			DBusSignatureIter structSiter;
+			DBusSignatureIter structSiter, structConcreteSiter;
 			
 			// Open array container to process elements in there
 			if (!dbus_message_iter_open_container(iter, DBUS_TYPE_STRUCT, NULL, &subIter)) {
@@ -442,28 +461,27 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
 			Local<Object> value_object = value->ToObject();
 
 			// Getting sub-signature object
-			dbus_signature_iter_recurse(&siter, &structSiter);
+			dbus_signature_iter_recurse(siter, &structSiter);
+			dbus_signature_iter_recurse(concreteSiter, &structConcreteSiter);
 
 			// process each elements
 			Local<Array> prop_names = value_object->GetPropertyNames();
 			unsigned int len = prop_names->Length();
 
 			for (unsigned int i = 0; i < len; ++i) {
 
-				char *sig = dbus_signature_iter_get_signature(&structSiter);
-
 				Local<Value> prop_key = prop_names->Get(i);
 
-				if (!EncodeObject(value_object->Get(prop_key), &subIter, sig)) {
-					dbus_free(sig);
+				if (!EncodeObject(value_object->Get(prop_key), &subIter, &structSiter, &structConcreteSiter)) {
 					printf("Failed to encode element of dictionary\n");
 					return false;
 				}
 
-				dbus_free(sig);
-
 				if (!dbus_signature_iter_next(&structSiter))
 					break;
+				
+				if (!dbus_signature_iter_next(&structConcreteSiter))
+					break;
 			}
 
 			dbus_message_iter_close_container(iter, &subIter); 
diff --git a/src/encoder.h b/src/encoder.h
@@ -8,7 +8,20 @@ namespace Encoder {
 	using namespace std;
 
 	string GetSignatureFromV8Type(Local<Value>& value);
-	bool EncodeObject(Local<Value> value, DBusMessageIter *iter, const char *signature);
+	
+	// expects two signatures - one for the type signature as DBus will see it,
+	// and a second that is the same as the first, except that variants requiring
+	// a particular concrete type inside should be replaced by their required concrete type.
+	// 
+	// for example, when writing a property, the property value in the Get method is
+	// a variant at the DBus interface level, but should only actually be encoded with
+	// the actual concrete type declared for the property.
+	//
+	// If the concrete type is NULL or the same as the main one, types of variant
+	// elements will be inferred based on the V8 type (and, in the case of numbers,
+	// their values)
+	bool EncodeObject(Local<Value> value, DBusMessageIter *iter,
+		const DBusSignatureIter *siter, const DBusSignatureIter *concreteSiter = NULL);
 }
 
 #endif
diff --git a/src/object_handler.cc b/src/object_handler.cc
@@ -77,13 +77,15 @@ namespace ObjectHandler {
 	{
 		Nan::HandleScope scope;
 		DBusMessageIter iter;
+		DBusSignatureIter siter;
 		DBusMessage *reply;
 		dbus_uint32_t serial = 0;
 
 		reply = dbus_message_new_method_return(message);
 
 		dbus_message_iter_init_append(reply, &iter);
-		if (!Encoder::EncodeObject(reply_value, &iter, signature)) {
+		dbus_signature_iter_init(&siter, signature);
+		if (!Encoder::EncodeObject(reply_value, &iter, &siter)) {
 			printf("Failed to encode reply value\n");
 		}
 
diff --git a/src/signal.cc b/src/signal.cc
@@ -41,6 +41,8 @@ namespace Signal {
 	}
 
 	NAN_METHOD(EmitSignal) {
+		DBusError error;
+
 		if (!info[0]->IsObject()) {
 			return Nan::ThrowTypeError("First parameter must be an object");
 		}
@@ -80,6 +82,9 @@ namespace Signal {
 		char *signal = strdup(*String::Utf8Value(info[3]->ToString()));
 		message = dbus_message_new_signal(path, interface, signal);
 
+		// Initializing error handler
+		dbus_error_init(&error);
+
 		// Preparing message
 		dbus_message_iter_init_append(message, &iter);
 
@@ -88,10 +93,15 @@ namespace Signal {
 		Local<Array> signatures = Local<Array>::Cast(info[5]);
 		for (unsigned int i = 0; i < arguments->Length(); ++i) {
 			Local<Value> arg = arguments->Get(i);
-
+			DBusSignatureIter siter;
+			
 			char *sig = strdup(*String::Utf8Value(signatures->Get(i)->ToString()));
-
-			if (!Encoder::EncodeObject(arg, &iter, sig)) {
+			if (!dbus_signature_validate(sig, &error)) {
+				return Nan::ThrowError(error.message);
+			}
+			
+			dbus_signature_iter_init(&siter, sig);
+			if (!Encoder::EncodeObject(arg, &iter, &siter)) {
 				dbus_free(sig);
 				printf("Failed to encode arguments of signal\n");
 				break;
diff --git a/test/client-call-intdict.test.js b/test/client-call-intdict.test.js
diff --git a/test/service.js b/test/service.js

Original file line number	Diff line number	Diff line change
`@@ -77,13 +77,15 @@ namespace ObjectHandler {`
`77`	`77`	`{`
`78`	`78`	`Nan::HandleScope scope;`
`79`	`79`	`DBusMessageIter iter;`
	`80`	`+ DBusSignatureIter siter;`
`80`	`81`	`DBusMessage *reply;`
`81`	`82`	`dbus_uint32_t serial = 0;`
`82`	`83`
`83`	`84`	`reply = dbus_message_new_method_return(message);`
`84`	`85`
`85`	`86`	`dbus_message_iter_init_append(reply, &iter);`
`86`		`- if (!Encoder::EncodeObject(reply_value, &iter, signature)) {`
	`87`	`+ dbus_signature_iter_init(&siter, signature);`
	`88`	`+ if (!Encoder::EncodeObject(reply_value, &iter, &siter)) {`
`87`	`89`	`printf("Failed to encode reply value\n");`
`88`	`90`	`}`
`89`	`91`