niklas/cyclonedds
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Properly handle "incompatible if unrecognized" in parameter lists (#79)

Browse source 
Previously it would fall through and assert in a debug build or return an error in a release build. The behaviour in a release build was almost correct, as the flag means the entity should be completely ignored if the parameter is not understood by the implementation, but I don't believe it should result in a warning — certainly not that claims the parameter list is invalid. A specific return code is now used to indicate a parameter list that was rejected because of this flag, and that suppresses the warning.

Signed-off-by: Erik Boasson <eb@ilities.com>
This commit is contained in:
Erik Boasson 2018-12-30 09:57:25 +01:00 • committed by Erik Boasson
parent e0d5587639
commit d6306bddbb
4 changed files with 33 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

1
src/core/ddsi/include/ddsi/q_error.h
View file

@ -22,5 +22,6 @@
#define ERR_BUSY -8
#define ERR_NO_ADDRESS -9
#define ERR_TIMEOUT -10
#define ERR_INCOMPATIBLE -11
#endif /* NN_ERROR_H */

32
src/core/ddsi/src/q_ddsi_discovery.c
View file

@ -799,14 +799,16 @@ static void handle_SPDP (const struct receiver_state *rst, nn_wctime_t timestamp
nn_plist_t decoded_data;
nn_plist_src_t src;
int interesting = 0;
int plist_ret;
src.protocol_version = rst->protocol_version;
src.vendorid = rst->vendor;
src.encoding = data->identifier;
src.buf = (unsigned char *) data + 4;
src.bufsz = len - 4;
if (nn_plist_init_frommsg (&decoded_data, NULL, ~(uint64_t)0, ~(uint64_t)0, &src) < 0)
if ((plist_ret = nn_plist_init_frommsg (&decoded_data, NULL, ~(uint64_t)0, ~(uint64_t)0, &src)) < 0)
{
DDS_WARNING("SPDP (vendor %u.%u): invalid qos/parameters\n", src.vendorid.id[0], src.vendorid.id[1]);
if (plist_ret != ERR_INCOMPATIBLE)
DDS_WARNING("SPDP (vendor %u.%u): invalid qos/parameters\n", src.vendorid.id[0], src.vendorid.id[1]);
return;
}
@ -1340,14 +1342,16 @@ static void handle_SEDP (const struct receiver_state *rst, nn_wctime_t timestamp
{
nn_plist_t decoded_data;
nn_plist_src_t src;
int plist_ret;
src.protocol_version = rst->protocol_version;
src.vendorid = rst->vendor;
src.encoding = data->identifier;
src.buf = (unsigned char *) data + 4;
src.bufsz = len - 4;
if (nn_plist_init_frommsg (&decoded_data, NULL, ~(uint64_t)0, ~(uint64_t)0, &src) < 0)
if ((plist_ret = nn_plist_init_frommsg (&decoded_data, NULL, ~(uint64_t)0, ~(uint64_t)0, &src)) < 0)
{
DDS_WARNING("SEDP (vendor %u.%u): invalid qos/parameters\n", src.vendorid.id[0], src.vendorid.id[1]);
if (plist_ret != ERR_INCOMPATIBLE)
DDS_WARNING("SEDP (vendor %u.%u): invalid qos/parameters\n", src.vendorid.id[0], src.vendorid.id[1]);
return;
}
@ -1466,14 +1470,16 @@ static void handle_SEDP_CM (const struct receiver_state *rst, nn_entityid_t wr_e
{
nn_plist_t decoded_data;
nn_plist_src_t src;
int plist_ret;
src.protocol_version = rst->protocol_version;
src.vendorid = rst->vendor;
src.encoding = data->identifier;
src.buf = (unsigned char *) data + 4;
src.bufsz = len - 4;
if (nn_plist_init_frommsg (&decoded_data, NULL, ~(uint64_t)0, ~(uint64_t)0, &src) < 0)
if ((plist_ret = nn_plist_init_frommsg (&decoded_data, NULL, ~(uint64_t)0, ~(uint64_t)0, &src)) < 0)
{
DDS_WARNING("SEDP_CM (vendor %u.%u): invalid qos/parameters\n", src.vendorid.id[0], src.vendorid.id[1]);
if (plist_ret != ERR_INCOMPATIBLE)
DDS_WARNING("SEDP_CM (vendor %u.%u): invalid qos/parameters\n", src.vendorid.id[0], src.vendorid.id[1]);
return;
}
@ -1626,14 +1632,16 @@ static void handle_SEDP_GROUP (const struct receiver_state *rst, nn_wctime_t tim
{
nn_plist_t decoded_data;
nn_plist_src_t src;
int plist_ret;
src.protocol_version = rst->protocol_version;
src.vendorid = rst->vendor;
src.encoding = data->identifier;
src.buf = (unsigned char *) data + 4;
src.bufsz = len - 4;
if (nn_plist_init_frommsg (&decoded_data, NULL, ~(uint64_t)0, ~(uint64_t)0, &src) < 0)
if ((plist_ret = nn_plist_init_frommsg (&decoded_data, NULL, ~(uint64_t)0, ~(uint64_t)0, &src)) < 0)
{
DDS_WARNING("SEDP_GROUP (vendor %u.%u): invalid qos/parameters\n", src.vendorid.id[0], src.vendorid.id[1]);
if (plist_ret != ERR_INCOMPATIBLE)
DDS_WARNING("SEDP_GROUP (vendor %u.%u): invalid qos/parameters\n", src.vendorid.id[0], src.vendorid.id[1]);
return;
}
@ -1747,15 +1755,17 @@ int builtins_dqueue_handler (const struct nn_rsample_info *sampleinfo, const str
{
nn_plist_src_t src;
size_t qos_offset = NN_RDATA_SUBMSG_OFF (fragchain) + offsetof (Data_DataFrag_common_t, octetsToInlineQos) + sizeof (msg->octetsToInlineQos) + msg->octetsToInlineQos;
int plist_ret;
src.protocol_version = sampleinfo->rst->protocol_version;
src.vendorid = sampleinfo->rst->vendor;
src.encoding = (msg->smhdr.flags & SMFLAG_ENDIANNESS) ? PL_CDR_LE : PL_CDR_BE;
src.buf = NN_RMSG_PAYLOADOFF (fragchain->rmsg, qos_offset);
src.bufsz = NN_RDATA_PAYLOAD_OFF (fragchain) - qos_offset;
if (nn_plist_init_frommsg (&qos, NULL, PP_STATUSINFO | PP_KEYHASH, 0, &src) < 0)
if ((plist_ret = nn_plist_init_frommsg (&qos, NULL, PP_STATUSINFO | PP_KEYHASH, 0, &src)) < 0)
{
DDS_WARNING("data(builtin, vendor %u.%u): %x:%x:%x:%x #%"PRId64": invalid inline qos\n",
src.vendorid.id[0], src.vendorid.id[1], PGUID (srcguid), sampleinfo->seq);
if (plist_ret != ERR_INCOMPATIBLE)
DDS_WARNING("data(builtin, vendor %u.%u): %x:%x:%x:%x #%"PRId64": invalid inline qos\n",
src.vendorid.id[0], src.vendorid.id[1], PGUID (srcguid), sampleinfo->seq);
goto done_upd_deliv;
}
/* Complex qos bit also gets set when statusinfo bits other than

12
src/core/ddsi/src/q_plist.c
View file

@ -2779,18 +2779,18 @@ static int init_one_parameter
one implemented, and fail it if it isn't. I know all RFPs say
to be tolerant in what is accepted, but that is where the
bugs & the buffer overflows originate! */
if (pid & PID_UNRECOGNIZED_INCOMPATIBLE_FLAG)
if (pid & PID_UNRECOGNIZED_INCOMPATIBLE_FLAG) {
dest->present |= PP_INCOMPATIBLE;
else if (pid & PID_VENDORSPECIFIC_FLAG)
return ERR_INCOMPATIBLE;
} else if (pid & PID_VENDORSPECIFIC_FLAG) {
return 0;
else if (!protocol_version_is_newer (dd->protocol_version) && NN_STRICT_P)
{
} else if (!protocol_version_is_newer (dd->protocol_version) && NN_STRICT_P) {
DDS_TRACE("plist/init_one_parameter[pid=%u,mode=STRICT,proto=%u.%u]: undefined paramter id\n",
pid, dd->protocol_version.major, dd->protocol_version.minor);
return ERR_INVALID;
}
else
} else {
return 0;
}
}
assert (0);

8
src/core/ddsi/src/q_receive.c
View file

@ -1959,15 +1959,17 @@ static int deliver_user_data (const struct nn_rsample_info *sampleinfo, const st
{
nn_plist_src_t src;
size_t qos_offset = NN_RDATA_SUBMSG_OFF (fragchain) + offsetof (Data_DataFrag_common_t, octetsToInlineQos) + sizeof (msg->octetsToInlineQos) + msg->octetsToInlineQos;
int plist_ret;
src.protocol_version = rst->protocol_version;
src.vendorid = rst->vendor;
src.encoding = (msg->smhdr.flags & SMFLAG_ENDIANNESS) ? PL_CDR_LE : PL_CDR_BE;
src.buf = NN_RMSG_PAYLOADOFF (fragchain->rmsg, qos_offset);
src.bufsz = NN_RDATA_PAYLOAD_OFF (fragchain) - qos_offset;
if (nn_plist_init_frommsg (&qos, NULL, PP_STATUSINFO | PP_KEYHASH | PP_COHERENT_SET | PP_PRISMTECH_EOTINFO, 0, &src) < 0)
if ((plist_ret = nn_plist_init_frommsg (&qos, NULL, PP_STATUSINFO | PP_KEYHASH | PP_COHERENT_SET | PP_PRISMTECH_EOTINFO, 0, &src)) < 0)
{
DDS_WARNING ("data(application, vendor %u.%u): %x:%x:%x:%x #%"PRId64": invalid inline qos\n",
src.vendorid.id[0], src.vendorid.id[1], PGUID (pwr->e.guid), sampleinfo->seq);
if (plist_ret != ERR_INCOMPATIBLE)
DDS_WARNING ("data(application, vendor %u.%u): %x:%x:%x:%x #%"PRId64": invalid inline qos\n",
src.vendorid.id[0], src.vendorid.id[1], PGUID (pwr->e.guid), sampleinfo->seq);
return 0;
}
statusinfo = (qos.present & PP_STATUSINFO) ? qos.statusinfo : 0;