diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 265d824e..65001b1b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -199,7 +199,7 @@ jobs: ctest -C ${{ matrix.config.build_type }} # Alpine/musl: vcpkg's default setup downloads glibc-linked tools, and - # Alpine lacks curlpp / INIReader packages -- exercises the pkg-config + + # Alpine lacks httplib / INIReader packages -- exercises the pkg-config + # upstream source fallback path. build-alpine: name: Alpine_Latest_GCC @@ -209,7 +209,7 @@ jobs: - name: Install dependencies run: | apk add --no-cache build-base cmake git ninja pkgconf \ - curl-dev inih-dev nlohmann-json openssl-dev pugixml-dev zlib-dev + inih-dev nlohmann-json openssl-dev pugixml-dev zlib-dev - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 with: persist-credentials: false diff --git a/CLAUDE.md b/CLAUDE.md index e8d123b2..1e7e0529 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -62,7 +62,7 @@ Examples are built when `MINIO_CPP_TEST=ON`. Run individual examples: - **`args.h`** - Argument structs for each S3 operation (e.g., `PutObjectArgs`, `GetObjectArgs`) - **`response.h`** - Response types returned by operations - **`request.h`** - HTTP request construction -- **`http.h`** - HTTP client abstraction using curlpp +- **`http.h`** - HTTP client abstraction using httplib ### Authentication @@ -85,7 +85,7 @@ RDMA and the caller falls back to HTTP. ## Dependencies (vcpkg) -- curlpp - HTTP client +- cpp-httplib - HTTP client - inih - INI file parsing for config - nlohmann-json - JSON handling - openssl - TLS/crypto diff --git a/CMakeLists.txt b/CMakeLists.txt index 9aed78b9..445c925e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -103,6 +103,13 @@ if (WIN32) list(APPEND MINIO_CPP_LIBS wsock32) list(APPEND MINIO_CPP_LIBS ws2_32) endif() +if (APPLE) + # httplib loads macOS system certificates from the Keychain + # (SecTrustSettingsCopyCertificates); the vcpkg httplib target does not + # link the Security framework, so add it here. + list(APPEND MINIO_CPP_LIBS + "-framework CFNetwork" "-framework CoreFoundation" "-framework Security") +endif() # Minio C++ Library # ----------------- @@ -172,6 +179,9 @@ target_include_directories(miniocpp PUBLIC $ ) target_link_libraries(miniocpp PUBLIC ${MINIO_CPP_LIBS}) +# httplib's class layout depends on CPPHTTPLIB_OPENSSL_SUPPORT; define it once +# target-wide so every translation unit compiles httplib.h identically. +target_compile_definitions(miniocpp PRIVATE CPPHTTPLIB_OPENSSL_SUPPORT) if (MINIO_CPP_ENABLE_RDMA) target_compile_definitions(miniocpp PUBLIC MINIO_CPP_RDMA) endif() @@ -315,7 +325,7 @@ install(TARGETS miniocpp INCLUDES DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}") # Source-built deps must be in an export set for install(EXPORT) to resolve -# them (curlpp exports itself; miniocpp_inih does not). +# them (miniocpp_inih does not export itself). if (MINIO_CPP_DEPS_EXPORT_TARGETS) install(TARGETS ${MINIO_CPP_DEPS_EXPORT_TARGETS} EXPORT miniocpp-targets diff --git a/README.md b/README.md index d53e16c3..7f6c3abb 100644 --- a/README.md +++ b/README.md @@ -135,14 +135,14 @@ $ ./configure.sh -DMINIO_CPP_TEST=ON `vcpkg` can run on Alpine, but its default setup downloads glibc-linked tools such as CMake, which do not run on musl (setting `VCPKG_FORCE_SYSTEM_BINARIES` forces use of the apk-installed tools instead). Alpine also has no packages -for `curlpp` or the C++ `INIReader`. The dependency resolver +for `cpp-httplib` or the C++ `INIReader`. The dependency resolver (`cmake/miniocpp-deps.cmake`) therefore falls back to `pkg-config` and then to fetching those two libraries from source, so a plain system-package build works: ```bash $ apk add build-base cmake git ninja pkgconf \ - curl-dev inih-dev nlohmann-json openssl-dev pugixml-dev zlib-dev + inih-dev nlohmann-json openssl-dev pugixml-dev zlib-dev $ cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DMINIO_CPP_TEST=ON $ cmake --build build ``` diff --git a/cmake/miniocpp-deps.cmake b/cmake/miniocpp-deps.cmake index a0a336e2..385d6d1b 100644 --- a/cmake/miniocpp-deps.cmake +++ b/cmake/miniocpp-deps.cmake @@ -4,7 +4,7 @@ # Resolution order per dependency: vcpkg CONFIG package -> pkg-config -> # upstream source. The source branch is for distros where vcpkg is # impractical (its default setup downloads glibc-linked tools that do not run -# on musl) and no curlpp / C++ INIReader packages exist. It clones at +# on musl) and no C++ INIReader packages exist. It clones at # configure time with plain git, so it works on the CMake 3.13.4 floor (no # FetchContent); vcpkg builds never reach it. # @@ -20,44 +20,62 @@ find_package(OpenSSL REQUIRED) find_package(ZLIB REQUIRED) find_package(nlohmann_json CONFIG REQUIRED) -# curlpp -- pinned master commit, no patches needed: CURLOPT_CLOSEPOLICY is -# gone upstream (dropped in curl 8.10) and the build is target-based and -# self-exporting. Static target keeps BUILD_SHARED_LIBS=OFF builds working. -find_package(unofficial-curlpp CONFIG QUIET) -if (unofficial-curlpp_FOUND) - set(MINIO_CPP_CURLPP_TARGET unofficial::curlpp::curlpp) +# cpp-httplib -- header-only; vcpkg -> pkg-config -> upstream source (pinned +# tag). The code needs the progress overloads and set_max_timeout added in +# 0.19; require 0.51 (the current vcpkg port) so ancient distro packages +# cannot be selected. +find_package(httplib CONFIG QUIET) +if (httplib_FOUND AND DEFINED httplib_VERSION AND + httplib_VERSION VERSION_LESS "0.51") + message(STATUS "cpp-httplib ${httplib_VERSION} is too old; falling back") + set(httplib_FOUND FALSE) +endif() +if (httplib_FOUND) + set(MINIO_CPP_HTTPLIB_TARGET httplib::httplib) else() if (PkgConfig_FOUND) - pkg_check_modules(MINIO_CPP_CURLPP QUIET IMPORTED_TARGET curlpp) + pkg_check_modules(MINIO_CPP_HTTPLIB QUIET IMPORTED_TARGET cpp-httplib) + endif() + if (MINIO_CPP_HTTPLIB_FOUND AND DEFINED MINIO_CPP_HTTPLIB_VERSION AND + MINIO_CPP_HTTPLIB_VERSION VERSION_LESS "0.51") + message(STATUS "cpp-httplib ${MINIO_CPP_HTTPLIB_VERSION} is too old") + set(MINIO_CPP_HTTPLIB_FOUND FALSE) endif() - if (MINIO_CPP_CURLPP_FOUND) - set(MINIO_CPP_CURLPP_TARGET PkgConfig::MINIO_CPP_CURLPP) + if (MINIO_CPP_HTTPLIB_FOUND) + set(MINIO_CPP_HTTPLIB_TARGET PkgConfig::MINIO_CPP_HTTPLIB) else() - message(STATUS "curlpp: no package found, building from source") - set(MINIO_CPP_CURLPP_SRC "${CMAKE_CURRENT_BINARY_DIR}/_deps/curlpp-src") - if (NOT EXISTS "${MINIO_CPP_CURLPP_SRC}/CMakeLists.txt") + message(STATUS "cpp-httplib: no usable package found, building from source") + set(MINIO_CPP_HTTPLIB_SRC "${CMAKE_CURRENT_BINARY_DIR}/_deps/cpp-httplib-src") + set(MINIO_CPP_HTTPLIB_PINNED_TAG "v0.53.1") + if (NOT EXISTS "${MINIO_CPP_HTTPLIB_SRC}/CMakeLists.txt") execute_process(COMMAND git clone --quiet - https://github.com/jpbarrette/curlpp.git - "${MINIO_CPP_CURLPP_SRC}" - RESULT_VARIABLE _curlpp_clone) - if (NOT _curlpp_clone STREQUAL "0") - message(FATAL_ERROR "curlpp: git clone failed") + https://github.com/yhirose/cpp-httplib.git + "${MINIO_CPP_HTTPLIB_SRC}" + RESULT_VARIABLE _httplib_clone) + if (NOT _httplib_clone STREQUAL "0") + message(FATAL_ERROR "cpp-httplib: git clone failed") endif() endif() - # Also reset a cached checkout to the pinned commit, not just a fresh - # clone. + # Fetch tags so a cached clone can resolve the pinned tag. + execute_process(COMMAND git fetch --quiet --tags origin + WORKING_DIRECTORY "${MINIO_CPP_HTTPLIB_SRC}" + RESULT_VARIABLE _httplib_fetch) + if (NOT _httplib_fetch STREQUAL "0") + message(FATAL_ERROR "cpp-httplib: git fetch failed") + endif() execute_process(COMMAND git checkout --quiet - ec1b66e699557cd9d608d322c013a1ebda16bd08 - WORKING_DIRECTORY "${MINIO_CPP_CURLPP_SRC}" - RESULT_VARIABLE _curlpp_checkout) - if (NOT _curlpp_checkout STREQUAL "0") - message(FATAL_ERROR "curlpp: git checkout of pinned commit failed") + ${MINIO_CPP_HTTPLIB_PINNED_TAG} + WORKING_DIRECTORY "${MINIO_CPP_HTTPLIB_SRC}" + RESULT_VARIABLE _httplib_checkout) + if (NOT _httplib_checkout STREQUAL "0") + message(FATAL_ERROR "cpp-httplib: git checkout of pinned tag failed") endif() - set(CURLPP_BUILD_SHARED_LIBS OFF CACHE BOOL "Build curlpp shared library" FORCE) - add_subdirectory("${MINIO_CPP_CURLPP_SRC}" - "${CMAKE_CURRENT_BINARY_DIR}/_deps/curlpp-build") - set(MINIO_CPP_CURLPP_TARGET curlpp_static) - set_target_properties(curlpp_static PROPERTIES POSITION_INDEPENDENT_CODE ON) + # cpp-httplib is header-only; enable its CMake install target so that + # find_package(httplib) works for downstream consumers after install. + set(HTTPLIB_BUILD_SHARED_LIBS OFF CACHE BOOL "" FORCE) + add_subdirectory("${MINIO_CPP_HTTPLIB_SRC}" + "${CMAKE_CURRENT_BINARY_DIR}/_deps/cpp-httplib-build") + set(MINIO_CPP_HTTPLIB_TARGET httplib::httplib) endif() endif() @@ -126,7 +144,7 @@ else() endif() set(MINIO_CPP_DEPS_LINK_LIBS - ${MINIO_CPP_CURLPP_TARGET} + ${MINIO_CPP_HTTPLIB_TARGET} ${MINIO_CPP_INIH_TARGET} nlohmann_json::nlohmann_json ${MINIO_CPP_PUGIXML_TARGET} diff --git a/examples/GetBucketTags.cc b/examples/GetBucketTags.cc index c73335be..7d6103ab 100644 --- a/examples/GetBucketTags.cc +++ b/examples/GetBucketTags.cc @@ -39,7 +39,8 @@ int main() { if (resp) { std::cout << "Bucket tags: " << std::endl; for (auto& [key, value] : resp->tags) { - std::cout << "Key: " << key << ", " << "Value: " << value << std::endl; + std::cout << "Key: " << key << ", " + << "Value: " << value << std::endl; } } else { std::cout << "unable to get bucket tags; " << resp.error().String() diff --git a/examples/GetObjectTags.cc b/examples/GetObjectTags.cc index 14556adb..c04dcd4b 100644 --- a/examples/GetObjectTags.cc +++ b/examples/GetObjectTags.cc @@ -40,7 +40,8 @@ int main() { if (resp) { std::cout << "Object tags: " << std::endl; for (auto& [key, value] : resp->tags) { - std::cout << "Key: " << key << ", " << "Value: " << value << std::endl; + std::cout << "Key: " << key << ", " + << "Value: " << value << std::endl; } } else { std::cout << "unable to get object tags; " << resp.error().String() diff --git a/include/miniocpp/client.h b/include/miniocpp/client.h index e440894b..592f9b4e 100644 --- a/include/miniocpp/client.h +++ b/include/miniocpp/client.h @@ -30,6 +30,11 @@ #include "response.h" #include "result.h" +// windows.h maps GetObject to GetObjectA; keep the member function names. +#ifdef _WIN32 +#undef GetObject +#endif + namespace minio::s3 { class Client; diff --git a/include/miniocpp/http.h b/include/miniocpp/http.h index c884087d..0c4f22f6 100644 --- a/include/miniocpp/http.h +++ b/include/miniocpp/http.h @@ -18,8 +18,6 @@ #ifndef MINIO_CPP_HTTP_H_INCLUDED #define MINIO_CPP_HTTP_H_INCLUDED -#include -#include #include #include #include @@ -77,18 +75,15 @@ using ProgressFunction = std::function; struct Response; struct DataFunctionArgs { - curlpp::Easy* handle = nullptr; Response* response = nullptr; std::string datachunk; void* userdata = nullptr; DataFunctionArgs() = default; - DataFunctionArgs(curlpp::Easy* handle, Response* response, void* userdata) - : handle(handle), response(response), userdata(userdata) {} - DataFunctionArgs(curlpp::Easy* handle, Response* response, - std::string datachunk, void* userdata) - : handle(handle), - response(response), + DataFunctionArgs(Response* response, void* userdata) + : response(response), userdata(userdata) {} + DataFunctionArgs(Response* response, std::string datachunk, void* userdata) + : response(response), datachunk(std::move(datachunk)), userdata(userdata) {} @@ -164,24 +159,11 @@ struct Response { Response() = default; ~Response() = default; - size_t ResponseCallback(curlpp::Multi* const requests, - curlpp::Easy* const request, const char* const buffer, - size_t size, size_t length); - explicit operator bool() const { return error.empty() && status_code >= 200 && status_code <= 299; } error::Error Error() const; - - private: - std::string response_; - bool continue100_ = false; - bool status_code_read_ = false; - bool headers_read_ = false; - - error::Error ReadStatusCode(); - error::Error ReadHeaders(); }; // struct Response } // namespace minio::http diff --git a/include/miniocpp/utils.h b/include/miniocpp/utils.h index f224e5a4..221c31e2 100644 --- a/include/miniocpp/utils.h +++ b/include/miniocpp/utils.h @@ -94,6 +94,13 @@ std::string Join(const std::list& values, std::string Join(const std::vector& values, const std::string& delimiter); +// AWS SigV4 percent-encoding: RFC 3986 unreserved characters are kept, +// everything else is percent-encoded with uppercase hex. +std::string UriEncode(const std::string& value); + +// UriDecode reverses percent-encoding; '+' is not treated as space. +std::string UriDecode(const std::string& value); + // EncodePath does URL encoding of path. It also normalizes multiple slashes. std::string EncodePath(const std::string& path); diff --git a/src/args.cc b/src/args.cc index e4a89eb8..7ced7df5 100644 --- a/src/args.cc +++ b/src/args.cc @@ -17,7 +17,6 @@ #include "miniocpp/args.h" -#include #include #include #include @@ -62,7 +61,7 @@ utils::Multimap ObjectWriteArgs::Headers() const { std::string tagging; for (auto& [key, value] : tags) { - std::string tag = curlpp::escape(key) + "=" + curlpp::escape(value); + std::string tag = utils::UriEncode(key) + "=" + utils::UriEncode(value); if (!tagging.empty()) { tagging += "&"; } @@ -126,9 +125,9 @@ utils::Multimap ObjectConditionalReadArgs::Headers() const { utils::Multimap ObjectConditionalReadArgs::CopyHeaders() const { utils::Multimap result_headers; - std::string copy_source = curlpp::escape("/" + bucket + "/" + object); + std::string copy_source = utils::UriEncode("/" + bucket + "/" + object); if (!version_id.empty()) { - copy_source += "?versionId=" + curlpp::escape(version_id); + copy_source += "?versionId=" + utils::UriEncode(version_id); } result_headers.Add("x-amz-copy-source", copy_source); diff --git a/src/baseclient.cc b/src/baseclient.cc index ad2247cf..55deb243 100644 --- a/src/baseclient.cc +++ b/src/baseclient.cc @@ -378,8 +378,11 @@ Result BaseClient::CompleteMultipartUpload( std::stringstream ss; ss << ""; for (auto& part : args.parts) { - ss << "" << "" << part.number << "" - << "" << "\"" << part.etag << "\"" << ""; + ss << "" + << "" << part.number << "" + << "" + << "\"" << part.etag << "\"" + << ""; if (!part.checksum_crc64nvme.empty()) { ss << "" << part.checksum_crc64nvme << ""; @@ -1416,8 +1419,9 @@ Result BaseClient::MakeBucket(MakeBucketArgs args) { std::string body; if (region != "us-east-1") { std::stringstream ss; - ss << "" << "" << region - << "" << ""; + ss << "" + << "" << region << "" + << ""; body = ss.str(); req.body = body; } @@ -1804,8 +1808,10 @@ Result BaseClient::SetBucketTags( if (!args.tags.empty()) { ss << ""; for (auto& [key, value] : args.tags) { - ss << "" << "" << key << "" << "" << value - << "" << ""; + ss << "" + << "" << key << "" + << "" << value << "" + << ""; } ss << ""; } @@ -1934,9 +1940,10 @@ Result BaseClient::SetObjectRetention( } std::stringstream ss; - ss << "" << "" << RetentionModeToString(args.retention_mode) - << "" << "" - << args.retain_until_date.ToISO8601UTC() << "" + ss << "" + << "" << RetentionModeToString(args.retention_mode) << "" + << "" << args.retain_until_date.ToISO8601UTC() + << "" << ""; std::string body = ss.str(); @@ -1978,8 +1985,10 @@ Result BaseClient::SetObjectTags( if (!args.tags.empty()) { ss << ""; for (auto& [key, value] : args.tags) { - ss << "" << "" << key << "" << "" << value - << "" << ""; + ss << "" + << "" << key << "" + << "" << value << "" + << ""; } ss << ""; } diff --git a/src/client.cc b/src/client.cc index b8474f21..cb5f9851 100644 --- a/src/client.cc +++ b/src/client.cc @@ -24,7 +24,14 @@ #include #endif -#include +#include + +// windows.h (via httplib) maps GetObject to GetObjectA; undo it so the +// Client::GetObject members keep their real names. +#ifdef _WIN32 +#undef GetObject +#endif + #include #include #include @@ -709,10 +716,8 @@ Result Client::GetObject(GetObjectArgs args) { {}, std::nullopt, {}, base_url_, region}; - // RAII, matching the multipart paths below. rdmaGetWithRetry signs and - // sends an HTTP request, and curlpp throws, so a manual Deregister after - // the call is skipped on that path and the buffer stays pinned for the - // life of the process. + // Keep the buffer registered through rdmaGetWithRetry. The RAII guard + // deregisters it when this block exits, including on early returns. ScopedRDMARegistration reg(&rdma_client, args.buf); ssize_t ret = @@ -1156,7 +1161,7 @@ Result Client::DownloadObject(DownloadObjectArgs args) { } std::string temp_filename = - args.filename + "." + curlpp::escape(etag) + ".part.minio"; + args.filename + "." + utils::UriEncode(etag) + ".part.minio"; std::ofstream fout(temp_filename, std::ios::trunc | std::ios::out | std::ios::binary); if (!fout.is_open()) { diff --git a/src/http.cc b/src/http.cc index efea6fd3..d5f2698a 100644 --- a/src/http.cc +++ b/src/http.cc @@ -17,28 +17,24 @@ #include "miniocpp/http.h" -#include +// cpp-httplib's OpenSSL backend is enabled once, target-wide, via +// target_compile_definitions(miniocpp PRIVATE CPPHTTPLIB_OPENSSL_SUPPORT): the +// header-only library's class layout depends on that macro, so every +// translation unit must agree on it. +#include #include -#include -#include -#include -#include -#include -#include -#include -#include #include #include #include #include #include -#include +#include +#include #include #include #include #include -#include #include #include "miniocpp/error.h" @@ -59,68 +55,10 @@ namespace { // Abort a transfer that makes no progress for this long. Guards against a // connection that drops mid-transfer without a clean close (TCP never RSTs), -// which would otherwise keep the request alive indefinitely. +// which would otherwise keep the request alive indefinitely. cpp-httplib +// enforces it as a read/write timeout, since it has no low-speed limit. constexpr long kStallTimeoutSecs = 60; -// curl_global_init() is documented as not thread-safe and is expensive -// (OpenSSL init etc). Run it exactly once per process via a function-local -// static (Meyers singleton; C++11 [stmt.dcl]/4 guarantees thread-safe -// initialization), instead of paying the cost — and the race — on every -// request via a stack-local curlpp::Cleanup. -void EnsureGlobalCurlInit() { - static const curlpp::Cleanup kCleanup; - (void)kCleanup; -} - -// Connection, DNS and TLS-session caches, kept per thread. -// -// These were once a single process-wide CURLSH with per-slot mutexes, which -// libcurl's own documentation warns against: a shared connection cache is not -// safe to use from several threads at once, and this crashed reliably under -// concurrent PUTs -- a wild pointer read inside curl_multi_perform, with the -// mutexes held exactly as documented. Bisecting the slots on libcurl 8.5: -// -// none clean CONNECT only crashes -// DNS only clean CONNECT + SSL_SESSION crashes -// DNS + SSL_SESSION crashes -// -// So the sharing itself is the problem, not one slot. Giving each thread its -// own share keeps what the share was for -- a connection and TLS session -// surviving past one Easy handle, so a signed S3 call does not pay a fresh -// handshake every time -- while removing the cross-thread access entirely. -// Nothing is shared between threads, so no lock callbacks are needed. -// -// The handle is destroyed when its thread exits. Every Easy that used it is -// stack-local to Request::execute() and long gone by then. -class ThreadCurlShare { - public: - ThreadCurlShare() : share_(curl_share_init()) { - if (share_ == nullptr) { - std::cerr << "curl_share_init failed" << std::endl; - std::terminate(); - } - curl_share_setopt(share_, CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT); - curl_share_setopt(share_, CURLSHOPT_SHARE, CURL_LOCK_DATA_DNS); - curl_share_setopt(share_, CURLSHOPT_SHARE, CURL_LOCK_DATA_SSL_SESSION); - } - ~ThreadCurlShare() { - if (share_ != nullptr) curl_share_cleanup(share_); - } - ThreadCurlShare(const ThreadCurlShare&) = delete; - ThreadCurlShare& operator=(const ThreadCurlShare&) = delete; - - CURLSH* get() const { return share_; } - - private: - CURLSH* share_; -}; - -CURLSH* CurlShare() { - EnsureGlobalCurlInit(); - static thread_local ThreadCurlShare share; - return share.get(); -} - } // namespace // MethodToString converts http Method enum to string. @@ -239,65 +177,6 @@ Url Url::Parse(std::string value) { std::move(query_string)); } -error::Error Response::ReadStatusCode() { - size_t pos = response_.find("\r\n"); - if (pos == std::string::npos) { - // Not yet received the first line. - return error::SUCCESS; - } - - std::string line = response_.substr(0, pos); - response_.erase(0, pos + 2); - - if (continue100_) { - if (!line.empty()) { - // After '100 Continue', next line must be empty new line. - return error::Error("invalid HTTP response"); - } - - continue100_ = false; - - pos = response_.find("\r\n"); - if (pos == std::string::npos) { - // Not yet received the first line after '100 Continue'. - return error::SUCCESS; - } - - line = response_.substr(0, pos); - response_.erase(0, pos + 2); - } - - // Skip HTTP/1.x. - pos = line.find(" "); - if (pos == std::string::npos) { - // First token must be HTTP/1.x - return error::Error("invalid HTTP response"); - } - line = line.substr(pos + 1); - - // Read status code. - pos = line.find(" "); - if (pos == std::string::npos) { - // The line must contain second token. - return error::Error("invalid HTTP response"); - } - std::string code = line.substr(0, pos); - std::string::size_type st; - status_code = std::stoi(code, &st); - if (st == std::string::npos) { - // Code must be a number. - return error::Error("invalid HTTP response code " + code); - } - - if (status_code == 100) { - continue100_ = true; - } else { - status_code_read_ = true; - } - - return error::SUCCESS; -} - error::Error Response::Error() const { if (!error.empty()) return error::Error(error); if (status_code && (status_code < 200 || status_code > 299)) { @@ -307,99 +186,6 @@ error::Error Response::Error() const { return error::SUCCESS; } -error::Error Response::ReadHeaders() { - size_t pos = response_.find("\r\n\r\n"); - if (pos == std::string::npos) { - // Not yet received the headers. - return error::SUCCESS; - } - - headers_read_ = true; - - std::string lines = response_.substr(0, pos); - response_.erase(0, pos + 4); - - auto add_header = [&headers = headers](std::string line) -> error::Error { - size_t pos = line.find(": "); - if (pos != std::string::npos) { - headers.Add(line.substr(0, pos), line.substr(pos + 2)); - return error::SUCCESS; - } - - return error::Error("invalid HTTP header: " + line); - }; - - while ((pos = lines.find("\r\n")) != std::string::npos) { - std::string line = lines.substr(0, pos); - lines.erase(0, pos + 2); - if (error::Error err = add_header(line)) return err; - } - - if (!lines.empty()) { - if (error::Error err = add_header(lines)) return err; - } - - return error::SUCCESS; -} - -size_t Response::ResponseCallback(curlpp::Multi* const requests, - curlpp::Easy* const request, - const char* const buffer, size_t size, - size_t length) { - size_t realsize = size * length; - - // If error occurred previously, just cancel the request. - if (!error.empty()) { - requests->remove(request); - return realsize; - } - - if (!status_code_read_ || !headers_read_) { - response_.append(buffer, length); - } - - if (!status_code_read_) { - if (error::Error err = ReadStatusCode()) { - error = err.String(); - requests->remove(request); - return realsize; - } - - if (!status_code_read_) return realsize; - } - - if (!headers_read_) { - if (error::Error err = ReadHeaders()) { - error = err.String(); - requests->remove(request); - return realsize; - } - - if (!headers_read_ || response_.empty()) return realsize; - - // If data function is set and the request is successful, send data. - if (datafunc != nullptr && status_code >= 200 && status_code <= 299) { - DataFunctionArgs args(request, this, std::string(this->response_), - userdata); - if (!datafunc(args)) requests->remove(request); - } else { - body = response_; - } - - return realsize; - } - - // If data function is set and the request is successful, send data. - if (datafunc != nullptr && status_code >= 200 && status_code <= 299) { - DataFunctionArgs args(request, this, std::string(buffer, length), userdata); - if (!datafunc(args)) requests->remove(request); - } else { - body.append(buffer, length); - } - - return realsize; -} - Request::Request(Method method, Url url) { this->method = method; this->url = url; @@ -410,197 +196,218 @@ Request::Request(Method method, Url url) { } Response Request::execute() { - EnsureGlobalCurlInit(); - curlpp::Easy request; - curlpp::Multi requests; - - // Attach this thread's share so connections, DNS resolutions and TLS - // sessions survive past this Easy handle's lifetime. Per thread rather than - // per process: see CurlShare() for why sharing these across threads - // corrupts libcurl's state. Also enable TCP keep-alive so the kernel keeps - // pooled sockets healthy across idle gaps between S3 calls. curlpp doesn't - // wrap either option, so set via libcurl. - CURL* const raw_handle = request.getHandle(); - curl_easy_setopt(raw_handle, CURLOPT_SHARE, CurlShare()); - curl_easy_setopt(raw_handle, CURLOPT_TCP_KEEPALIVE, 1L); - - // Fail a stalled transfer instead of hanging forever. Skipped when the caller - // set an explicit total timeout (RDMA control plane) — that already bounds - // it. - if (timeout_secs <= 0) { - curl_easy_setopt(raw_handle, CURLOPT_LOW_SPEED_LIMIT, 1L); - curl_easy_setopt(raw_handle, CURLOPT_LOW_SPEED_TIME, kStallTimeoutSecs); - } + Response response; + response.datafunc = datafunc; + response.userdata = userdata; + + // httplib::Client is bound to one endpoint and not thread-safe; build a + // fresh one per request. + std::string endpoint = (url.https ? "https://" : "http://") + url.host; + if (url.port) endpoint += ":" + std::to_string(url.port); - // Request settings. - request.setOpt(new curlpp::options::CustomRequest{MethodToString(method)}); - std::string urlstring = url.String(); - request.setOpt(new curlpp::Options::Url(urlstring)); - if (debug) request.setOpt(new curlpp::Options::Verbose(true)); - if (ignore_cert_check) { - request.setOpt(new curlpp::Options::SslVerifyPeer(false)); - request.setOpt(new curlpp::Options::SslVerifyHost(0L)); + auto client = + std::make_unique(endpoint, cert_file, key_file); + if (!client->is_valid()) { + response.error = "unable to create HTTP client for " + endpoint; + return response; } + httplib::Client& cli = *client; + // httplib's default 5s read/write timeout is too short for S3 transfers; + // use the 60s stall guard unless the caller set an explicit total timeout. + cli.set_keep_alive(true); + cli.set_follow_location(false); + // Paths are pre-encoded by the caller (EncodePath). + cli.set_path_encode(false); + if (connect_timeout_secs > 0) { + cli.set_connection_timeout(connect_timeout_secs, 0); + } + if (timeout_secs > 0) { + // Total transfer deadline, like the old CURLOPT_TIMEOUT. + cli.set_max_timeout(static_cast(timeout_secs) * 1000); + } else { + cli.set_read_timeout(kStallTimeoutSecs, 0); + cli.set_write_timeout(kStallTimeoutSecs, 0); + } + if (debug) { + cli.set_logger( + [](const httplib::Request& req, const httplib::Response& res) { + std::cerr << req.method << " " << req.path << " -> " << res.status + << std::endl; + }); + } + if (!nic_interface.empty()) cli.set_interface(nic_interface); if (url.https) { + // An explicit CA bundle overrides IGNORE_CERT_CHECK, matching the curl + // backend: verification is enabled against the given CA file. if (!ssl_cert_file.empty()) { - request.setOpt(new curlpp::Options::SslVerifyPeer(true)); - request.setOpt(new curlpp::Options::CaInfo(ssl_cert_file)); - } - if (!key_file.empty()) { - request.setOpt(new curlpp::Options::SslKey(key_file)); - } - if (!cert_file.empty()) { - request.setOpt(new curlpp::Options::SslCert(cert_file)); + cli.set_ca_cert_path(ssl_cert_file); + cli.enable_server_certificate_verification(true); + } else { + cli.enable_server_certificate_verification(!ignore_cert_check); } } - if (!nic_interface.empty()) { - request.setOpt(new curlpp::Options::Interface(nic_interface)); - } - if (connect_timeout_secs > 0) { - request.setOpt(new curlpp::Options::ConnectTimeout(connect_timeout_secs)); - } - if (timeout_secs > 0) { - request.setOpt(new curlpp::Options::Timeout(timeout_secs)); + httplib::Headers request_headers; + for (const auto& key : headers.Keys()) { + for (const auto& value : headers.Get(key)) { + request_headers.insert({key, value}); + } } + // httplib sets Host itself when absent and derives Content-Length from the + // body; the SigV4-signed Host must be sent verbatim. An empty Expect + // disables the 100-continue handshake, as the curl backend did. + request_headers.erase("Content-Length"); + request_headers.emplace("Expect", ""); + std::string content_type = headers.GetFront("Content-Type"); + request_headers.erase("Content-Type"); + + std::string path = url.path; + if (path.empty()) { + path = "/"; + } else if (path.front() != '/') { + path = "/" + path; + } + if (!url.query_string.empty()) path += "?" + url.query_string; + + // Track bytes and elapsed time to report average speeds in the final + // progress call, as the curl backend did. + auto start_time = std::chrono::steady_clock::now(); + size_t bytes_downloaded = 0; + size_t bytes_uploaded = 0; + auto report_speed = [&, this]() { + if (progressfunc == nullptr) return; + const double elapsed = std::chrono::duration( + std::chrono::steady_clock::now() - start_time) + .count(); + ProgressFunctionArgs args; + if (elapsed > 0) { + args.download_speed = static_cast(bytes_downloaded) / elapsed; + args.upload_speed = static_cast(bytes_uploaded) / elapsed; + } + args.userdata = progress_userdata; + progressfunc(args); + }; - utils::CharBuffer charbuf((char*)body.data(), body.size()); - std::istream body_stream(&charbuf); + auto download_progress = [this, &bytes_downloaded](size_t current, + size_t total) -> bool { + bytes_downloaded = current; + if (progressfunc == nullptr) return true; + ProgressFunctionArgs args; + args.download_total_bytes = static_cast(total); + args.downloaded_bytes = static_cast(current); + args.userdata = progress_userdata; + return progressfunc(args); + }; + auto upload_progress = [this, &bytes_uploaded](size_t current, + size_t total) -> bool { + bytes_uploaded = current; + if (progressfunc == nullptr) return true; + ProgressFunctionArgs args; + args.upload_total_bytes = static_cast(total); + args.uploaded_bytes = static_cast(current); + args.userdata = progress_userdata; + return progressfunc(args); + }; + + // Stream the response body to the data function; a false return aborts the + // transfer, recorded so a caller-initiated abort is not reported as an + // error below (e.g. ListenBucketNotification stops once it has its records). + // Non-2xx bodies are buffered instead so error payloads reach the caller + // (the GET response handler gates this on the status; the POST overloads + // have no response handler, so Select always streams and surfaces errors + // through the event stream). + bool datafunc_canceled = false; + bool stream_to_datafunc = true; + httplib::ContentReceiver content_receiver = + [this, &response, &datafunc_canceled, &stream_to_datafunc]( + const char* data, size_t length) -> bool { + if (!stream_to_datafunc) { + response.body.append(data, length); + return true; + } + DataFunctionArgs args(&response, std::string(data, length), userdata); + const bool cont = datafunc(args); + if (!cont) datafunc_canceled = true; + return cont; + }; + httplib::Result res; + httplib::ResponseHandler response_handler = + [&response, &stream_to_datafunc](const httplib::Response& res) -> bool { + // Status is known here, before any body is streamed, so a caller- + // initiated cancel still yields a response with the correct status. + response.status_code = res.status; + stream_to_datafunc = res.status >= 200 && res.status <= 299; + return true; + }; switch (method) { - case Method::kDelete: case Method::kGet: + if (datafunc != nullptr) { + res = cli.Get(path, request_headers, response_handler, content_receiver, + download_progress); + } else { + res = cli.Get(path, request_headers, download_progress); + } break; case Method::kHead: - request.setOpt(new curlpp::options::NoBody(true)); + res = cli.Head(path, request_headers); break; - case Method::kPut: case Method::kPost: - if (!headers.Contains("Content-Length")) { - headers.Add("Content-Length", std::to_string(body.size())); + if (datafunc != nullptr) { + // Stream the response body to the data function (e.g. the S3 Select + // event stream); the request body here is small, so a copy is fine. + res = cli.Post(path, request_headers, + std::string(body.data(), body.size()), content_type, + content_receiver, download_progress); + } else { + res = cli.Post(path, request_headers, body.data(), body.size(), + content_type, upload_progress); } - request.setOpt(new curlpp::Options::ReadStream(&body_stream)); - // CURLOPT_INFILESIZE_LARGE (curl_off_t), not CURLOPT_INFILESIZE (long): - // the latter is documented to be capped at 2 GiB and silently truncates - // the upload for larger single-request bodies (e.g. a >4 GiB buffer that - // could not be RDMA-registered and falls back to a single PUT). - request.setOpt(new curlpp::Options::InfileSizeLarge( - static_cast(body.size()))); - request.setOpt(new curlpp::Options::Upload(true)); + break; + case Method::kPut: + res = cli.Put(path, request_headers, body.data(), body.size(), + content_type, upload_progress); + break; + case Method::kDelete: + res = cli.Delete(path, request_headers, download_progress); break; } - std::list headerlist = headers.ToHttpHeaders(); - headerlist.push_back("Expect:"); // Disable 100 continue from server. - request.setOpt(new curlpp::Options::HttpHeader(headerlist)); - - // Response settings. - request.setOpt(new curlpp::options::Header(true)); - - Response response; - response.datafunc = datafunc; - response.userdata = userdata; - - using namespace std::placeholders; - request.setOpt(new curlpp::options::WriteFunction( - std::bind(&Response::ResponseCallback, &response, &requests, &request, _1, - _2, _3))); - - auto progress = - [&progressfunc = progressfunc, &progress_userdata = progress_userdata]( - double dltotal, double dlnow, double ultotal, double ulnow) -> int { - ProgressFunctionArgs args; - args.download_total_bytes = dltotal; - args.downloaded_bytes = dlnow; - args.upload_total_bytes = ultotal; - args.uploaded_bytes = ulnow; - args.userdata = progress_userdata; - if (progressfunc(args)) { - return CURL_PROGRESSFUNC_CONTINUE; - } - return 1; - }; - if (progressfunc != nullptr) { - request.setOpt(new curlpp::options::NoProgress(false)); - request.setOpt(new curlpp::options::ProgressFunction(progress)); - } - - int left = 0; - requests.add(&request); - - // Execute. - while (!requests.perform(&left)) { - } - while (left) { - fd_set fdread{}; - fd_set fdwrite{}; - fd_set fdexcep{}; - int maxfd = -1; - - FD_ZERO(&fdread); - FD_ZERO(&fdwrite); - FD_ZERO(&fdexcep); - - requests.fdset(&fdread, &fdwrite, &fdexcep, &maxfd); - - // Bound the wait so the loop keeps pumping libcurl even when no socket ever - // becomes ready — otherwise a dropped/stalled connection blocks select() - // forever and this (synchronous) call hangs the calling thread. The bounded - // poll lets libcurl enforce its own timeouts (e.g. the low-speed limit set - // above) and abort the dead transfer. - if (maxfd < 0) { - // libcurl has no fd to wait on yet; select() with empty sets errors out - // on Windows, so just poll again shortly. - std::this_thread::sleep_for(std::chrono::milliseconds(100)); - } else { - timeval timeout{}; - timeout.tv_sec = 1; - timeout.tv_usec = 0; - if (select(maxfd + 1, &fdread, &fdwrite, &fdexcep, &timeout) < 0) { -#ifndef _WIN32 - if (errno == EINTR) continue; // interrupted by a signal; retry -#endif - std::cerr << "select() failed; this should not happen" << std::endl; - std::terminate(); - } - } - while (!requests.perform(&left)) { + if (!res) { + // A false return from the data function cancels the transfer; for + // streaming callers that is a normal completion, not an error. GET + // captures the status via the response handler; httplib discards the + // response when a POST receiver cancels, so report success there since + // the caller ended the transfer itself. + if (res.error() == httplib::Error::Canceled && datafunc_canceled) { + if (response.status_code == 0) response.status_code = 200; + report_speed(); + return response; } + response.error = httplib::to_string(res.error()); + report_speed(); + return response; } - // The loop exits once libcurl has no running transfers left. If the transfer - // aborted before delivering a single byte (e.g. the low-speed limit or - // connect timeout fired on a dropped/stalled connection), the write callback - // never ran, so neither status_code nor error was set. Surface a diagnostic - // instead of returning a silently-empty failure. - if (response.error.empty() && response.status_code == 0) { - response.error = - "transfer ended without a response (connection dropped, timed out, or " - "was aborted before any data was received)"; - } - - if (progressfunc != nullptr) { - ProgressFunctionArgs args; - args.userdata = progress_userdata; - curlpp::infos::SpeedUpload::get(request, args.upload_speed); - curlpp::infos::SpeedDownload::get(request, args.download_speed); - progressfunc(args); + response.status_code = res->status; + for (const auto& [key, value] : res->headers) { + response.headers.Add(key, value); } + // With a data function the body is streamed to it; otherwise keep the + // buffered response body (including error payloads for non-2xx statuses). + if (datafunc == nullptr) response.body = res->body; + report_speed(); return response; } Response Request::Execute() { try { return execute(); - } catch (curlpp::LogicError& e) { - Response response; - response.error = std::string("curlpp::LogicError: ") + e.what(); - return response; - } catch (curlpp::RuntimeError& e) { + } catch (const std::exception& e) { Response response; - response.error = std::string("curlpp::RuntimeError: ") + e.what(); + response.error = std::string("HTTP error: ") + e.what(); return response; } } diff --git a/src/response.cc b/src/response.cc index ffdfc64f..87bee0b1 100644 --- a/src/response.cc +++ b/src/response.cc @@ -18,7 +18,6 @@ #include "miniocpp/response.h" #include -#include #include #include #include @@ -168,7 +167,7 @@ Result ListObjectsResponse::ParseXML(std::string_view data, const char* raw, std::string_view& target) -> void { if (encoding_type == "url") { - resp.owned_.emplace_back(curlpp::unescape(raw)); + resp.owned_.emplace_back(utils::UriDecode(raw)); target = resp.owned_.back(); } else { target = raw; @@ -260,7 +259,7 @@ Result ListObjectsResponse::ParseXML(std::string_view data, text = content.node().select_node("Key/text()"); raw = text.node().value(); if (resp.encoding_type == "url") { - resp.owned_.emplace_back(curlpp::unescape(raw)); + resp.owned_.emplace_back(utils::UriDecode(raw)); item.name = resp.owned_.back(); } else { item.name = raw; @@ -317,7 +316,7 @@ Result ListObjectsResponse::ParseXML(std::string_view data, text = common_prefix.node().select_node("Prefix/text()"); raw = text.node().value(); if (resp.encoding_type == "url") { - resp.owned_.emplace_back(curlpp::unescape(raw)); + resp.owned_.emplace_back(utils::UriDecode(raw)); item.name = resp.owned_.back(); } else { item.name = raw; diff --git a/src/types.cc b/src/types.cc index b180d6e2..17f4fa1e 100644 --- a/src/types.cc +++ b/src/types.cc @@ -340,8 +340,10 @@ std::string ReplicationConfig::ToXML() const { auto tag_xml = [](std::string key, std::string value) -> std::string { std::stringstream ss; - ss << "" << "" << key << "" << "" << value - << "" << ""; + ss << "" + << "" << key << "" + << "" << value << "" + << ""; return ss.str(); }; @@ -371,7 +373,8 @@ std::string ReplicationConfig::ToXML() const { ss << ""; } if (rule.destination.metrics) { - ss << "" << ""; + ss << "" + << ""; if (rule.destination.metrics.event_threshold_minutes > 0) { ss << minutes_xml(rule.destination.metrics.event_threshold_minutes); } @@ -379,7 +382,8 @@ std::string ReplicationConfig::ToXML() const { << ""; } if (rule.destination.replication_time) { - ss << "" << "