From 0d8807ba2d4a8cc127097912dcc3afaf9e5570e6 Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Fri, 7 Jun 2019 14:56:05 -0700 Subject: [PATCH 01/10] Fix memory corruption when maximum number of parameters is exceeded (#456) If the maximum number is exceeded fail with an informative error message. Fixes #419. Signed-off-by: Jacob Perron --- rcl_yaml_param_parser/src/parser.c | 9 + .../test/max_num_params.yaml | 519 ++++++++++++++++++ .../test/test_parse_yaml.cpp | 18 + 3 files changed, 546 insertions(+) create mode 100644 rcl_yaml_param_parser/test/max_num_params.yaml diff --git a/rcl_yaml_param_parser/src/parser.c b/rcl_yaml_param_parser/src/parser.c index 4dff02392..48bc28644 100644 --- a/rcl_yaml_param_parser/src/parser.c +++ b/rcl_yaml_param_parser/src/parser.c @@ -1129,6 +1129,15 @@ static rcl_ret_t parse_key( } *is_new_map = false; } + + // Guard against adding more than the maximum allowed parameters + if (params_st->params[node_idx].num_params >= MAX_NUM_PARAMS_PER_NODE) { + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( + "Exceeded maximum allowed number of parameters for a node (%d)", + MAX_NUM_PARAMS_PER_NODE); + return RCL_RET_ERROR; + } + /// Add a parameter name into the node parameters parameter_idx = params_st->params[node_idx].num_params; parameter_ns = ns_tracker->parameter_ns; diff --git a/rcl_yaml_param_parser/test/max_num_params.yaml b/rcl_yaml_param_parser/test/max_num_params.yaml new file mode 100644 index 000000000..2e50c84fd --- /dev/null +++ b/rcl_yaml_param_parser/test/max_num_params.yaml @@ -0,0 +1,519 @@ +# config/test_yaml +--- + +foo_ns: + foo_name: + ros__parameters: + param1: 1 + param2: 2 + param3: 3 + param4: 4 + param5: 5 + param6: 6 + param7: 7 + param8: 8 + param9: 9 + param10: 10 + param11: 11 + param12: 12 + param13: 13 + param14: 14 + param15: 15 + param16: 16 + param17: 17 + param18: 18 + param19: 19 + param20: 20 + param21: 21 + param22: 22 + param23: 23 + param24: 24 + param25: 25 + param26: 26 + param27: 27 + param28: 28 + param29: 29 + param30: 30 + param31: 31 + param32: 32 + param33: 33 + param34: 34 + param35: 35 + param36: 36 + param37: 37 + param38: 38 + param39: 39 + param40: 40 + param41: 41 + param42: 42 + param43: 43 + param44: 44 + param45: 45 + param46: 46 + param47: 47 + param48: 48 + param49: 49 + param50: 50 + param51: 51 + param52: 52 + param53: 53 + param54: 54 + param55: 55 + param56: 56 + param57: 57 + param58: 58 + param59: 59 + param60: 60 + param61: 61 + param62: 62 + param63: 63 + param64: 64 + param65: 65 + param66: 66 + param67: 67 + param68: 68 + param69: 69 + param70: 70 + param71: 71 + param72: 72 + param73: 73 + param74: 74 + param75: 75 + param76: 76 + param77: 77 + param78: 78 + param79: 79 + param80: 80 + param81: 81 + param82: 82 + param83: 83 + param84: 84 + param85: 85 + param86: 86 + param87: 87 + param88: 88 + param89: 89 + param90: 90 + param91: 91 + param92: 92 + param93: 93 + param94: 94 + param95: 95 + param96: 96 + param97: 97 + param98: 98 + param99: 99 + param100: 100 + param101: 101 + param102: 102 + param103: 103 + param104: 104 + param105: 105 + param106: 106 + param107: 107 + param108: 108 + param109: 109 + param110: 110 + param111: 111 + param112: 112 + param113: 113 + param114: 114 + param115: 115 + param116: 116 + param117: 117 + param118: 118 + param119: 119 + param120: 120 + param121: 121 + param122: 122 + param123: 123 + param124: 124 + param125: 125 + param126: 126 + param127: 127 + param128: 128 + param129: 129 + param130: 130 + param131: 131 + param132: 132 + param133: 133 + param134: 134 + param135: 135 + param136: 136 + param137: 137 + param138: 138 + param139: 139 + param140: 140 + param141: 141 + param142: 142 + param143: 143 + param144: 144 + param145: 145 + param146: 146 + param147: 147 + param148: 148 + param149: 149 + param150: 150 + param151: 151 + param152: 152 + param153: 153 + param154: 154 + param155: 155 + param156: 156 + param157: 157 + param158: 158 + param159: 159 + param160: 160 + param161: 161 + param162: 162 + param163: 163 + param164: 164 + param165: 165 + param166: 166 + param167: 167 + param168: 168 + param169: 169 + param170: 170 + param171: 171 + param172: 172 + param173: 173 + param174: 174 + param175: 175 + param176: 176 + param177: 177 + param178: 178 + param179: 179 + param180: 180 + param181: 181 + param182: 182 + param183: 183 + param184: 184 + param185: 185 + param186: 186 + param187: 187 + param188: 188 + param189: 189 + param190: 190 + param191: 191 + param192: 192 + param193: 193 + param194: 194 + param195: 195 + param196: 196 + param197: 197 + param198: 198 + param199: 199 + param200: 200 + param201: 201 + param202: 202 + param203: 203 + param204: 204 + param205: 205 + param206: 206 + param207: 207 + param208: 208 + param209: 209 + param210: 210 + param211: 211 + param212: 212 + param213: 213 + param214: 214 + param215: 215 + param216: 216 + param217: 217 + param218: 218 + param219: 219 + param220: 220 + param221: 221 + param222: 222 + param223: 223 + param224: 224 + param225: 225 + param226: 226 + param227: 227 + param228: 228 + param229: 229 + param230: 230 + param231: 231 + param232: 232 + param233: 233 + param234: 234 + param235: 235 + param236: 236 + param237: 237 + param238: 238 + param239: 239 + param240: 240 + param241: 241 + param242: 242 + param243: 243 + param244: 244 + param245: 245 + param246: 246 + param247: 247 + param248: 248 + param249: 249 + param250: 250 + param251: 251 + param252: 252 + param253: 253 + param254: 254 + param255: 255 + param256: 256 + param257: 257 + param258: 258 + param259: 259 + param260: 260 + param261: 261 + param262: 262 + param263: 263 + param264: 264 + param265: 265 + param266: 266 + param267: 267 + param268: 268 + param269: 269 + param270: 270 + param271: 271 + param272: 272 + param273: 273 + param274: 274 + param275: 275 + param276: 276 + param277: 277 + param278: 278 + param279: 279 + param280: 280 + param281: 281 + param282: 282 + param283: 283 + param284: 284 + param285: 285 + param286: 286 + param287: 287 + param288: 288 + param289: 289 + param290: 290 + param291: 291 + param292: 292 + param293: 293 + param294: 294 + param295: 295 + param296: 296 + param297: 297 + param298: 298 + param299: 299 + param300: 300 + param301: 301 + param302: 302 + param303: 303 + param304: 304 + param305: 305 + param306: 306 + param307: 307 + param308: 308 + param309: 309 + param310: 310 + param311: 311 + param312: 312 + param313: 313 + param314: 314 + param315: 315 + param316: 316 + param317: 317 + param318: 318 + param319: 319 + param320: 320 + param321: 321 + param322: 322 + param323: 323 + param324: 324 + param325: 325 + param326: 326 + param327: 327 + param328: 328 + param329: 329 + param330: 330 + param331: 331 + param332: 332 + param333: 333 + param334: 334 + param335: 335 + param336: 336 + param337: 337 + param338: 338 + param339: 339 + param340: 340 + param341: 341 + param342: 342 + param343: 343 + param344: 344 + param345: 345 + param346: 346 + param347: 347 + param348: 348 + param349: 349 + param350: 350 + param351: 351 + param352: 352 + param353: 353 + param354: 354 + param355: 355 + param356: 356 + param357: 357 + param358: 358 + param359: 359 + param360: 360 + param361: 361 + param362: 362 + param363: 363 + param364: 364 + param365: 365 + param366: 366 + param367: 367 + param368: 368 + param369: 369 + param370: 370 + param371: 371 + param372: 372 + param373: 373 + param374: 374 + param375: 375 + param376: 376 + param377: 377 + param378: 378 + param379: 379 + param380: 380 + param381: 381 + param382: 382 + param383: 383 + param384: 384 + param385: 385 + param386: 386 + param387: 387 + param388: 388 + param389: 389 + param390: 390 + param391: 391 + param392: 392 + param393: 393 + param394: 394 + param395: 395 + param396: 396 + param397: 397 + param398: 398 + param399: 399 + param400: 400 + param401: 401 + param402: 402 + param403: 403 + param404: 404 + param405: 405 + param406: 406 + param407: 407 + param408: 408 + param409: 409 + param410: 410 + param411: 411 + param412: 412 + param413: 413 + param414: 414 + param415: 415 + param416: 416 + param417: 417 + param418: 418 + param419: 419 + param420: 420 + param421: 421 + param422: 422 + param423: 423 + param424: 424 + param425: 425 + param426: 426 + param427: 427 + param428: 428 + param429: 429 + param430: 430 + param431: 431 + param432: 432 + param433: 433 + param434: 434 + param435: 435 + param436: 436 + param437: 437 + param438: 438 + param439: 439 + param440: 440 + param441: 441 + param442: 442 + param443: 443 + param444: 444 + param445: 445 + param446: 446 + param447: 447 + param448: 448 + param449: 449 + param450: 450 + param451: 451 + param452: 452 + param453: 453 + param454: 454 + param455: 455 + param456: 456 + param457: 457 + param458: 458 + param459: 459 + param460: 460 + param461: 461 + param462: 462 + param463: 463 + param464: 464 + param465: 465 + param466: 466 + param467: 467 + param468: 468 + param469: 469 + param470: 470 + param471: 471 + param472: 472 + param473: 473 + param474: 474 + param475: 475 + param476: 476 + param477: 477 + param478: 478 + param479: 479 + param480: 480 + param481: 481 + param482: 482 + param483: 483 + param484: 484 + param485: 485 + param486: 486 + param487: 487 + param488: 488 + param489: 489 + param490: 490 + param491: 491 + param492: 492 + param493: 493 + param494: 494 + param495: 495 + param496: 496 + param497: 497 + param498: 498 + param499: 499 + param500: 500 + param501: 501 + param502: 502 + param503: 503 + param504: 504 + param505: 505 + param506: 506 + param507: 507 + param508: 508 + param509: 509 + param510: 510 + param511: 511 + param512: 512 + param513: 513 diff --git a/rcl_yaml_param_parser/test/test_parse_yaml.cpp b/rcl_yaml_param_parser/test/test_parse_yaml.cpp index 03934a351..339de5edd 100644 --- a/rcl_yaml_param_parser/test/test_parse_yaml.cpp +++ b/rcl_yaml_param_parser/test/test_parse_yaml.cpp @@ -225,6 +225,24 @@ TEST(test_file_parser, indented_ns) { allocator.deallocate(path, allocator.state); } +// Regression test for https://github.com/ros2/rcl/issues/419 +TEST(test_file_parser, maximum_number_parameters) { + rcutils_reset_error(); + EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + char * path = rcutils_join_path(test_path, "max_num_params.yaml", allocator); + fprintf(stderr, "cur_path: %s\n", path); + EXPECT_TRUE(rcutils_exists(path)); + rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); + EXPECT_FALSE(NULL == params_hdl); + bool res = rcl_parse_yaml_file(path, params_hdl); + fprintf(stderr, "%s\n", rcutils_get_error_string().str); + EXPECT_FALSE(res); + allocator.deallocate(test_path, allocator.state); + allocator.deallocate(path, allocator.state); +} + int32_t main(int32_t argc, char ** argv) { ::testing::InitGoogleTest(&argc, argv); From 9aa5bb1368576369a54652661aa8a4220e2cb08e Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Wed, 12 Jun 2019 20:28:15 +0000 Subject: [PATCH 02/10] Changelog. Signed-off-by: Chris Lalancette --- rcl/CHANGELOG.rst | 3 +++ rcl_action/CHANGELOG.rst | 3 +++ rcl_lifecycle/CHANGELOG.rst | 3 +++ rcl_yaml_param_parser/CHANGELOG.rst | 5 +++++ 4 files changed, 14 insertions(+) diff --git a/rcl/CHANGELOG.rst b/rcl/CHANGELOG.rst index c2ec3544f..99d758d3e 100644 --- a/rcl/CHANGELOG.rst +++ b/rcl/CHANGELOG.rst @@ -2,6 +2,9 @@ Changelog for package rcl ^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- + 0.7.4 (2019-05-29) ------------------ * Fix tests now that FastRTPS correctly reports that liveliness is not supported (`#452 `_) diff --git a/rcl_action/CHANGELOG.rst b/rcl_action/CHANGELOG.rst index 516bab32e..fbd80e035 100644 --- a/rcl_action/CHANGELOG.rst +++ b/rcl_action/CHANGELOG.rst @@ -2,6 +2,9 @@ Changelog for package rcl_action ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- + 0.7.4 (2019-05-29) ------------------ * rcl_action - user friendly error messages for invalid transitions (`#448 `_) diff --git a/rcl_lifecycle/CHANGELOG.rst b/rcl_lifecycle/CHANGELOG.rst index 008b4e5d2..8df11bccc 100644 --- a/rcl_lifecycle/CHANGELOG.rst +++ b/rcl_lifecycle/CHANGELOG.rst @@ -2,6 +2,9 @@ Changelog for package rcl_lifecycle ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- + 0.7.4 (2019-05-29) ------------------ diff --git a/rcl_yaml_param_parser/CHANGELOG.rst b/rcl_yaml_param_parser/CHANGELOG.rst index 81c8df8b7..90135ec8a 100644 --- a/rcl_yaml_param_parser/CHANGELOG.rst +++ b/rcl_yaml_param_parser/CHANGELOG.rst @@ -2,6 +2,11 @@ Changelog for package rcl_yaml_param_parser ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- +* Fix memory corruption when maximum number of parameters is exceeded (`#456 `_) +* Contributors: Jacob Perron + 0.7.4 (2019-05-29) ------------------ * Allow empty strings if they are quoted. (`#450 `_) From 1bf11ed0c2b53090f990262ba4003d63cb1e5460 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Wed, 12 Jun 2019 20:28:27 +0000 Subject: [PATCH 03/10] 0.7.5 --- rcl/CHANGELOG.rst | 4 ++-- rcl/package.xml | 2 +- rcl_action/CHANGELOG.rst | 4 ++-- rcl_action/package.xml | 2 +- rcl_lifecycle/CHANGELOG.rst | 4 ++-- rcl_lifecycle/package.xml | 2 +- rcl_yaml_param_parser/CHANGELOG.rst | 4 ++-- rcl_yaml_param_parser/package.xml | 2 +- 8 files changed, 12 insertions(+), 12 deletions(-) diff --git a/rcl/CHANGELOG.rst b/rcl/CHANGELOG.rst index 99d758d3e..ed8bed7a5 100644 --- a/rcl/CHANGELOG.rst +++ b/rcl/CHANGELOG.rst @@ -2,8 +2,8 @@ Changelog for package rcl ^^^^^^^^^^^^^^^^^^^^^^^^^ -Forthcoming ------------ +0.7.5 (2019-06-12) +------------------ 0.7.4 (2019-05-29) ------------------ diff --git a/rcl/package.xml b/rcl/package.xml index 041bde4fd..462b97661 100644 --- a/rcl/package.xml +++ b/rcl/package.xml @@ -2,7 +2,7 @@ rcl - 0.7.4 + 0.7.5 The ROS client library common implementation. This package contains an API which builds on the ROS middleware API and is optionally built upon by the other ROS client libraries. diff --git a/rcl_action/CHANGELOG.rst b/rcl_action/CHANGELOG.rst index fbd80e035..755e783cc 100644 --- a/rcl_action/CHANGELOG.rst +++ b/rcl_action/CHANGELOG.rst @@ -2,8 +2,8 @@ Changelog for package rcl_action ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Forthcoming ------------ +0.7.5 (2019-06-12) +------------------ 0.7.4 (2019-05-29) ------------------ diff --git a/rcl_action/package.xml b/rcl_action/package.xml index 8a148468f..3737d952d 100644 --- a/rcl_action/package.xml +++ b/rcl_action/package.xml @@ -2,7 +2,7 @@ rcl_action - 0.7.4 + 0.7.5 Package containing a C-based ROS action implementation Jacob Perron Apache License 2.0 diff --git a/rcl_lifecycle/CHANGELOG.rst b/rcl_lifecycle/CHANGELOG.rst index 8df11bccc..c5b3a10db 100644 --- a/rcl_lifecycle/CHANGELOG.rst +++ b/rcl_lifecycle/CHANGELOG.rst @@ -2,8 +2,8 @@ Changelog for package rcl_lifecycle ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Forthcoming ------------ +0.7.5 (2019-06-12) +------------------ 0.7.4 (2019-05-29) ------------------ diff --git a/rcl_lifecycle/package.xml b/rcl_lifecycle/package.xml index 025280069..cd92d323f 100644 --- a/rcl_lifecycle/package.xml +++ b/rcl_lifecycle/package.xml @@ -2,7 +2,7 @@ rcl_lifecycle - 0.7.4 + 0.7.5 Package containing a C-based lifecycle implementation Karsten Knese Apache License 2.0 diff --git a/rcl_yaml_param_parser/CHANGELOG.rst b/rcl_yaml_param_parser/CHANGELOG.rst index 90135ec8a..b41759ac1 100644 --- a/rcl_yaml_param_parser/CHANGELOG.rst +++ b/rcl_yaml_param_parser/CHANGELOG.rst @@ -2,8 +2,8 @@ Changelog for package rcl_yaml_param_parser ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Forthcoming ------------ +0.7.5 (2019-06-12) +------------------ * Fix memory corruption when maximum number of parameters is exceeded (`#456 `_) * Contributors: Jacob Perron diff --git a/rcl_yaml_param_parser/package.xml b/rcl_yaml_param_parser/package.xml index 2df2edf29..1c4b383c3 100644 --- a/rcl_yaml_param_parser/package.xml +++ b/rcl_yaml_param_parser/package.xml @@ -2,7 +2,7 @@ rcl_yaml_param_parser - 0.7.4 + 0.7.5 Package containing various utility types and functions for C Anup Pemmaiah Apache License 2.0 From 2d3587806da9be0bf0231ab076abdba9aeb213b0 Mon Sep 17 00:00:00 2001 From: ivanpauno Date: Mon, 29 Jul 2019 15:08:54 -0300 Subject: [PATCH 04/10] Accept quoted int or float values as strings (#464) (#474) Signed-off-by: ivanpauno --- rcl_yaml_param_parser/src/parser.c | 56 +++++++++++-------- .../test/string_array_with_quoted_number.yaml | 4 ++ .../test/test_parse_yaml.cpp | 22 +++++++- 3 files changed, 57 insertions(+), 25 deletions(-) create mode 100644 rcl_yaml_param_parser/test/string_array_with_quoted_number.yaml diff --git a/rcl_yaml_param_parser/src/parser.c b/rcl_yaml_param_parser/src/parser.c index 48bc28644..704470055 100644 --- a/rcl_yaml_param_parser/src/parser.c +++ b/rcl_yaml_param_parser/src/parser.c @@ -772,40 +772,48 @@ static void * get_value( } /// Check for int - errno = 0; - ival = strtol(value, &endptr, 0); - if ((0 == errno) && (NULL != endptr)) { - if ((NULL != endptr) && (endptr != value)) { - if (('\0' != *value) && ('\0' == *endptr)) { - *val_type = DATA_TYPE_INT64; - ret_val = allocator.zero_allocate(1U, sizeof(int64_t), allocator.state); - if (NULL == ret_val) { - return NULL; + if (style != YAML_SINGLE_QUOTED_SCALAR_STYLE && + style != YAML_DOUBLE_QUOTED_SCALAR_STYLE) + { + errno = 0; + ival = strtol(value, &endptr, 0); + if ((0 == errno) && (NULL != endptr)) { + if ((NULL != endptr) && (endptr != value)) { + if (('\0' != *value) && ('\0' == *endptr)) { + *val_type = DATA_TYPE_INT64; + ret_val = allocator.zero_allocate(1U, sizeof(int64_t), allocator.state); + if (NULL == ret_val) { + return NULL; + } + *((int64_t *)ret_val) = ival; + return ret_val; } - *((int64_t *)ret_val) = ival; - return ret_val; } } } /// Check for float - errno = 0; - endptr = NULL; - dval = strtod(value, &endptr); - if ((0 == errno) && (NULL != endptr)) { - if ((NULL != endptr) && (endptr != value)) { - if (('\0' != *value) && ('\0' == *endptr)) { - *val_type = DATA_TYPE_DOUBLE; - ret_val = allocator.zero_allocate(1U, sizeof(double), allocator.state); - if (NULL == ret_val) { - return NULL; + if (style != YAML_SINGLE_QUOTED_SCALAR_STYLE && + style != YAML_DOUBLE_QUOTED_SCALAR_STYLE) + { + errno = 0; + endptr = NULL; + dval = strtod(value, &endptr); + if ((0 == errno) && (NULL != endptr)) { + if ((NULL != endptr) && (endptr != value)) { + if (('\0' != *value) && ('\0' == *endptr)) { + *val_type = DATA_TYPE_DOUBLE; + ret_val = allocator.zero_allocate(1U, sizeof(double), allocator.state); + if (NULL == ret_val) { + return NULL; + } + *((double *)ret_val) = dval; + return ret_val; } - *((double *)ret_val) = dval; - return ret_val; } } + errno = 0; } - errno = 0; /// It is a string *val_type = DATA_TYPE_STRING; diff --git a/rcl_yaml_param_parser/test/string_array_with_quoted_number.yaml b/rcl_yaml_param_parser/test/string_array_with_quoted_number.yaml new file mode 100644 index 000000000..d6d304a2b --- /dev/null +++ b/rcl_yaml_param_parser/test/string_array_with_quoted_number.yaml @@ -0,0 +1,4 @@ +initial_params_node: + ros__parameters: + sa1: ["Four", "score"] + sa2: ["and", "7"] diff --git a/rcl_yaml_param_parser/test/test_parse_yaml.cpp b/rcl_yaml_param_parser/test/test_parse_yaml.cpp index 339de5edd..274e62f79 100644 --- a/rcl_yaml_param_parser/test/test_parse_yaml.cpp +++ b/rcl_yaml_param_parser/test/test_parse_yaml.cpp @@ -22,7 +22,6 @@ #include "rcutils/filesystem.h" static char cur_dir[1024]; -rcutils_allocator_t allocator = rcutils_get_default_allocator(); TEST(test_file_parser, correct_syntax) { rcutils_reset_error(); @@ -43,6 +42,27 @@ TEST(test_file_parser, correct_syntax) { allocator.deallocate(path, allocator.state); } +TEST(test_file_parser, string_array_with_quoted_number) { + rcutils_reset_error(); + EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); + rcutils_allocator_t allocator = rcutils_get_default_allocator(); + char * test_path = rcutils_join_path(cur_dir, "test", allocator); + char * path = rcutils_join_path(test_path, "string_array_with_quoted_number.yaml", allocator); + fprintf(stderr, "cur_path: %s\n", path); + EXPECT_TRUE(rcutils_exists(path)); + rcl_params_t * params_hdl = rcl_yaml_node_struct_init(allocator); + EXPECT_TRUE(params_hdl); + if (params_hdl) { + bool res = rcl_parse_yaml_file(path, params_hdl); + fprintf(stderr, "%s\n", rcutils_get_error_string().str); + EXPECT_TRUE(res); + rcl_yaml_node_struct_print(params_hdl); + rcl_yaml_node_struct_fini(params_hdl); + } + allocator.deallocate(test_path, allocator.state); + allocator.deallocate(path, allocator.state); +} + TEST(test_file_parser, multi_ns_correct_syntax) { rcutils_reset_error(); EXPECT_TRUE(rcutils_get_cwd(cur_dir, 1024)); From 477e27678d9bba2444362fce5a8cf099da89843e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steven!=20Ragnar=C3=B6k?= Date: Thu, 1 Aug 2019 04:38:29 -0700 Subject: [PATCH 05/10] 0.7.6 --- rcl/CHANGELOG.rst | 3 +++ rcl/package.xml | 2 +- rcl_action/CHANGELOG.rst | 3 +++ rcl_action/package.xml | 2 +- rcl_lifecycle/CHANGELOG.rst | 3 +++ rcl_lifecycle/package.xml | 2 +- rcl_yaml_param_parser/CHANGELOG.rst | 5 +++++ rcl_yaml_param_parser/package.xml | 2 +- 8 files changed, 18 insertions(+), 4 deletions(-) diff --git a/rcl/CHANGELOG.rst b/rcl/CHANGELOG.rst index ed8bed7a5..1f2518f65 100644 --- a/rcl/CHANGELOG.rst +++ b/rcl/CHANGELOG.rst @@ -2,6 +2,9 @@ Changelog for package rcl ^^^^^^^^^^^^^^^^^^^^^^^^^ +0.7.6 (2019-08-01) +------------------ + 0.7.5 (2019-06-12) ------------------ diff --git a/rcl/package.xml b/rcl/package.xml index 462b97661..d8befaca9 100644 --- a/rcl/package.xml +++ b/rcl/package.xml @@ -2,7 +2,7 @@ rcl - 0.7.5 + 0.7.6 The ROS client library common implementation. This package contains an API which builds on the ROS middleware API and is optionally built upon by the other ROS client libraries. diff --git a/rcl_action/CHANGELOG.rst b/rcl_action/CHANGELOG.rst index 755e783cc..8318dd9b4 100644 --- a/rcl_action/CHANGELOG.rst +++ b/rcl_action/CHANGELOG.rst @@ -2,6 +2,9 @@ Changelog for package rcl_action ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +0.7.6 (2019-08-01) +------------------ + 0.7.5 (2019-06-12) ------------------ diff --git a/rcl_action/package.xml b/rcl_action/package.xml index 3737d952d..e3ac5fdb7 100644 --- a/rcl_action/package.xml +++ b/rcl_action/package.xml @@ -2,7 +2,7 @@ rcl_action - 0.7.5 + 0.7.6 Package containing a C-based ROS action implementation Jacob Perron Apache License 2.0 diff --git a/rcl_lifecycle/CHANGELOG.rst b/rcl_lifecycle/CHANGELOG.rst index c5b3a10db..d1e50f177 100644 --- a/rcl_lifecycle/CHANGELOG.rst +++ b/rcl_lifecycle/CHANGELOG.rst @@ -2,6 +2,9 @@ Changelog for package rcl_lifecycle ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +0.7.6 (2019-08-01) +------------------ + 0.7.5 (2019-06-12) ------------------ diff --git a/rcl_lifecycle/package.xml b/rcl_lifecycle/package.xml index cd92d323f..a48d5abc7 100644 --- a/rcl_lifecycle/package.xml +++ b/rcl_lifecycle/package.xml @@ -2,7 +2,7 @@ rcl_lifecycle - 0.7.5 + 0.7.6 Package containing a C-based lifecycle implementation Karsten Knese Apache License 2.0 diff --git a/rcl_yaml_param_parser/CHANGELOG.rst b/rcl_yaml_param_parser/CHANGELOG.rst index b41759ac1..ec351e9cd 100644 --- a/rcl_yaml_param_parser/CHANGELOG.rst +++ b/rcl_yaml_param_parser/CHANGELOG.rst @@ -2,6 +2,11 @@ Changelog for package rcl_yaml_param_parser ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +0.7.6 (2019-08-01) +------------------ +* Accept quoted int or float values as strings. (`#474 `_) +* Contributors: ivanpauno + 0.7.5 (2019-06-12) ------------------ * Fix memory corruption when maximum number of parameters is exceeded (`#456 `_) diff --git a/rcl_yaml_param_parser/package.xml b/rcl_yaml_param_parser/package.xml index 1c4b383c3..a12cf2215 100644 --- a/rcl_yaml_param_parser/package.xml +++ b/rcl_yaml_param_parser/package.xml @@ -2,7 +2,7 @@ rcl_yaml_param_parser - 0.7.5 + 0.7.6 Package containing various utility types and functions for C Anup Pemmaiah Apache License 2.0 From 6aa3eb5f48ec85aa07260f240d1a59dc0ecd0491 Mon Sep 17 00:00:00 2001 From: Zachary Michaels Date: Thu, 19 Sep 2019 15:57:08 -0700 Subject: [PATCH 06/10] Increase MAX_STRING_SIZE (#487) (#503) * Increase MAX_STRING_SIZE It's too short for string length. It occurs the error when the string field in yaml files are too long.... Signed-off-by: Hyunseok Yang * update test to match increased limit Signed-off-by: Dirk Thomas Signed-off-by: Zachary Michaels --- rcl_yaml_param_parser/src/parser.c | 2 +- rcl_yaml_param_parser/test/max_string_sz.yaml | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/rcl_yaml_param_parser/src/parser.c b/rcl_yaml_param_parser/src/parser.c index 704470055..9042697ef 100644 --- a/rcl_yaml_param_parser/src/parser.c +++ b/rcl_yaml_param_parser/src/parser.c @@ -62,7 +62,7 @@ typedef struct namespace_tracker_s uint32_t num_parameter_ns; } namespace_tracker_t; -#define MAX_STRING_SIZE 128U +#define MAX_STRING_SIZE 256U #define PARAMS_KEY "ros__parameters" #define NODE_NS_SEPERATOR "/" #define PARAMETER_NS_SEPERATOR "." diff --git a/rcl_yaml_param_parser/test/max_string_sz.yaml b/rcl_yaml_param_parser/test/max_string_sz.yaml index 1b279f614..386f7be53 100644 --- a/rcl_yaml_param_parser/test/max_string_sz.yaml +++ b/rcl_yaml_param_parser/test/max_string_sz.yaml @@ -5,5 +5,4 @@ lidar_ns: lidar_1: ros__parameters: id: 10 - name: "A very long name that will not be supported by the yaml parser. The max supported string size is 128 - characters. Anything over the max size will be rejected" + name: "A long string that will not be supported by the yaml parser. The maximum supported length is 256 characters. Anything over that limit will be rejected and therefore this message has to be a bit longer in order to exceed that threshold by just one character." From 8a7fd407ea062f242d49c01025e18a57c5754abe Mon Sep 17 00:00:00 2001 From: Zachary Michaels Date: Fri, 20 Sep 2019 13:35:17 -0700 Subject: [PATCH 07/10] reset error message before setting a new one, embed the original one (#501) (#505) * reset error message before setting a new one, embed the original one Signed-off-by: Dirk Thomas * fix max line length Signed-off-by: Dirk Thomas Signed-off-by: Zachary Michaels --- rcl_lifecycle/src/rcl_lifecycle.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/rcl_lifecycle/src/rcl_lifecycle.c b/rcl_lifecycle/src/rcl_lifecycle.c index c6a8c7151..de1b29cf4 100644 --- a/rcl_lifecycle/src/rcl_lifecycle.c +++ b/rcl_lifecycle/src/rcl_lifecycle.c @@ -236,14 +236,20 @@ rcl_lifecycle_state_machine_fini( rcl_ret_t fcn_ret = RCL_RET_OK; if (rcl_lifecycle_com_interface_fini(&state_machine->com_interface, node_handle) != RCL_RET_OK) { - RCL_SET_ERROR_MSG("could not free lifecycle com interface. Leaking memory!\n"); + rcl_error_string_t error_string = rcl_get_error_string(); + rcutils_reset_error(); + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( + "could not free lifecycle com interface. Leaking memory!\n%s", error_string.str); fcn_ret = RCL_RET_ERROR; } if (rcl_lifecycle_transition_map_fini( &state_machine->transition_map, allocator) != RCL_RET_OK) { - RCL_SET_ERROR_MSG("could not free lifecycle transition map. Leaking memory!\n"); + rcl_error_string_t error_string = rcl_get_error_string(); + rcutils_reset_error(); + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( + "could not free lifecycle transition map. Leaking memory!\n%s", error_string.str); fcn_ret = RCL_RET_ERROR; } @@ -333,7 +339,9 @@ _trigger_transition( rcl_ret_t ret = rcl_lifecycle_com_interface_publish_notification( &state_machine->com_interface, transition->start, state_machine->current_state); if (ret != RCL_RET_OK) { - RCL_SET_ERROR_MSG("Could not publish transition"); + rcl_error_string_t error_string = rcl_get_error_string(); + rcutils_reset_error(); + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING("Could not publish transition: %s", error_string.str); return RCL_RET_ERROR; } } From 1b4b3dd5795ec5a3b8d2ed11d60d10c60657a26d Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Fri, 20 Sep 2019 15:45:30 -0500 Subject: [PATCH 08/10] 0.7.7 Signed-off-by: Michael Carroll --- rcl/CHANGELOG.rst | 3 +++ rcl/package.xml | 2 +- rcl_action/CHANGELOG.rst | 3 +++ rcl_action/package.xml | 2 +- rcl_lifecycle/CHANGELOG.rst | 5 +++++ rcl_lifecycle/package.xml | 2 +- rcl_yaml_param_parser/CHANGELOG.rst | 5 +++++ rcl_yaml_param_parser/package.xml | 2 +- 8 files changed, 20 insertions(+), 4 deletions(-) diff --git a/rcl/CHANGELOG.rst b/rcl/CHANGELOG.rst index 1f2518f65..fdb6505d3 100644 --- a/rcl/CHANGELOG.rst +++ b/rcl/CHANGELOG.rst @@ -2,6 +2,9 @@ Changelog for package rcl ^^^^^^^^^^^^^^^^^^^^^^^^^ +0.7.7 (2019-09-20) +------------------ + 0.7.6 (2019-08-01) ------------------ diff --git a/rcl/package.xml b/rcl/package.xml index d8befaca9..3fb04d7d8 100644 --- a/rcl/package.xml +++ b/rcl/package.xml @@ -2,7 +2,7 @@ rcl - 0.7.6 + 0.7.7 The ROS client library common implementation. This package contains an API which builds on the ROS middleware API and is optionally built upon by the other ROS client libraries. diff --git a/rcl_action/CHANGELOG.rst b/rcl_action/CHANGELOG.rst index 8318dd9b4..1524f9e6c 100644 --- a/rcl_action/CHANGELOG.rst +++ b/rcl_action/CHANGELOG.rst @@ -2,6 +2,9 @@ Changelog for package rcl_action ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +0.7.7 (2019-09-20) +------------------ + 0.7.6 (2019-08-01) ------------------ diff --git a/rcl_action/package.xml b/rcl_action/package.xml index e3ac5fdb7..bd3232e1a 100644 --- a/rcl_action/package.xml +++ b/rcl_action/package.xml @@ -2,7 +2,7 @@ rcl_action - 0.7.6 + 0.7.7 Package containing a C-based ROS action implementation Jacob Perron Apache License 2.0 diff --git a/rcl_lifecycle/CHANGELOG.rst b/rcl_lifecycle/CHANGELOG.rst index d1e50f177..6b4bbb2d9 100644 --- a/rcl_lifecycle/CHANGELOG.rst +++ b/rcl_lifecycle/CHANGELOG.rst @@ -2,6 +2,11 @@ Changelog for package rcl_lifecycle ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +0.7.7 (2019-09-20) +------------------ +* reset error message before setting a new one, embed the original one (`#501 `_) (`#505 `_) +* Contributors: Zachary Michaels + 0.7.6 (2019-08-01) ------------------ diff --git a/rcl_lifecycle/package.xml b/rcl_lifecycle/package.xml index a48d5abc7..b960f9946 100644 --- a/rcl_lifecycle/package.xml +++ b/rcl_lifecycle/package.xml @@ -2,7 +2,7 @@ rcl_lifecycle - 0.7.6 + 0.7.7 Package containing a C-based lifecycle implementation Karsten Knese Apache License 2.0 diff --git a/rcl_yaml_param_parser/CHANGELOG.rst b/rcl_yaml_param_parser/CHANGELOG.rst index ec351e9cd..b1164b91b 100644 --- a/rcl_yaml_param_parser/CHANGELOG.rst +++ b/rcl_yaml_param_parser/CHANGELOG.rst @@ -2,6 +2,11 @@ Changelog for package rcl_yaml_param_parser ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +0.7.7 (2019-09-20) +------------------ +* Increase MAX_STRING_SIZE (`#487 `_) (`#503 `_) +* Contributors: Zachary Michaels, Hyunseok Yang + 0.7.6 (2019-08-01) ------------------ * Accept quoted int or float values as strings. (`#474 `_) diff --git a/rcl_yaml_param_parser/package.xml b/rcl_yaml_param_parser/package.xml index a12cf2215..e9698cca5 100644 --- a/rcl_yaml_param_parser/package.xml +++ b/rcl_yaml_param_parser/package.xml @@ -2,7 +2,7 @@ rcl_yaml_param_parser - 0.7.6 + 0.7.7 Package containing various utility types and functions for C Anup Pemmaiah Apache License 2.0 From 4e4d49a0d6ed178765f3c31d1307e819a3956ab8 Mon Sep 17 00:00:00 2001 From: Pablo Garrido Date: Tue, 26 Nov 2019 15:09:20 +0100 Subject: [PATCH 09/10] Removed security directory --- rcl/CMakeLists.txt | 1 - rcl/include/rcl/security_directory.h | 67 ------- rcl/src/rcl/node.c | 2 +- rcl/src/rcl/security_directory.c | 264 --------------------------- 4 files changed, 1 insertion(+), 333 deletions(-) delete mode 100644 rcl/include/rcl/security_directory.h delete mode 100644 rcl/src/rcl/security_directory.c diff --git a/rcl/CMakeLists.txt b/rcl/CMakeLists.txt index c9779c41b..10649ba08 100644 --- a/rcl/CMakeLists.txt +++ b/rcl/CMakeLists.txt @@ -56,7 +56,6 @@ set(${PROJECT_NAME}_sources src/rcl/timer.c src/rcl/validate_topic_name.c src/rcl/wait.c - src/rcl/security_directory.c ) add_library(${PROJECT_NAME} ${${PROJECT_NAME}_sources}) diff --git a/rcl/include/rcl/security_directory.h b/rcl/include/rcl/security_directory.h deleted file mode 100644 index e3d8b52f7..000000000 --- a/rcl/include/rcl/security_directory.h +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright 2018 Open Source Robotics Foundation, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef RCL__SECURITY_DIRECTORY_H_ -#define RCL__SECURITY_DIRECTORY_H_ - -#ifdef __cplusplus -extern "C" -{ -#endif - -#include "rcl/allocator.h" -#include "rcl/visibility_control.h" - -#ifndef ROS_SECURITY_NODE_DIRECTORY_VAR_NAME - #define ROS_SECURITY_NODE_DIRECTORY_VAR_NAME "ROS_SECURITY_NODE_DIRECTORY" -#endif - -#ifndef ROS_SECURITY_ROOT_DIRECTORY_VAR_NAME - #define ROS_SECURITY_ROOT_DIRECTORY_VAR_NAME "ROS_SECURITY_ROOT_DIRECTORY" -#endif - -#ifndef ROS_SECURITY_LOOKUP_TYPE_VAR_NAME - #define ROS_SECURITY_LOOKUP_TYPE_VAR_NAME "ROS_SECURITY_LOOKUP_TYPE" -#endif - -/// Return the secure root directory associated with a node given its validated name and namespace. -/** - * E.g. for a node named "c" in namespace "/a/b", the secure root path will be - * "a/b/c", where the delimiter "/" is native for target file system (e.g. "\\" for _WIN32). - * If no exact match is found for the node name, a best match would be used instead - * (by performing longest-prefix matching). - * - * However, this expansion can be overridden by setting the secure node directory environment - * variable, allowing users to explicitly specify the exact secure root directory to be utilized. - * Such an override is useful for where the FQN of a node is non-deterministic before runtime, - * or when testing and using additional tools that may not otherwise be easily provisioned. - * - * \param[in] node_name validated node name (a single token) - * \param[in] node_namespace validated, absolute namespace (starting with "/") - * \param[in] allocator the allocator to use for allocation - * \returns machine specific (absolute) node secure root path or NULL on failure - * returned pointer must be deallocated by the caller of this function - */ -RCL_PUBLIC -char * rcl_get_secure_root( - const char * node_name, - const char * node_namespace, - const rcl_allocator_t * allocator -); - -#ifdef __cplusplus -} -#endif - -#endif // RCL__SECURITY_DIRECTORY_H_ diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index 2a31abe33..443a076d5 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -29,7 +29,7 @@ extern "C" #include "rcl/logging_rosout.h" #include "rcl/rcl.h" #include "rcl/remap.h" -#include "rcl/security_directory.h" +#include "rcutils/security_directory.h" #include "rcutils/filesystem.h" #include "rcutils/find.h" #include "rcutils/format_string.h" diff --git a/rcl/src/rcl/security_directory.c b/rcl/src/rcl/security_directory.c deleted file mode 100644 index 6e7d9f54e..000000000 --- a/rcl/src/rcl/security_directory.c +++ /dev/null @@ -1,264 +0,0 @@ -// Copyright 2018 Open Source Robotics Foundation, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "rcl/security_directory.h" - -#include "rcl/error_handling.h" -#include "rcutils/filesystem.h" -#include "rcutils/get_env.h" -#include "rcutils/format_string.h" - -#ifdef __clang__ -# pragma clang diagnostic push -# pragma clang diagnostic ignored "-Wembedded-directive" -#endif -#include "tinydir/tinydir.h" -#ifdef __clang__ -# pragma clang diagnostic pop -#endif - -/** - * A security lookup function takes in the node's name, namespace, a security root directory and an allocator; - * It returns the relevant information required to load the security credentials, - * which is currently a path to a directory on the filesystem containing DDS Security permission files. - */ -typedef char * (* security_lookup_fn_t) ( - const char * node_name, - const char * node_namespace, - const char * ros_secure_root_env, - const rcl_allocator_t * allocator -); - -char * exact_match_lookup( - const char * node_name, - const char * node_namespace, - const char * ros_secure_root_env, - const rcl_allocator_t * allocator -); - -char * prefix_match_lookup( - const char * node_name, - const char * node_namespace, - const char * ros_secure_root_env, - const rcl_allocator_t * allocator -); - -security_lookup_fn_t g_security_lookup_fns[] = { - NULL, - exact_match_lookup, - prefix_match_lookup, -}; - -typedef enum ros_security_lookup_type_e -{ - ROS_SECURITY_LOOKUP_NODE_OVERRIDE = 0, - ROS_SECURITY_LOOKUP_MATCH_EXACT = 1, - ROS_SECURITY_LOOKUP_MATCH_PREFIX = 2, -} ros_security_lookup_type_t; - -char * g_security_lookup_type_strings[] = { - "NODE_OVERRIDE", - "MATCH_EXACT", - "MATCH_PREFIX" -}; - -/// Return the directory whose name most closely matches node_name (longest-prefix match), -/// scanning under base_dir. -/** - * By using a prefix match, a node named e.g. "my_node_123" will be able to load and use the - * directory "my_node" if no better match exists. - * \param[in] base_dir - * \param[in] node_name - * \param[out] matched_name must be a valid memory address allocated with at least - * _TINYDIR_FILENAME_MAX characters. - * \return true if a match was found - */ -static bool get_best_matching_directory( - const char * base_dir, - const char * node_name, - char * matched_name) -{ - size_t max_match_length = 0; - tinydir_dir dir; - if (NULL == base_dir || NULL == node_name || NULL == matched_name) { - return false; - } - if (-1 == tinydir_open(&dir, base_dir)) { - return false; - } - while (dir.has_next) { - tinydir_file file; - if (-1 == tinydir_readfile(&dir, &file)) { - goto cleanup; - } - if (file.is_dir) { - size_t matched_name_length = strnlen(file.name, sizeof(file.name) - 1); - if (0 == - strncmp(file.name, node_name, - matched_name_length) && matched_name_length > max_match_length) - { - max_match_length = matched_name_length; - memcpy(matched_name, file.name, max_match_length); - } - } - if (-1 == tinydir_next(&dir)) { - goto cleanup; - } - } -cleanup: - tinydir_close(&dir); - return max_match_length > 0; -} - -char * exact_match_lookup( - const char * node_name, - const char * node_namespace, - const char * ros_secure_root_env, - const rcl_allocator_t * allocator) -{ - // Perform an exact match for the node's name in directory /. - char * node_secure_root = NULL; - // "/" case when root namespace is explicitly passed in - if (1 == strlen(node_namespace)) { - node_secure_root = rcutils_join_path(ros_secure_root_env, node_name, *allocator); - } else { - char * node_fqn = NULL; - char * node_root_path = NULL; - // Combine node namespace with node name - // TODO(ros2team): remove the hard-coded value of the root namespace - node_fqn = rcutils_format_string(*allocator, "%s%s%s", node_namespace, "/", node_name); - // Get native path, ignore the leading forward slash - // TODO(ros2team): remove the hard-coded length, use the length of the root namespace instead - node_root_path = rcutils_to_native_path(node_fqn + 1, *allocator); - node_secure_root = rcutils_join_path(ros_secure_root_env, node_root_path, *allocator); - allocator->deallocate(node_fqn, allocator->state); - allocator->deallocate(node_root_path, allocator->state); - } - return node_secure_root; -} - -char * prefix_match_lookup( - const char * node_name, - const char * node_namespace, - const char * ros_secure_root_env, - const rcl_allocator_t * allocator) -{ - // Perform longest prefix match for the node's name in directory /. - char * node_secure_root = NULL; - char matched_dir[_TINYDIR_FILENAME_MAX] = {0}; - char * base_lookup_dir = NULL; - if (strlen(node_namespace) == 1) { - base_lookup_dir = (char *) ros_secure_root_env; - } else { - // TODO(ros2team): remove the hard-coded length, use the length of the root namespace instead. - base_lookup_dir = rcutils_join_path(ros_secure_root_env, node_namespace + 1, *allocator); - } - if (get_best_matching_directory(base_lookup_dir, node_name, matched_dir)) { - node_secure_root = rcutils_join_path(base_lookup_dir, matched_dir, *allocator); - } - if (base_lookup_dir != ros_secure_root_env && NULL != base_lookup_dir) { - allocator->deallocate(base_lookup_dir, allocator->state); - } - return node_secure_root; -} - -char * rcl_get_secure_root( - const char * node_name, - const char * node_namespace, - const rcl_allocator_t * allocator) -{ - bool ros_secure_node_override = true; - - // find out if either of the configuration environment variables are set - const char * env_buf = NULL; - if (NULL == node_name) { - return NULL; - } - if (rcutils_get_env(ROS_SECURITY_NODE_DIRECTORY_VAR_NAME, &env_buf)) { - return NULL; - } - if (!env_buf) { - return NULL; - } - size_t ros_secure_root_size = strlen(env_buf); - if (!ros_secure_root_size) { - // check root directory if node directory environment variable is empty - if (rcutils_get_env(ROS_SECURITY_ROOT_DIRECTORY_VAR_NAME, &env_buf)) { - return NULL; - } - if (!env_buf) { - return NULL; - } - ros_secure_root_size = strlen(env_buf); - if (!ros_secure_root_size) { - return NULL; // environment variable was empty - } else { - ros_secure_node_override = false; - } - } - - // found a usable environment variable, copy into our memory before overwriting with next lookup - char * ros_secure_root_env = - (char *)allocator->allocate(ros_secure_root_size + 1, allocator->state); - memcpy(ros_secure_root_env, env_buf, ros_secure_root_size + 1); - // TODO(ros2team): This make an assumption on the value and length of the root namespace. - // This should likely come from another (rcl/rmw?) function for reuse. - // If the namespace is the root namespace ("/"), the secure root is just the node name. - - char * lookup_strategy = NULL; - char * node_secure_root = NULL; - if (ros_secure_node_override) { - node_secure_root = (char *)allocator->allocate(ros_secure_root_size + 1, allocator->state); - memcpy(node_secure_root, ros_secure_root_env, ros_secure_root_size + 1); - lookup_strategy = g_security_lookup_type_strings[ROS_SECURITY_LOOKUP_NODE_OVERRIDE]; - - } else { - // Check which lookup method to use and invoke the relevant function. - const char * ros_security_lookup_type = NULL; - if (rcutils_get_env(ROS_SECURITY_LOOKUP_TYPE_VAR_NAME, &ros_security_lookup_type)) { - allocator->deallocate(ros_secure_root_env, allocator->state); - return NULL; - } - if (0 == strcmp(ros_security_lookup_type, - g_security_lookup_type_strings[ROS_SECURITY_LOOKUP_MATCH_PREFIX])) - { - node_secure_root = g_security_lookup_fns[ROS_SECURITY_LOOKUP_MATCH_PREFIX] - (node_name, node_namespace, ros_secure_root_env, allocator); - lookup_strategy = g_security_lookup_type_strings[ROS_SECURITY_LOOKUP_MATCH_PREFIX]; - } else { /* Default is MATCH_EXACT */ - node_secure_root = g_security_lookup_fns[ROS_SECURITY_LOOKUP_MATCH_EXACT] - (node_name, node_namespace, ros_secure_root_env, allocator); - lookup_strategy = g_security_lookup_type_strings[ROS_SECURITY_LOOKUP_MATCH_EXACT]; - } - } - - if (NULL == node_secure_root || !rcutils_is_directory(node_secure_root)) { - // Check node_secure_root is not NULL before checking directory - if (NULL == node_secure_root) { - RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( - "SECURITY ERROR: unable to find a folder matching the node name in %s%s." - "Lookup strategy: %s", - ros_secure_root_env, node_namespace, lookup_strategy); - } else { - RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( - "SECURITY ERROR: directory %s does not exist. Lookup strategy: %s", - node_secure_root, lookup_strategy); - } - allocator->deallocate(ros_secure_root_env, allocator->state); - allocator->deallocate(node_secure_root, allocator->state); - return NULL; - } - allocator->deallocate(ros_secure_root_env, allocator->state); - return node_secure_root; -} From 6c404c84af367a1935556a06b20af6a7c2c53601 Mon Sep 17 00:00:00 2001 From: Pablo Garrido Date: Tue, 26 Nov 2019 16:08:01 +0100 Subject: [PATCH 10/10] Updated security directory --- rcl/CMakeLists.txt | 2 -- rcl/package.xml | 2 -- rcl/src/rcl/node.c | 2 +- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/rcl/CMakeLists.txt b/rcl/CMakeLists.txt index 10649ba08..364d69eef 100644 --- a/rcl/CMakeLists.txt +++ b/rcl/CMakeLists.txt @@ -9,7 +9,6 @@ find_package(rcutils REQUIRED) find_package(rmw REQUIRED) find_package(rmw_implementation REQUIRED) find_package(rosidl_generator_c REQUIRED) -find_package(tinydir_vendor REQUIRED) include_directories(include) include(cmake/rcl_set_symbol_visibility_hidden.cmake) @@ -67,7 +66,6 @@ ament_target_dependencies(${PROJECT_NAME} "rcutils" "rosidl_generator_c" ${RCL_LOGGING_IMPL} - "tinydir_vendor" ) # Causes the visibility macros to use dllexport rather than dllimport, diff --git a/rcl/package.xml b/rcl/package.xml index 3fb04d7d8..aa86d5f75 100644 --- a/rcl/package.xml +++ b/rcl/package.xml @@ -16,12 +16,10 @@ rcl_interfaces rcutils rosidl_generator_c - tinydir_vendor rcl_interfaces rcutils rosidl_generator_c - tinydir_vendor rcl_interfaces ament_cmake diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index 443a076d5..7a0bf52ca 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -307,7 +307,7 @@ rcl_node_init( node_security_options.enforce_security = RMW_SECURITY_ENFORCEMENT_PERMISSIVE; } else { // if use_security // File discovery magic here - node_secure_root = rcl_get_secure_root(name, local_namespace_, allocator); + node_secure_root = rcutils_get_secure_root(name, local_namespace_, allocator); if (node_secure_root) { RCUTILS_LOG_INFO_NAMED(ROS_PACKAGE_NAME, "Found security directory: %s", node_secure_root); node_security_options.security_root_path = node_secure_root;