Skip to content

Commit ccae62e

Browse files
mokus0mokus0
committed
Encode values correctly when setting properties.
This was a problem, for example, when writing a NetworkManager interface. When trying to set a device statistics interface's "RefreshRateMs" property, if the value was < 255, node-dbus would send the property as a byte, which the interface rejected because the property is declared with signature "u".
1 parent c607e48 commit ccae62e

File tree

4 files changed

+100
-19
lines changed

4 files changed

+100
-19
lines changed

‎lib/interface.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,14 @@ Interface.prototype.setProperty = function(propertyName, value, callback) {
103103
return;
104104
}
105105

106+
var propSig = self.object['property'][propertyName].type;
107+
106108
self.bus.callMethod(self.bus.connection,
107109
self.serviceName,
108110
self.objectPath,
109111
'org.freedesktop.DBus.Properties',
110112
'Set',
111-
'ssv',
113+
{ type: 'ssv', concrete_type: 'ss' + propSig },
112114
-1,
113115
[ self.interfaceName, propertyName, value ],
114116
function(err) {

‎src/dbus.cc‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ namespace NodeDBus {
196196
// Preparing method arguments
197197
if (info[7]->IsObject()) {
198198
DBusMessageIter iter;
199-
DBusSignatureIter siter;
199+
DBusSignatureIter siter, concrete_siter;
200200

201201
Local<Array> argument_arr = Local<Array>::Cast(info[7]);
202202
if (argument_arr->Length() > 0) {
@@ -205,31 +205,71 @@ namespace NodeDBus {
205205
dbus_message_iter_init_append(message, &iter);
206206

207207
// Initializing signature
208-
char *sig = strdup(*String::Utf8Value(info[5]->ToString()));
208+
char *sig = NULL, *concrete_sig = NULL;
209+
if (info[5]->IsObject())
210+
{
211+
Local<Object> obj(info[5]->ToObject());
212+
213+
Local<Value> typeKey(Nan::New("type").ToLocalChecked());
214+
Nan::MaybeLocal<Value> mb_sig(Nan::Get(obj, typeKey));
215+
sig = strdup(*String::Utf8Value(mb_sig.ToLocalChecked()->ToString()));
216+
217+
Local<Value> concreteTypeKey(Nan::New("concrete_type").ToLocalChecked());
218+
Nan::MaybeLocal<Value> mb_concrete_sig(Nan::Get(obj, concreteTypeKey));
219+
Local<Value> concrete_sig_value;
220+
if (mb_concrete_sig.ToLocal(&concrete_sig_value))
221+
{
222+
concrete_sig = strdup(*String::Utf8Value(concrete_sig_value->ToString()));
223+
}
224+
}
225+
else
226+
{
227+
sig = strdup(*String::Utf8Value(info[5]->ToString()));
228+
}
229+
230+
if (concrete_sig == NULL) {
231+
concrete_sig = strdup(sig);
232+
}
233+
209234
if (!dbus_signature_validate(sig, &error)) {
210235
return Nan::ThrowError(error.message);
211236
}
212237

238+
if (!dbus_signature_validate(concrete_sig, &error)) {
239+
return Nan::ThrowError(error.message);
240+
}
241+
213242
// Getting all signatures
214243
dbus_signature_iter_init(&siter, sig);
244+
dbus_signature_iter_init(&concrete_siter, concrete_sig);
215245
for (unsigned int i = 0; i < argument_arr->Length(); ++i) {
216246
char *arg_sig = dbus_signature_iter_get_signature(&siter);
217-
DBusSignatureIter subSiter;
247+
char *arg_concrete_sig = dbus_signature_iter_get_signature(&concrete_siter);
248+
249+
DBusSignatureIter item_siter, item_concrete_siter;
218250
Local<Value> arg = argument_arr->Get(i);
219251

220-
dbus_signature_iter_init(&subSiter, arg_sig);
221-
if (!Encoder::EncodeObject(arg, &iter, &subSiter)) {
252+
dbus_signature_iter_init(&item_siter, arg_sig);
253+
dbus_signature_iter_init(&item_concrete_siter, arg_concrete_sig);
254+
255+
if (!Encoder::EncodeObject(arg, &iter, &item_siter, &item_concrete_siter)) {
222256
dbus_free(arg_sig);
257+
dbus_free(arg_concrete_sig);
223258
break;
224259
}
225260

226261
dbus_free(arg_sig);
262+
dbus_free(arg_concrete_sig);
227263

228264
if (!dbus_signature_iter_next(&siter))
229265
break;
266+
267+
if (!dbus_signature_iter_next(&concrete_siter))
268+
break;
230269
}
231270

232271
dbus_free(sig);
272+
dbus_free(concrete_sig);
233273
}
234274
}
235275

‎src/encoder.cc‎

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -151,12 +151,20 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
151151
return "";
152152
}
153153

154-
bool EncodeObject(Local<Value> value, DBusMessageIter *iter, DBusSignatureIter *siter)
154+
bool EncodeObject(Local<Value> value, DBusMessageIter *iter,
155+
const DBusSignatureIter *siter,
156+
const DBusSignatureIter *concreteSiter)
155157
{
156158
// printf("EncodeObject %s\n",signature);
157159
// printf("%p", value);
158160
Nan::HandleScope scope;
159161
int type;
162+
163+
if (concreteSiter == NULL)
164+
{
165+
// this is safe because we never modify siter
166+
concreteSiter = siter;
167+
}
160168

161169
// Get type of current value
162170
type = dbus_signature_iter_get_current_type(siter);
@@ -313,11 +321,12 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
313321
}
314322

315323
DBusMessageIter subIter;
316-
DBusSignatureIter arraySiter;
324+
DBusSignatureIter arraySiter, arrayConcreteSiter;
317325
char *array_sig = NULL;
318326

319327
// Getting signature of array object
320328
dbus_signature_iter_recurse(siter, &arraySiter);
329+
dbus_signature_iter_recurse(concreteSiter, &arrayConcreteSiter);
321330
array_sig = dbus_signature_iter_get_signature(&arraySiter);
322331

323332
// Open array container to process elements in there
@@ -338,11 +347,12 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
338347

339348
bool failed = false;
340349
for (unsigned int i = 0; i < len; ++i) {
341-
DBusSignatureIter dictSubSiter;
350+
DBusSignatureIter dictSubSiter, dictSubConcreteSiter;
342351
DBusMessageIter dict_iter;
343352

344353
// Getting sub-signature object
345354
dbus_signature_iter_recurse(&arraySiter, &dictSubSiter);
355+
dbus_signature_iter_recurse(&arrayConcreteSiter, &dictSubConcreteSiter);
346356

347357
// Open dict entry container
348358
if (!dbus_message_iter_open_container(&subIter, DBUS_TYPE_DICT_ENTRY, NULL, &dict_iter)) {
@@ -356,7 +366,7 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
356366
Local<Value> prop_value = value_object->Get(prop_key);
357367

358368
// Append the key
359-
if (!EncodeObject(prop_key, &dict_iter, &dictSubSiter)) {
369+
if (!EncodeObject(prop_key, &dict_iter, &dictSubSiter, &dictSubConcreteSiter)) {
360370
dbus_message_iter_close_container(&subIter, &dict_iter);
361371
printf("Failed to encode element of dictionary\n");
362372
failed = true;
@@ -365,7 +375,8 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
365375

366376
// Append the value
367377
dbus_signature_iter_next(&dictSubSiter);
368-
if (!EncodeObject(prop_value, &dict_iter, &dictSubSiter)) {
378+
dbus_signature_iter_next(&dictSubConcreteSiter);
379+
if (!EncodeObject(prop_value, &dict_iter, &dictSubSiter, &dictSubConcreteSiter)) {
369380
dbus_message_iter_close_container(&subIter, &dict_iter);
370381
printf("Failed to encode element of dictionary\n");
371382
failed = true;
@@ -392,7 +403,7 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
392403
Local<Array> arrayData = Local<Array>::Cast(value);
393404
for (unsigned int i = 0; i < arrayData->Length(); ++i) {
394405
Local<Value> arrayItem = arrayData->Get(i);
395-
if (!EncodeObject(arrayItem, &subIter, &arraySiter))
406+
if (!EncodeObject(arrayItem, &subIter, &arraySiter, &arrayConcreteSiter))
396407
break;
397408
}
398409

@@ -405,30 +416,41 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
405416
{
406417
DBusMessageIter subIter;
407418
DBusSignatureIter subSiter;
408-
409-
string str_sig = GetSignatureFromV8Type(value);
410-
const char *var_sig = str_sig.c_str();
419+
420+
char *var_sig;
421+
if (dbus_signature_iter_get_current_type(concreteSiter) == DBUS_TYPE_VARIANT)
422+
{
423+
string str_sig = GetSignatureFromV8Type(value);
424+
var_sig = strdup(str_sig.c_str());
425+
}
426+
else
427+
{
428+
var_sig = dbus_signature_iter_get_signature(concreteSiter);
429+
}
411430

412431
if (!dbus_message_iter_open_container(iter, DBUS_TYPE_VARIANT, var_sig, &subIter)) {
432+
dbus_free(var_sig);
413433
printf("Can't open container for VARIANT type\n");
414434
return false;
415435
}
416436

417437
dbus_signature_iter_init(&subSiter, var_sig);
418-
if (!EncodeObject(value, &subIter, &subSiter)) {
438+
if (!EncodeObject(value, &subIter, &subSiter, &subSiter)) {
419439
dbus_message_iter_close_container(iter, &subIter);
440+
dbus_free(var_sig);
420441
return false;
421442
}
422443

423444
dbus_message_iter_close_container(iter, &subIter);
445+
dbus_free(var_sig);
424446

425447
break;
426448
}
427449
case DBUS_TYPE_STRUCT:
428450
{
429451
// printf("struct\n");
430452
DBusMessageIter subIter;
431-
DBusSignatureIter structSiter;
453+
DBusSignatureIter structSiter, structConcreteSiter;
432454

433455
// Open array container to process elements in there
434456
if (!dbus_message_iter_open_container(iter, DBUS_TYPE_STRUCT, NULL, &subIter)) {
@@ -440,6 +462,7 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
440462

441463
// Getting sub-signature object
442464
dbus_signature_iter_recurse(siter, &structSiter);
465+
dbus_signature_iter_recurse(concreteSiter, &structConcreteSiter);
443466

444467
// process each elements
445468
Local<Array> prop_names = value_object->GetPropertyNames();
@@ -449,13 +472,16 @@ typedef bool (*CheckTypeCallback) (Local<Value>& value, const char* sig);
449472

450473
Local<Value> prop_key = prop_names->Get(i);
451474

452-
if (!EncodeObject(value_object->Get(prop_key), &subIter, &structSiter)) {
475+
if (!EncodeObject(value_object->Get(prop_key), &subIter, &structSiter, &structConcreteSiter)) {
453476
printf("Failed to encode element of dictionary\n");
454477
return false;
455478
}
456479

457480
if (!dbus_signature_iter_next(&structSiter))
458481
break;
482+
483+
if (!dbus_signature_iter_next(&structConcreteSiter))
484+
break;
459485
}
460486

461487
dbus_message_iter_close_container(iter, &subIter);

‎src/encoder.h‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,20 @@ namespace Encoder {
88
using namespace std;
99

1010
string GetSignatureFromV8Type(Local<Value>& value);
11-
bool EncodeObject(Local<Value> value, DBusMessageIter *iter, DBusSignatureIter *siter);
11+
12+
// expects two signatures - one for the type signature as DBus will see it,
13+
// and a second that is the same as the first, except that variants requiring
14+
// a particular concrete type inside should be replaced by their required concrete type.
15+
//
16+
// for example, when writing a property, the property value in the Get method is
17+
// a variant at the DBus interface level, but should only actually be encoded with
18+
// the actual concrete type declared for the property.
19+
//
20+
// If the concrete type is NULL or the same as the main one, types of variant
21+
// elements will be inferred based on the V8 type (and, in the case of numbers,
22+
// their values)
23+
bool EncodeObject(Local<Value> value, DBusMessageIter *iter,
24+
const DBusSignatureIter *siter, const DBusSignatureIter *concreteSiter = NULL);
1225
}
1326

1427
#endif

0 commit comments

Comments
 (0)