[BUG] Consistent and critical failure parsing large (>500 lines) YAML configuration files.

[joncppl]

Bug report

Required Info:

• Operating System:
  • Ubuntu 18.04
• Installation type:
  • Latest Crystal Binaries (also Docker Image)
• Version or commit hash:
  • Crystal Patch 4
• DDS implementation:
  • Fast-RTPS
• Client library (if applicable):
  • both

Steps to reproduce issue

Make any yaml parameter file is very long

/:
  test:
    ros__parameters:
        param1: 0
        param2: 1
        # .... continue for about 500 lines

Run with any node

ros2 run examples_rclcpp_minimal_subscriber subscriber_member_function __params:=./bigyamlfile.yml

The same occurs with the python examples, and every node I've tried.

Expected behavior

Node load as normal with the parameters set.

Actual behavior

Critical Failure with corrupted size vs. prev_size during node construction.

Additional information

The abort comes during the call to yaml_parser_delete. https://github.com/ros2/rcl/blob/master/rcl_yaml_param_parser/src/parser.c#L1384

I think that libyaml's parser object is somehow getting corrupted.

I have succeeded in parsing such yaml files with libyaml (0.1.8), pyyaml, etc., with no issue.

[joncppl]

I was looking over this again and found that maximum parameters is a hard coded constant at 512.

This splits this issue into separate parts:

1. still a bug imo - Parameter loading doesn't check against the maximum, allowing for write past end of buffer, hence the memory corruption issues I'm seeing.

2. Likely an indepentend Feature request - max parameters would be configurable by some means, such as override by environment variable or an rcl method to set it. Maybe this is hard as the parameter buffer would need to be converted to dynamic memory.

3. max parameters should be documented somewhere so others dont go through the frustration I have.

[kekstee]

There's a related issue concerning the other hardcoded lengths in the parser: #244

[jacobperron]

I've added a guard against the memory corruption and an informative error message In #456.

I think we can re-phrase #244 to be about letting users configure the currently hard-coded limits.