apacheGH-39690: [C++][FlightRPC] Fix nullptr dereference in PollInfo (a…

…pache#39711)

### Rationale for this change

The current implementation is a bit painful to use due to the lack of a move constructor.

### What changes are included in this PR?

- Fix a crash in PollInfo with a nullptr FlightInfo.
- Declare all necessary constructors (https://en.cppreference.com/w/cpp/language/rule_of_three)

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes, this adds new copy constructors.

* Closes: apache#39673.
* Closes: apache#39690

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: David Li <li.davidm96@gmail.com>

cpp/cmake_modules/FindClangTools.cmake

@@ -40,7 +40,8 @@ set(CLANG_TOOLS_SEARCH_PATHS
     /usr/local/bin
     /usr/bin
     "C:/Program Files/LLVM/bin" # Windows, non-conda
-    "$ENV{CONDA_PREFIX}/Library/bin") # Windows, conda
+    "$ENV{CONDA_PREFIX}/Library/bin" # Windows, conda
+    "$ENV{CONDA_PREFIX}/bin") # Unix, conda
 if(APPLE)
   find_program(BREW brew)
   if(BREW)

cpp/src/arrow/flight/flight_internals_test.cc

@@ -282,6 +282,7 @@ TEST(FlightTypes, PollInfo) {
                std::nullopt},
       PollInfo{std::make_unique<FlightInfo>(info), FlightDescriptor::Command("poll"), 0.1,
                expiration_time},
+      PollInfo{},
   };
   std::vector<std::string> reprs = {
       "<PollInfo info=" + info.ToString() +
@@ -290,6 +291,7 @@ TEST(FlightTypes, PollInfo) {
       "<PollInfo info=" + info.ToString() +
           " descriptor=<FlightDescriptor cmd='poll'> "
           "progress=0.1 expiration_time=2023-06-19 03:14:06.004339000>",
+      "<PollInfo info=null descriptor=null progress=null expiration_time=null>",
   };
 
   ASSERT_NO_FATAL_FAILURE(TestRoundtrip<pb::PollInfo>(values, reprs));

cpp/src/arrow/flight/serialization_internal.cc

@@ -306,8 +306,10 @@ Status ToProto(const FlightInfo& info, pb::FlightInfo* pb_info) {
 // PollInfo
 
 Status FromProto(const pb::PollInfo& pb_info, PollInfo* info) {
-  ARROW_ASSIGN_OR_RAISE(auto flight_info, FromProto(pb_info.info()));
-  info->info = std::make_unique<FlightInfo>(std::move(flight_info));
+  if (pb_info.has_info()) {
+    ARROW_ASSIGN_OR_RAISE(auto flight_info, FromProto(pb_info.info()));
+    info->info = std::make_unique<FlightInfo>(std::move(flight_info));
+  }
   if (pb_info.has_flight_descriptor()) {
     FlightDescriptor descriptor;
     RETURN_NOT_OK(FromProto(pb_info.flight_descriptor(), &descriptor));
@@ -331,7 +333,9 @@ Status FromProto(const pb::PollInfo& pb_info, PollInfo* info) {
 }
 
 Status ToProto(const PollInfo& info, pb::PollInfo* pb_info) {
-  RETURN_NOT_OK(ToProto(*info.info, pb_info->mutable_info()));
+  if (info.info) {
+    RETURN_NOT_OK(ToProto(*info.info, pb_info->mutable_info()));
+  }
   if (info.descriptor) {
     RETURN_NOT_OK(ToProto(*info.descriptor, pb_info->mutable_flight_descriptor()));
   }

cpp/src/arrow/flight/types.cc

@@ -373,7 +373,12 @@ arrow::Result<std::unique_ptr<PollInfo>> PollInfo::Deserialize(
 
 std::string PollInfo::ToString() const {
   std::stringstream ss;
-  ss << "<PollInfo info=" << info->ToString();
+  ss << "<PollInfo info=";
+  if (info) {
+    ss << info->ToString();
+  } else {
+    ss << "null";
+  }
   ss << " descriptor=";
   if (descriptor) {
     ss << descriptor->ToString();

cpp/src/arrow/flight/types.h

@@ -693,11 +693,22 @@ class ARROW_FLIGHT_EXPORT PollInfo {
         progress(progress),
         expiration_time(expiration_time) {}
 
-  explicit PollInfo(const PollInfo& other)
+  // Must not be explicit; to declare one we must declare all ("rule of five")
+  PollInfo(const PollInfo& other)  // NOLINT(runtime/explicit)
       : info(other.info ? std::make_unique<FlightInfo>(*other.info) : NULLPTR),
         descriptor(other.descriptor),
         progress(other.progress),
         expiration_time(other.expiration_time) {}
+  PollInfo(PollInfo&& other) noexcept = default;  // NOLINT(runtime/explicit)
+  ~PollInfo() = default;
+  PollInfo& operator=(const PollInfo& other) {
+    info = other.info ? std::make_unique<FlightInfo>(*other.info) : NULLPTR;
+    descriptor = other.descriptor;
+    progress = other.progress;
+    expiration_time = other.expiration_time;
+    return *this;
+  }
+  PollInfo& operator=(PollInfo&& other) = default;
 
   /// \brief Get the wire-format representation of this type.
   ///