From f9050cd6663ccde9c4b6cabc8c7fa3ad154879ce Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 13 Dec 2022 15:40:16 -0300 Subject: [PATCH] fix nullptr dereference in prune_requests_older_than (#2008) (#2065) * fix nullptr dereference in prune_requests_older_than Signed-off-by: akela1101 * add tests for prune_requests_older_than Signed-off-by: akela1101 * Update rclcpp/test/rclcpp/test_client.cpp Co-authored-by: Chen Lihui Signed-off-by: akela1101 Signed-off-by: akela1101 Co-authored-by: Chen Lihui (cherry picked from commit 1ac37b692c4cce54f0ffeaad1f4fe3d5688322bd) Co-authored-by: andrei --- rclcpp/include/rclcpp/client.hpp | 4 +++- rclcpp/test/rclcpp/test_client.cpp | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/rclcpp/include/rclcpp/client.hpp b/rclcpp/include/rclcpp/client.hpp index e88fa8a949..adc9094f61 100644 --- a/rclcpp/include/rclcpp/client.hpp +++ b/rclcpp/include/rclcpp/client.hpp @@ -769,7 +769,9 @@ class Client : public ClientBase auto old_size = pending_requests_.size(); for (auto it = pending_requests_.begin(), last = pending_requests_.end(); it != last; ) { if (it->second.first < time_point) { - pruned_requests->push_back(it->first); + if (pruned_requests) { + pruned_requests->push_back(it->first); + } it = pending_requests_.erase(it); } else { ++it; diff --git a/rclcpp/test/rclcpp/test_client.cpp b/rclcpp/test/rclcpp/test_client.cpp index 7cb9b0af65..7cfb7c3213 100644 --- a/rclcpp/test/rclcpp/test_client.cpp +++ b/rclcpp/test/rclcpp/test_client.cpp @@ -282,6 +282,27 @@ TEST_F(TestClientWithServer, test_client_remove_pending_request) { EXPECT_TRUE(client->remove_pending_request(future)); } +TEST_F(TestClientWithServer, prune_requests_older_than_no_pruned) { + auto client = node->create_client(service_name); + auto request = std::make_shared(); + auto future = client->async_send_request(request); + auto time = std::chrono::system_clock::now() + 1s; + + EXPECT_EQ(1u, client->prune_requests_older_than(time)); +} + +TEST_F(TestClientWithServer, prune_requests_older_than_with_pruned) { + auto client = node->create_client(service_name); + auto request = std::make_shared(); + auto future = client->async_send_request(request); + auto time = std::chrono::system_clock::now() + 1s; + + std::vector pruned_requests; + EXPECT_EQ(1u, client->prune_requests_older_than(time, &pruned_requests)); + ASSERT_EQ(1u, pruned_requests.size()); + EXPECT_EQ(future.request_id, pruned_requests[0]); +} + TEST_F(TestClientWithServer, async_send_request_rcl_send_request_error) { // Checking rcl_send_request in rclcpp::Client::async_send_request() auto mock = mocking_utils::patch_and_return("lib:rclcpp", rcl_send_request, RCL_RET_ERROR);