From 49c05e86c7831d8920e7a29cddbf9956c76fa878 Mon Sep 17 00:00:00 2001 From: Mateen Anjum Date: Fri, 11 Sep 2026 00:53:04 -0400 Subject: [PATCH 1/4] [BUG] Rewind the curl request body with a seek callback The curl client installs CURLOPT_READFUNCTION but no CURLOPT_SEEKFUNCTION. When libcurl restarts an upload it already began, which happens when a connection it reused is closed before the response arrives, it has no way to rewind the body and fails the transfer with CURLE_SEND_FAIL_REWIND. IsRetryable() then declines to retry, because it also requires last_curl_result_ == CURLE_OK, so the export batch is dropped. The request body is a fully buffered span owned by the caller, so an absolute seek inside it is a move of the read cursor. Other origins, a negative offset, or an offset past the end are refused with CURL_SEEKFUNC_CANTSEEK rather than approximated, so libcurl fails cleanly instead of resuming from the wrong place. Fixes #4549 --- CHANGELOG.md | 5 ++ .../http/client/curl/http_operation_curl.h | 14 ++++++ .../http/client/curl/http_operation_curl.cc | 35 ++++++++++++++ ext/test/http/curl_http_test.cc | 46 +++++++++++++++++++ 4 files changed, 100 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8fdaf94d4..b4198ee96f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,11 @@ Increment the: ## [Unreleased] +* [BUG] Install a curl seek callback so an OTLP/HTTP export body can be rewound + when libcurl restarts an upload, instead of failing with + `CURLE_SEND_FAIL_REWIND` and dropping the batch + ([#4549](https://github.com/open-telemetry/opentelemetry-cpp/issues/4549)) + * [DOC] Fix and clarify the `StartSpanOptions` documentation [#4526](https://github.com/open-telemetry/opentelemetry-cpp/pull/4526) diff --git a/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h b/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h index 5328847767..dc0dd37924 100644 --- a/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h +++ b/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h @@ -115,6 +115,20 @@ class HttpOperation static size_t ReadMemoryCallback(char *buffer, size_t size, size_t nitems, void *userp); + /** + * Reposition the request body for libcurl. + * + * libcurl calls this when it has to restart an upload it already began, for example after a + * connection it had reused was closed before the response arrived. Without it libcurl has no way + * to rewind the body and fails the transfer with CURLE_SEND_FAIL_REWIND. + * + * @param userp The HttpOperation, set through CURLOPT_SEEKDATA + * @param offset Byte offset to seek to, interpreted relative to origin + * @param origin One of SEEK_SET, SEEK_CUR or SEEK_END + * @return CURL_SEEKFUNC_OK on success, CURL_SEEKFUNC_CANTSEEK to tell libcurl to find another way + */ + static int SeekCallback(void *userp, curl_off_t offset, int origin); + static int CurlLoggerCallback(const CURL * /* handle */, curl_infotype type, const char *data, diff --git a/ext/src/http/client/curl/http_operation_curl.cc b/ext/src/http/client/curl/http_operation_curl.cc index d32f2736f3..8d399d5f2a 100644 --- a/ext/src/http/client/curl/http_operation_curl.cc +++ b/ext/src/http/client/curl/http_operation_curl.cc @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -334,6 +335,27 @@ size_t HttpOperation::ReadMemoryCallback(char *buffer, size_t size, size_t nitem return nwrite; } +int HttpOperation::SeekCallback(void *userp, curl_off_t offset, int origin) +{ + HttpOperation *self = reinterpret_cast(userp); + if (nullptr == self) + { + return CURL_SEEKFUNC_CANTSEEK; + } + + // The body is a fully buffered span owned by the caller, so an absolute seek inside it is just a + // move of the read cursor. Anything else is refused rather than approximated, because reporting + // success without repositioning would resume the upload from the wrong offset and send a + // truncated or misaligned body. + if (origin != SEEK_SET || offset < 0 || static_cast(offset) > self->request_body_.size()) + { + return CURL_SEEKFUNC_CANTSEEK; + } + + self->request_nwrite_ = static_cast(offset); + return CURL_SEEKFUNC_OK; +} + #if LIBCURL_VERSION_NUM >= 0x075000 int HttpOperation::PreRequestCallback(void *clientp, char *, char *, int, int) { @@ -1336,6 +1358,19 @@ CURLcode HttpOperation::Setup() { return rc; } + + rc = SetCurlPtrOption(CURLOPT_SEEKFUNCTION, + reinterpret_cast(&HttpOperation::SeekCallback)); + if (rc != CURLE_OK) + { + return rc; + } + + rc = SetCurlPtrOption(CURLOPT_SEEKDATA, this); + if (rc != CURLE_OK) + { + return rc; + } } else if (method_ == opentelemetry::ext::http::client::Method::Get) { diff --git a/ext/test/http/curl_http_test.cc b/ext/test/http/curl_http_test.cc index 35cff8e24e..f1376c9c85 100644 --- a/ext/test/http/curl_http_test.cc +++ b/ext/test/http/curl_http_test.cc @@ -13,6 +13,7 @@ # include #endif // ENABLE_OTLP_COMPRESSION_PREVIEW +#include #include #include #include @@ -405,6 +406,51 @@ TEST_F(BasicCurlHttpTests, SendPostRequest) session_manager->FinishAllSessions(); } +// The request body is uploaded through CURLOPT_READFUNCTION, and CURLOPT_SEEKFUNCTION is +// registered alongside it so libcurl can rewind the body when it restarts an upload. Send a body +// large enough to span several read callbacks and check it arrives whole, so a mistake in either +// option shows up as a corrupted or short upload rather than silently. +TEST_F(BasicCurlHttpTests, SendPostRequestWithMultiChunkBody) +{ + received_requests_.clear(); + auto session_manager = std::make_shared()->Create(); + EXPECT_TRUE(session_manager != nullptr); + + auto session = session_manager->CreateSession("http://127.0.0.1:19000"); + auto request = session->CreateRequest(); + request->SetUri("post/"); + request->SetMethod(http_client::Method::Post); + + // Not a round number, so an off-by-one in the read cursor cannot land on a chunk boundary. + constexpr size_t kBodySize = 257u * 1024u + 7u; + http_client::Body body(kBodySize); + for (size_t i = 0; i < kBodySize; ++i) + { + body[i] = static_cast('a' + (i % 26)); + } + const http_client::Body expected = body; + + request->SetBody(body); + request->AddHeader("Content-Type", "application/octet-stream"); + auto handler = std::make_shared(); + session->SendRequest(handler); + ASSERT_TRUE(waitForRequests(30, 1)); + session->FinishSession(); + ASSERT_TRUE(handler->is_called_.load(std::memory_order_acquire)); + ASSERT_TRUE(handler->got_response_.load(std::memory_order_acquire)); + + { + std::unique_lock lk(mtx_requests); + ASSERT_EQ(received_requests_.size(), 1u); + const auto &received = received_requests_[0].content; + ASSERT_EQ(received.size(), expected.size()); + EXPECT_TRUE(std::equal(expected.begin(), expected.end(), received.begin())); + } + + session_manager->CancelAllSessions(); + session_manager->FinishAllSessions(); +} + TEST_F(BasicCurlHttpTests, RequestTimeout) { received_requests_.clear(); From 5a1dad05545deccdbd6cfb2ff3e8e7505f383d73 Mon Sep 17 00:00:00 2001 From: Mateen Anjum Date: Fri, 11 Sep 2026 02:26:04 -0400 Subject: [PATCH 2/4] Unit test the seek callback through a test peer Codecov flagged the callback body as uncovered, since no test invoked it. Expose it through HttpOperationTestPeer, following HttpClientTestPeer in the same header and ReservoirCellTestPeer in the metrics SDK, and cover the reposition, the in-range boundary at exactly the body size, a negative offset, an offset past the end, SEEK_CUR, SEEK_END and a null user pointer. The refused cases assert the read cursor is left where it was, so a future change cannot move it and then report that it could not seek. Co-authored-by: Mats Taraldsvik <1156416+meastp@users.noreply.github.com> --- .../http/client/curl/http_operation_curl.h | 1 + ext/test/http/curl_http_test.cc | 63 +++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h b/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h index dc0dd37924..ce62d9f1e7 100644 --- a/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h +++ b/ext/include/opentelemetry/ext/http/client/curl/http_operation_curl.h @@ -385,6 +385,7 @@ class HttpOperation std::future result_future; }; friend class HttpOperationAccessor; + friend class HttpOperationTestPeer; std::unique_ptr async_data_; }; } // namespace curl diff --git a/ext/test/http/curl_http_test.cc b/ext/test/http/curl_http_test.cc index 907afe0a4a..c0fda643d6 100644 --- a/ext/test/http/curl_http_test.cc +++ b/ext/test/http/curl_http_test.cc @@ -58,6 +58,27 @@ class HttpClientTestPeer public: static void ResetMultiHandle(HttpClient &client) { client.resetMultiHandle(); } }; + +class HttpOperationTestPeer +{ +public: + static int Seek(HttpOperation &operation, curl_off_t offset, int origin) + { + return HttpOperation::SeekCallback(&operation, offset, origin); + } + + static int SeekNullUserData(curl_off_t offset, int origin) + { + return HttpOperation::SeekCallback(nullptr, offset, origin); + } + + static size_t ReadCursor(const HttpOperation &operation) { return operation.request_nwrite_; } + + static void SetReadCursor(HttpOperation &operation, size_t value) + { + operation.request_nwrite_ = value; + } +}; } // namespace curl } // namespace client } // namespace http @@ -451,6 +472,48 @@ TEST_F(BasicCurlHttpTests, SendPostRequestWithMultiChunkBody) session_manager->FinishAllSessions(); } +// libcurl calls the seek callback when it has to restart an upload it already began. The body is a +// fully buffered span, so an absolute seek inside it repositions the read cursor, and anything the +// callback cannot honour is refused so libcurl fails rather than resuming from the wrong offset. +TEST_F(BasicCurlHttpTests, SeekCallbackRepositionsTheRequestBody) +{ + CustomEventHandler handler; + http_client::HttpSslOptions no_ssl; + http_client::Headers headers; + const char *payload = "0123456789"; + http_client::Body body = {payload, payload + std::strlen(payload)}; + + curl::HttpOperation operation(http_client::Method::Post, "http://127.0.0.1:19000/post/", no_ssl, + &handler, headers, body, http_client::Compression::kNone, false, + curl::kDefaultHttpConnTimeout); + + using Peer = curl::HttpOperationTestPeer; + + // An absolute seek inside the body moves the cursor. + Peer::SetReadCursor(operation, 10); + EXPECT_EQ(CURL_SEEKFUNC_OK, Peer::Seek(operation, 4, SEEK_SET)); + EXPECT_EQ(4u, Peer::ReadCursor(operation)); + + // Rewinding to the start is the case libcurl actually asks for. + EXPECT_EQ(CURL_SEEKFUNC_OK, Peer::Seek(operation, 0, SEEK_SET)); + EXPECT_EQ(0u, Peer::ReadCursor(operation)); + + // Seeking to exactly the end is in range and leaves nothing left to send. + EXPECT_EQ(CURL_SEEKFUNC_OK, Peer::Seek(operation, 10, SEEK_SET)); + EXPECT_EQ(10u, Peer::ReadCursor(operation)); + + // Everything below is refused, and must leave the cursor where it was. + Peer::SetReadCursor(operation, 3); + + EXPECT_EQ(CURL_SEEKFUNC_CANTSEEK, Peer::Seek(operation, 11, SEEK_SET)); + EXPECT_EQ(CURL_SEEKFUNC_CANTSEEK, Peer::Seek(operation, -1, SEEK_SET)); + EXPECT_EQ(CURL_SEEKFUNC_CANTSEEK, Peer::Seek(operation, 0, SEEK_CUR)); + EXPECT_EQ(CURL_SEEKFUNC_CANTSEEK, Peer::Seek(operation, 0, SEEK_END)); + EXPECT_EQ(3u, Peer::ReadCursor(operation)); + + EXPECT_EQ(CURL_SEEKFUNC_CANTSEEK, Peer::SeekNullUserData(0, SEEK_SET)); +} + TEST_F(BasicCurlHttpTests, RequestTimeout) { received_requests_.clear(); From 4a26843c258c1dbd9dd4a898c6c03df2ac46e4fe Mon Sep 17 00:00:00 2001 From: Mateen Anjum Date: Fri, 11 Sep 2026 10:28:35 -0400 Subject: [PATCH 3/4] Include curl.h and cstdio directly in the curl test iwyu failed on three configs. The new seek test uses curl_off_t and CURL_SEEKFUNC_* outside the ENABLE_OTLP_RETRY_PREVIEW guard, where curl/curl.h was only included inside it, and it uses SEEK_SET with nothing providing it. Move curl/curl.h out of the guard and add cstdio, matching http_operation_curl.cc. Verified both arms build and pass, 28 tests with OTELCPP_WITH_OTLP_RETRY_PREVIEW=ON and 25 with it OFF. --- ext/test/http/curl_http_test.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/test/http/curl_http_test.cc b/ext/test/http/curl_http_test.cc index c0fda643d6..2448663364 100644 --- a/ext/test/http/curl_http_test.cc +++ b/ext/test/http/curl_http_test.cc @@ -1,11 +1,11 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +#include #include #include "gtest/gtest.h" #ifdef ENABLE_OTLP_RETRY_PREVIEW -# include # include "gmock/gmock.h" #endif // ENABLE_OTLP_RETRY_PREVIEW @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include From a55a5996cde8f761755dcfd9eac259a45ca86c1b Mon Sep 17 00:00:00 2001 From: Mateen Anjum Date: Fri, 11 Sep 2026 10:32:01 -0400 Subject: [PATCH 4/4] Tighten the test comments One sentence was left dangling, and the same rationale was restated in three places. State it once on the declaration and let the tests say what they cover. --- ext/test/http/curl_http_test.cc | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/ext/test/http/curl_http_test.cc b/ext/test/http/curl_http_test.cc index 2448663364..0d7d09bd7f 100644 --- a/ext/test/http/curl_http_test.cc +++ b/ext/test/http/curl_http_test.cc @@ -428,10 +428,9 @@ TEST_F(BasicCurlHttpTests, SendPostRequest) session_manager->FinishAllSessions(); } -// The request body is uploaded through CURLOPT_READFUNCTION, and CURLOPT_SEEKFUNCTION is -// registered alongside it so libcurl can rewind the body when it restarts an upload. Send a body -// large enough to span several read callbacks and check it arrives whole, so a mistake in either -// option shows up as a corrupted or short upload rather than silently. +// Send a body large enough to span several read callbacks and check it arrives whole, so a mistake +// in either CURLOPT_READFUNCTION or the seek callback registered beside it shows up as a corrupted +// or short upload. TEST_F(BasicCurlHttpTests, SendPostRequestWithMultiChunkBody) { received_requests_.clear(); @@ -473,9 +472,7 @@ TEST_F(BasicCurlHttpTests, SendPostRequestWithMultiChunkBody) session_manager->FinishAllSessions(); } -// libcurl calls the seek callback when it has to restart an upload it already began. The body is a -// fully buffered span, so an absolute seek inside it repositions the read cursor, and anything the -// callback cannot honour is refused so libcurl fails rather than resuming from the wrong offset. +// Cover both halves of the callback contract, the seeks it honours and the ones it refuses. TEST_F(BasicCurlHttpTests, SeekCallbackRepositionsTheRequestBody) { CustomEventHandler handler; @@ -490,7 +487,6 @@ TEST_F(BasicCurlHttpTests, SeekCallbackRepositionsTheRequestBody) using Peer = curl::HttpOperationTestPeer; - // An absolute seek inside the body moves the cursor. Peer::SetReadCursor(operation, 10); EXPECT_EQ(CURL_SEEKFUNC_OK, Peer::Seek(operation, 4, SEEK_SET)); EXPECT_EQ(4u, Peer::ReadCursor(operation));