Skip to content

Commit

Permalink
Add willCheckConsistency option to enable/disable message consistency…
Browse files Browse the repository at this point in the history
… checking (#947)

Currently, when a publisher/client sends a topic/request, it will check the consistency of the message to be sent. For example, the following code will lead to an exception:

```js
  const pub_ = this.node.createPublisher('sensor_msgs/msg/JointState', 'topic_js');
  const stringMsgObject = rclnodejs.createMessageObject('sensor_msgs/msg/JointState');
  stringMsgObject.header.frame_id = '0';
```
Because the `stamp` in `header` is not initialized.

This patch adds an additional option for node when initializing, which is `willCheckConsistency` (default is `false`), so the code above can run as expected with the  `stamp` initialized with default value. Meanwhile, user can set it to `true` to check the consistency intentionally.

To achieve, this patch implements:

1.  Add `willCheckConsistency` to the node option.
2. Update `message.dot` accordingly to check the consistency based on the node option above.
3. Update the unit tests:
   - test/test-security-related.js

Fix: #937
  • Loading branch information
minggangw committed Feb 4, 2024
1 parent 90ede83 commit c4c590a
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 13 deletions.
8 changes: 5 additions & 3 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,15 @@ class Client extends Entity {
* @see {@link ResponseCallback}
*/
sendRequest(request, callback) {
if (typeof callback !== 'function') {
throw new TypeError('Invalid argument');
}

let requestToSend =
request instanceof this._typeClass.Request
? request
: new this._typeClass.Request(request);
if (typeof callback !== 'function') {
throw new TypeError('Invalid argument');
}
requestToSend._willCheckConsistency = this._options.willCheckConsistency;

let rawRequest = requestToSend.serialize();
let sequenceNumber = rclnodejs.sendRequest(this._handle, rawRequest);
Expand Down
2 changes: 1 addition & 1 deletion lib/lifecycle_publisher.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const Publisher = require('./publisher.js');
*/
class LifecyclePublisher extends Publisher {
constructor(handle, typeClass, topic, options) {
super(handle, typeClass, options);
super(handle, typeClass, /*topic=*/'', options);

this._enabled = false;
this._loggger = Logging.getLogger('LifecyclePublisher');
Expand Down
7 changes: 7 additions & 0 deletions lib/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,10 @@ class Node extends rclnodejs.ShadowNode {
options = Object.assign(options, { isRaw: false });
}

if (options.willCheckConsistency === undefined) {
options = Object.assign(options, { willCheckConsistency: false });
}

return options;
}

Expand Down Expand Up @@ -588,6 +592,7 @@ class Node extends rclnodejs.ShadowNode {
* @param {object} options - The options argument used to parameterize the publisher.
* @param {boolean} options.enableTypedArray - The topic will use TypedArray if necessary, default: true.
* @param {QoS} options.qos - ROS Middleware "quality of service" settings for the publisher, default: QoS.profileDefault.
* @param {boolean} options.willCheckConsistency - Pulisher will check the consistancy of the message to be sent, default: false.
* @return {Publisher} - An instance of Publisher.
*/
createPublisher(typeClass, topic, options) {
Expand Down Expand Up @@ -690,6 +695,7 @@ class Node extends rclnodejs.ShadowNode {
* @param {object} options - The options argument used to parameterize the client.
* @param {boolean} options.enableTypedArray - The response will use TypedArray if necessary, default: true.
* @param {QoS} options.qos - ROS Middleware "quality of service" settings for the client, default: QoS.profileDefault.
* @param {boolean} options.willCheckConsistency - Client will check the consistancy of the message to be sent, default: false.
* @return {Client} - An instance of Client.
*/
createClient(typeClass, serviceName, options) {
Expand Down Expand Up @@ -1690,6 +1696,7 @@ Node.getDefaultOptions = function () {
isRaw: false,
qos: QoS.profileDefault,
contentFilter: undefined,
willCheckConsistency: false
};
};

Expand Down
2 changes: 1 addition & 1 deletion lib/publisher.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class Publisher extends Entity {
message instanceof this._typeClass
? message
: new this._typeClass(message);

messageToSend._willCheckConsistency = this._options.willCheckConsistency;
let rawMessage = messageToSend.serialize();
rclnodejs.publish(this._handle, rawMessage);
}
Expand Down
2 changes: 1 addition & 1 deletion rosidl_gen/generator.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "rosidl-generator",
"version": "0.3.5",
"version": "0.3.6",
"description": "Generate JavaScript object from ROS IDL(.msg) files",
"main": "index.js",
"authors": [
Expand Down
9 changes: 4 additions & 5 deletions rosidl_gen/templates/message.dot
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,9 @@ const {{=refObjectArrayType}} = StructType({

// Define the wrapper class.
class {{=objectWrapper}} {
constructor(other) {
constructor(other, willCheckConsistency = false) {
this._wrapperFields = {};
this._willCheckConsistency = willCheckConsistency;
{{~ it.spec.fields :field}}
{{? field.type.isArray && field.type.isPrimitiveType && !isTypedArrayType(field.type)}}
this._{{=field.name}}Array = [];
Expand Down Expand Up @@ -366,15 +367,13 @@ class {{=objectWrapper}} {
}

freeze(own = false, checkConsistency = false) {
if (checkConsistency) {
{{~ it.spec.fields :field}}
{{? field.type.isPrimitiveType && !field.type.isArray}}
if (!this._{{=field.name}}Intialized) {
if (checkConsistency && !this._{{=field.name}}Intialized) {
throw new TypeError('Invalid argument: {{=field.name}} in {{=it.spec.msgName}}');
}
{{?}}
{{~}}
}

{{~ it.spec.fields :field}}
{{? field.type.isArray && field.type.isPrimitiveType && field.type.isFixedSizeArray}}
Expand Down Expand Up @@ -420,7 +419,7 @@ class {{=objectWrapper}} {
}

serialize() {
this.freeze(false, true);
this.freeze(/*own=*/false, this._willCheckConsistency);
return this._refObject.ref();
}

Expand Down
29 changes: 27 additions & 2 deletions test/test-security-related.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ describe('Fuzzing API calls testing', function () {
var node = rclnodejs.createNode('node1', '/inconsistent');
const RclString = 'std_msgs/msg/String';

var publisher = node.createPublisher(RclString, 'chatter7');
var publisher = node.createPublisher(RclString, 'chatter7', {willCheckConsistency: true});
assertThrowsError(
() => {
publisher.publish({ a: 1 });
Expand All @@ -227,6 +227,18 @@ describe('Fuzzing API calls testing', function () {
`Type should be ${RclString}`
);

const String = rclnodejs.require(RclString);
const str = new String();
str.a = 1;
assertThrowsError(
() => {
publisher.publish(str);
},
TypeError,
'Invalid argument',
`Type should be ${RclString}`
);

rclnodejs.spin(node);
node.destroy();
});
Expand All @@ -235,7 +247,7 @@ describe('Fuzzing API calls testing', function () {
var node = rclnodejs.createNode('node2', '/inconsistent');
const AddTwoInts = 'example_interfaces/srv/AddTwoInts';

var client = node.createClient(AddTwoInts, 'add_two_ints');
var client = node.createClient(AddTwoInts, 'add_two_ints', {willCheckConsistency: true});
var service = node.createService(
AddTwoInts,
'add_two_ints',
Expand All @@ -258,6 +270,19 @@ describe('Fuzzing API calls testing', function () {
'Invalid argument',
'request.b does not exist'
);

const Request = rclnodejs.require(AddTwoInts).Request;
const req = new Request();
req.a = 1;
assertThrowsError(
() => {
client.sendRequest(req, (response) => {});
},
TypeError,
'Invalid argument',
'request.b does not exist'
);

rclnodejs.spin(node);
node.destroy();
});
Expand Down

0 comments on commit c4c590a

Please sign in to comment.