Lack of Robustness in SelectionManager::setTextureSize Parameter Handling

[zhihaoshang]

Operating System:

Linux shangzh-VMware-Virtual-Platform 6.11.0-19-generic #19~24.04.1-Ubuntu SMP PREEMPT_DYNAMIC Mon Feb 17 11:51:52 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

ROS version or commit hash:

ros2 jazzy

RMW implementation (if applicable):

No response

RMW Configuration (if applicable):

No response

Client library (if applicable):

rviz

'ros2 doctor --report' output

ros2 doc --report

/home/shangzh/ros2_jazzy/install/ros2doctor/lib/python3.12/site-packages/ros2doctor/api/__init__.py: 162: UserWarning: Fail to call PackageReport class functions.
/home/shangzh/ros2_jazzy/install/ros2doctor/lib/python3.12/site-packages/ros2doctor/api/__init__.py: 162: UserWarning: Fail to call RosdistroReport class functions.

   NETWORK CONFIGURATION
inet         : 127.0.0.1
inet4        : ['127.0.0.1']
inet6        : ['::1']
netmask      : 255.0.0.0
device       : lo
flags        : 73<RUNNING,LOOPBACK,UP>
mtu          : 65536
inet         : 192.168.148.137
inet4        : ['192.168.148.137']
ether        : 00:0c:29:be:c8:19
inet6        : ['fe80::20c:29ff:febe:c819%ens33']
netmask      : 255.255.255.0
device       : ens33
flags        : 4163<RUNNING,UP,BROADCAST,MULTICAST>
mtu          : 1500
broadcast    : 192.168.148.255

   PLATFORM INFORMATION
system           : Linux
platform info    : Linux-6.11.0-19-generic-x86_64-with-glibc2.39
release          : 6.11.0-19-generic
processor        : x86_64

   QOS COMPATIBILITY LIST
compatibility status    : No publisher/subscriber pairs found

   RMW MIDDLEWARE
middleware name    : rmw_fastrtps_cpp

   TOPIC LIST
topic               : none
publisher count     : 0
subscriber count    : 0

Steps to reproduce issue

Environment

OS Version: Ubuntu 24.04
rviz version: ros2 jazzy
Compiler name and version number: Ubuntu clang version 18.1.3
Source or binary build?
source build
build options: --mixin asan-gcc

Description:

The current implementation of SelectionManager::setTextureSize uses an unsigned type for the size parameter and lacks proper validation for invalid inputs (e.g., negative values or excessively large values). When a negative value such as -1 is passed, it is implicitly converted to a large unsigned value (4294967295), which is then silently clamped to 1024 without raising any error or warning.

This behavior violates principles of robust API design and contradicts the expectations set in the unit test.

Test Case

#include <gmock/gmock.h>
#include <memory>
#include <string>
#include <vector>
#include "rviz_common/interaction/selection_manager.hpp"
#include "rviz_common/interaction/selection_handler.hpp"
#include "rviz_common/display_context.hpp"
#include "selection_test_fixture.hpp"
#include "mock_selection_renderer.hpp"
using namespace ::testing;  // NOLINT

class SelectionManagerTestFixture : public SelectionTestFixture
{
public:
  VisibleObject addVisibleObject(int x, int y)
  {
    VisibleObject object(x, y, context_.get());
    renderer_->addVisibleObject(object);
    return object;
  }
  ~SelectionManagerTestFixture()
  {
    if (render_window_) {
      delete render_window_;
    }
  }
  rviz_rendering::RenderWindow * render_window_ = nullptr;
};

TEST_F(SelectionManagerTestFixture, texture_size_range_check) {
  // Ensure invalid texture size does not cause issues
  EXPECT_THROW(selection_manager_->setTextureSize(-1), std::out_of_range);
  EXPECT_NO_THROW(selection_manager_->setTextureSize(256));
}

Output

Running main() from gmock_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from SelectionManagerTestFixture
[ RUN      ] SelectionManagerTestFixture.texture_size_range_check
[rviz_rendering:debug] Available Renderers(1): OpenGL Rendering Subsystem, at /home/shangzh/ros2_jazzy/src/ros2/rviz/rviz_rendering/src/rviz_rendering/render_system.cpp:289
[rviz_rendering:info] Stereo is NOT SUPPORTED, at /home/shangzh/ros2_jazzy/src/ros2/rviz/rviz_rendering/src/rviz_rendering/render_system.cpp:531
[rviz_rendering:info] OpenGl version: 4.3 (GLSL 4.3), at /home/shangzh/ros2_jazzy/src/ros2/rviz/rviz_rendering/src/rviz_rendering/render_system.cpp:272
/home/shangzh/ros2_jazzy/src/ros2/rviz/rviz_common/test/interaction/selection_manager_test.cpp:32: Failure
Expected: selection_manager_->setTextureSize(-1) throws an exception of type std::out_of_range.
  Actual: it throws nothing.

[  FAILED  ] SelectionManagerTestFixture.texture_size_range_check (2596 ms)
[----------] 1 test from SelectionManagerTestFixture (2596 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (2596 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SelectionManagerTestFixture.texture_size_range_check

 1 FAILED TEST

Expected behavior

When an invalid texture size (e.g., -1) is passed to setTextureSize, the function should:

Reject the input as invalid.

Throw a std::out_of_range exception to notify the caller of improper use.

Prevent silent fallback to a default or clamped value without user acknowledgment.

Actual behavior

When -1 is passed to setTextureSize:

The value is implicitly cast to 4294967295 due to the use of an unsigned type.

The function clamps the value to 1024 silently without throwing any exception or warning.

The unit test expecting std::out_of_range fails because no exception is thrown.

Additional information

No response