Backport various bugfixes to v3

.github/workflows/windows.yml

@@ -59,7 +59,7 @@ jobs:
           choco install jom
           echo "C:\Program Files\jom" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append
       - name: configure
-        run: cmake -G"Visual Studio 17 2022" -A ${{ matrix.platform }}
+        run: cmake -G"Visual Studio 18 2026" -A ${{ matrix.platform }}
           $env:superbuild
           $env:cmake_prefix_path
           -DCMAKE_BUILD_TYPE=RelWithDebInfo
@@ -144,7 +144,7 @@ jobs:
           choco install jom
           echo "C:\Program Files\jom" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append
       - name: configure
-        run: cmake -G"Visual Studio 17 2022" -A ${{ matrix.platform }}
+        run: cmake -G"Visual Studio 18 2026" -A ${{ matrix.platform }}
           $env:superbuild
           $env:cmake_prefix_path
           -DCMAKE_BUILD_TYPE=${{ matrix.config }}
@@ -223,7 +223,7 @@ jobs:
         run: |
           rem Build examples in Debug configuration
           cd /d %GITHUB_WORKSPACE%
-          cmake -G"Visual Studio 17 2022" -A ${{ matrix.platform }} ^
+          cmake -G"Visual Studio 18 2026" -A ${{ matrix.platform }} ^
             -DCMAKE_BUILD_TYPE=Debug ^
             -DCMAKE_PREFIX_PATH=%GITHUB_WORKSPACE%\combined ^
             -Bbuild-examples-debug ^
@@ -235,7 +235,7 @@ jobs:
         run: |
           rem Build examples in RelWithDebInfo configuration
           cd /d %GITHUB_WORKSPACE%
-          cmake -G"Visual Studio 17 2022" -A ${{ matrix.platform }} ^
+          cmake -G"Visual Studio 18 2026" -A ${{ matrix.platform }} ^
             -DCMAKE_BUILD_TYPE=RelWithDebInfo ^
             -DCMAKE_PREFIX_PATH=%GITHUB_WORKSPACE%\combined ^
             -Bbuild-examples-release ^

src/mavsdk/core/cli_arg.cpp

@@ -230,7 +230,9 @@ std::optional<int> CliArg::port_from_str(std::string_view str)
     int value;
     auto [ptr, ec] = std::from_chars(str.data(), str.data() + str.size(), value);
 
-    if (static_cast<bool>(ec) || value < 1 || value > 65535) {
+    // from_chars stops at the first non-digit and reports success, so also require
+    // that the whole string was consumed to reject inputs like "8080abc".
+    if (static_cast<bool>(ec) || ptr != str.data() + str.size() || value < 1 || value > 65535) {
         return {};
     }
     return {value};
@@ -284,7 +286,8 @@ bool CliArg::parse_serial(const std::string_view rest, bool flow_control_enabled
     auto [ptr, ec] =
         std::from_chars(baudrate_str.data(), baudrate_str.data() + baudrate_str.size(), value);
 
-    if (static_cast<bool>(ec)) {
+    // Reject trailing garbage (e.g. "57600xyz"); from_chars would otherwise accept it.
+    if (static_cast<bool>(ec) || ptr != baudrate_str.data() + baudrate_str.size()) {
         return {};
     }
 

src/mavsdk/core/connection.cpp

@@ -78,7 +78,8 @@ void Connection::receive_libmav_message(
     const Mavsdk::MavlinkMessage& message, Connection* connection)
 {
     // Register system ID when receiving a message from a new system.
-    if (_system_ids.find(message.system_id) == _system_ids.end()) {
+    {
+        std::lock_guard<std::mutex> lock(_system_ids_mutex);
         _system_ids.insert(message.system_id);
     }
 

src/mavsdk/core/curl_wrapper.cpp

@@ -103,6 +103,13 @@ bool CurlWrapper::download_file_to_path(
         progress.progress_callback = progress_callback;
 
         fp = fopen(path.c_str(), "wb");
+        if (fp == nullptr) {
+            LogErr() << "Error: cannot open file for writing: " << path;
+            if (nullptr != progress_callback) {
+                progress_callback(0, HttpStatus::Error, CURLcode::CURLE_WRITE_ERROR);
+            }
+            return false;
+        }
         curl_easy_setopt(curl.get(), CURLOPT_CONNECTTIMEOUT, 5L);
         curl_easy_setopt(curl.get(), CURLOPT_XFERINFOFUNCTION, download_progress_update);
         curl_easy_setopt(curl.get(), CURLOPT_PROGRESSDATA, &progress);

src/mavsdk/core/fs_utils.cpp

@@ -70,17 +70,17 @@ std::optional<std::filesystem::path> get_cache_directory()
     // Read /proc/self/cmdline
     std::ifstream cmdline("/proc/self/cmdline");
     std::string line;
-    if (std::getline(cmdline, line)) {
-        // line might have a trailing \0
-        if (line.length() > 0 && *line.end() == 0) {
-            line.pop_back();
-        }
+    // /proc/self/cmdline stores the package name followed by a NUL byte, which we want to exclude
+    if (std::getline(cmdline, line, '\0')) {
         return "/data/data/" + line + "/mavsdk_cache";
     }
 #elif defined(APPLE) || defined(LINUX)
     const char* homedir;
     if ((homedir = getenv("HOME")) == NULL) {
-        homedir = getpwuid(getuid())->pw_dir;
+        // getpwuid() can return NULL if there is no passwd entry for the uid (common in
+        // minimal containers), so guard against dereferencing it.
+        const struct passwd* pw = getpwuid(getuid());
+        homedir = (pw != nullptr) ? pw->pw_dir : nullptr;
     }
     if (!homedir) {
         return std::nullopt;

src/mavsdk/core/hostname_to_ip.cpp

@@ -41,20 +41,21 @@ std::optional<std::string> resolve_hostname_to_ip(const std::string& hostname)
         return {};
     }
 
-    std::string ipAddress;
+    std::optional<std::string> ipAddress;
     for (addrinfo* ptr = result; ptr != nullptr; ptr = ptr->ai_next) {
         sockaddr_in* sockaddrIpv4 = reinterpret_cast<sockaddr_in*>(ptr->ai_addr);
-        char ipStr[INET_ADDRSTRLEN];
-        inet_ntop(AF_INET, &(sockaddrIpv4->sin_addr), ipStr, INET_ADDRSTRLEN);
-        ipAddress = ipStr;
-        break; // Take the first result
+        char ipStr[INET_ADDRSTRLEN] = {};
+        if (inet_ntop(AF_INET, &(sockaddrIpv4->sin_addr), ipStr, INET_ADDRSTRLEN) != nullptr) {
+            ipAddress = ipStr;
+            break; // Take the first result
+        }
     }
 
     freeaddrinfo(result);
 #if defined(WINDOWS)
     WSACleanup();
 #endif
-    return {ipAddress};
+    return ipAddress;
 }
 
 } // namespace mavsdk

src/mavsdk/core/libmav_receiver.h

@@ -59,9 +59,16 @@ class LibmavReceiver {
     Mavsdk::MavlinkMessage _last_message;
     std::optional<mav::Message> _last_libmav_message; // Separate libmav message for integration
 
-    // Accumulation buffer for serial connections where messages can span multiple reads
-    // Buffer size is max message size (280 bytes: 255 payload + 10 header + 2 CRC + 13 signature)
-    static constexpr size_t ACCUMULATION_BUFFER_SIZE = mav::MessageDefinition::MAX_MESSAGE_SIZE;
+    // Accumulation buffer for connections where messages can span multiple reads.
+    //
+    // This bound is only a safety valve against unbounded growth; the parser normally
+    // drains the buffer (it consumes complete messages and skips garbage before the magic
+    // byte), so at most one partial message is ever pending. It must therefore be at least
+    // as large as a single read (the 2048-byte connection receive buffers) plus one full
+    // message, otherwise set_new_datagram() would drop the front of a large read *before*
+    // parsing and silently lose the messages in it.
+    static constexpr size_t ACCUMULATION_BUFFER_SIZE =
+        2048 + mav::MessageDefinition::MAX_MESSAGE_SIZE;
     std::vector<uint8_t> _accumulation_buffer;
 
     bool _debugging = false;

src/mavsdk/core/mavlink_component_metadata.cpp

@@ -5,6 +5,7 @@
 #include "unused.h"
 #include "system_impl.h"
 
+#include <cstdlib>
 #include <utility>
 #include <filesystem>
 #include <fstream>
@@ -136,7 +137,16 @@ bool MavlinkComponentMetadata::uri_is_mavlinkftp(
         std::regex compid_regex(R"(^(?:\[\;comp\=(\d+)\])(.*))");
         std::smatch match;
         if (std::regex_search(download_path, match, compid_regex)) {
-            target_compid = std::stoi(match[1]);
+            // Parse the component id without throwing (MAVSDK builds with -fno-exceptions,
+            // so std::stoi would terminate() on an out-of-range value from the wire).
+            const std::string compid_str = match[1].str();
+            char* end = nullptr;
+            const long parsed = std::strtol(compid_str.c_str(), &end, 10);
+            if (end == compid_str.c_str() || *end != '\0' || parsed < 0 || parsed > 255) {
+                LogWarn() << "Ignoring invalid component id in metadata uri: " << compid_str;
+                return false;
+            }
+            target_compid = static_cast<uint8_t>(parsed);
             download_path = match[2];
         }
         return true;

src/mavsdk/core/mavlink_ftp_client.cpp

@@ -150,11 +150,32 @@ void MavlinkFtpClient::process_mavlink_ftp_message(const mavlink_message_t& msg)
                   << ", req: " << (int)payload->req_opcode;
         return;
     }
-    if (work->last_received_seq_number != 0 &&
-        work->last_received_seq_number == payload->seq_number) {
-        // We have already seen this ack/nak.
-        LogWarn() << "Already seen";
-        return;
+    // For non-burst transfers, strictly check the expected seq_number.
+    // work->payload.seq_number is the seq_number of the most-recently sent
+    // request; the server echoes back seq_number + 1.  Accepting any other
+    // value would allow stale/duplicate UDP datagrams from a previous request
+    // to corrupt the transfer (e.g. writing a full 239-byte chunk when only
+    // the final 49 bytes remain).
+    //
+    // For burst transfers we cannot use a strict check: the server sends
+    // multiple packets per CMD_BURST_READ_FILE request, each with an
+    // incrementing seq_number, and packets may arrive out of order or with
+    // gaps that are filled by a subsequent re-request.  For burst we fall back
+    // to the original duplicate-rejection approach.
+    const bool is_burst = std::holds_alternative<DownloadBurstItem>(work->item);
+    if (!is_burst) {
+        const auto expected_seq = static_cast<uint16_t>(work->payload.seq_number + 1);
+        if (payload->seq_number != expected_seq) {
+            LogWarn() << "Unexpected seq: got " << (int)payload->seq_number << ", expected "
+                      << (int)expected_seq;
+            return;
+        }
+    } else {
+        if (work->last_received_seq_number != 0 &&
+            work->last_received_seq_number == payload->seq_number) {
+            LogWarn() << "Already seen seq: " << (int)payload->seq_number;
+            return;
+        }
     }
 
     std::visit(
@@ -378,7 +399,10 @@ void MavlinkFtpClient::process_mavlink_ftp_message(const mavlink_message_t& msg)
             }},
         work->item);
 
-    work->last_received_seq_number = payload->seq_number;
+    // Track the last received seq for burst duplicate detection.
+    if (is_burst) {
+        work->last_received_seq_number = payload->seq_number;
+    }
 }
 
 bool MavlinkFtpClient::download_start(Work& work, DownloadItem& item)
@@ -538,6 +562,16 @@ bool MavlinkFtpClient::download_burst_continue(
                 return false;
             }
 
+            if (payload->offset > item.file_size) {
+                // The server should never point us past the end of the file. Reject rather
+                // than allocating/zero-filling a gap of up to ~4 GB from a bad offset.
+                LogWarn() << "Got payload offset " << (uint32_t)payload->offset
+                          << " past file size " << item.file_size;
+                item.callback(ClientResult::ProtocolError, {});
+                download_burst_end(work);
+                return false;
+            }
+
             // we missed a part
             item.missing_data.emplace_back(DownloadBurstItem::MissingData{
                 item.current_offset, payload->offset - item.current_offset});

src/mavsdk/core/mavlink_ftp_client.h

@@ -229,7 +229,7 @@ class MavlinkFtpClient {
         unsigned retries{RETRIES};
         bool started{false};
         Opcode last_opcode{};
-        uint16_t last_received_seq_number{0};
+        uint16_t last_received_seq_number{0}; // used for burst duplicate detection
         uint8_t target_compid{};
         Work(Item new_item, uint8_t target_compid_) :
             item(std::move(new_item)),

src/mavsdk/core/mavlink_ftp_server.cpp

@@ -223,17 +223,32 @@ void MavlinkFtpServer::_send_mavlink_ftp_message(const PayloadHeader& payload)
 
 std::string MavlinkFtpServer::_data_as_string(const PayloadHeader& payload, size_t entry)
 {
+    // Only ever scan within the bytes the sender claims are valid. payload.size is
+    // validated to be <= max_data_length before we get here, but clamp defensively so
+    // we can never read past the fixed data[] buffer regardless of the input.
+    const size_t data_length = std::min(static_cast<size_t>(payload.size), size_t{max_data_length});
+
     size_t start = 0;
     size_t end = 0;
-    std::string result;
 
     for (int i = entry; i >= 0; --i) {
         start = end;
+        if (start >= data_length) {
+            // The requested entry is beyond the available data.
+            return {};
+        }
         end +=
-            strnlen(reinterpret_cast<const char*>(&payload.data[start]), max_data_length - start) +
-            1;
+            strnlen(reinterpret_cast<const char*>(&payload.data[start]), data_length - start) + 1;
+    }
+
+    // The trailing field may not be null-terminated within the valid data, in which case
+    // strnlen pushed end one past data_length. Clamp it back so the memcpy stays in bounds.
+    end = std::min(end, data_length);
+    if (end <= start) {
+        return {};
     }
 
+    std::string result;
     result.resize(end - start);
     std::memcpy(result.data(), &payload.data[start], end - start);
 
@@ -789,18 +804,30 @@ void MavlinkFtpServer::_work_burst(const PayloadHeader& payload)
 
     // Schedule sending out burst messages.
     _session_info.burst_thread = std::thread([this]() {
-        while (!_session_info.burst_stop)
-            if (_send_burst_packet())
+        while (!_session_info.burst_stop) {
+            // Try to grab the lock rather than blocking on it. If another thread holds
+            // _mutex to stop and join us, blocking here would deadlock; instead we fall
+            // back to observing burst_stop (atomic) and exit.
+            std::unique_lock<std::mutex> burst_lock(_mutex, std::try_to_lock);
+            if (!burst_lock.owns_lock()) {
+                std::this_thread::yield();
+                continue;
+            }
+            if (_session_info.burst_stop) {
                 break;
+            }
+            if (_send_burst_packet()) {
+                break;
+            }
+        }
     });
 
     // Don't send response as that's done in the call every burst call above.
 }
 
-// Returns true if sending is complete
+// Requires _mutex to be held. Returns true if sending is complete.
 bool MavlinkFtpServer::_send_burst_packet()
 {
-    std::lock_guard<std::mutex> lock(_mutex);
     if (!_session_info.ifstream.is_open()) {
         return false;
     }
@@ -867,7 +894,7 @@ void MavlinkFtpServer::_work_write(const PayloadHeader& payload)
     }
 
     _session_info.ofstream.seekp(payload.offset);
-    if (_session_info.ifstream.fail()) {
+    if (_session_info.ofstream.fail()) {
         response.opcode = Opcode::RSP_NAK;
         response.size = 1;
         response.data[0] = ServerResult::ERR_FAIL;

src/mavsdk/core/mavlink_ftp_server.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <atomic>
 #include <cinttypes>
 #include <fstream>
 #include <unordered_map>
@@ -134,7 +135,7 @@ class MavlinkFtpServer {
     void _work_rename(const PayloadHeader& payload);
     void _work_calc_file_CRC32(const PayloadHeader& payload);
 
-    bool _send_burst_packet();
+    bool _send_burst_packet(); // Requires _mutex to be held.
     void _make_burst_packet(PayloadHeader& packet);
 
     std::mutex _mutex{};
@@ -144,13 +145,18 @@ class MavlinkFtpServer {
         uint8_t burst_chunk_size{0};
         std::ifstream ifstream;
         std::ofstream ofstream;
-        bool burst_stop{false};
+        // Atomic so the burst worker can observe a stop request without holding
+        // _mutex, which lets a thread that holds _mutex join the worker without
+        // deadlocking (the worker would otherwise block trying to re-acquire _mutex).
+        std::atomic<bool> burst_stop{false};
         std::thread burst_thread;
     } _session_info{};
 
     uint8_t _network_id = 0;
-    uint8_t _target_system_id = 0;
-    uint8_t _target_component_id = 0;
+    // Written from the message-processing thread and read from the burst thread
+    // (via _send_mavlink_ftp_message), so keep these atomic.
+    std::atomic<uint8_t> _target_system_id{0};
+    std::atomic<uint8_t> _target_component_id{0};
     std::string _root_dir{};
 
     std::mutex _tmp_files_mutex{};

src/mavsdk/core/mavlink_message_handler.cpp

@@ -58,6 +58,22 @@ void MavlinkMessageHandler::unregister_all_blocking(const void* cookie)
 {
     // Blocking version for use in destructors - waits for any in-flight callbacks to complete.
     // WARNING: Do NOT call this from within a message handler callback - it will deadlock.
+
+    // Also purge any deferred registrations for this cookie from _register_later_table.
+    // If register_one() was called while process_message() held _mutex, the entries land in
+    // _register_later_table instead of _table.  Without this step, check_register_later()
+    // would later promote those stale entries into _table and fire callbacks on a
+    // destroyed object (heap-use-after-free).
+    {
+        std::lock_guard<std::mutex> register_later_lock(_register_later_mutex);
+        _register_later_table.erase(
+            std::remove_if(
+                _register_later_table.begin(),
+                _register_later_table.end(),
+                [&](const auto& entry) { return entry.cookie == cookie; }),
+            _register_later_table.end());
+    }
+
     std::lock_guard<std::mutex> lock(_mutex);
     _table.erase(
         std::remove_if(