diff --git a/include/miniocpp/args.h b/include/miniocpp/args.h index 9bf8905..dae815b 100644 --- a/include/miniocpp/args.h +++ b/include/miniocpp/args.h @@ -382,7 +382,7 @@ struct ComposeSource : public ObjectConditionalReadArgs { utils::Multimap Headers() const; private: - long object_size_ = -1; + std::optional object_size_; utils::Multimap headers_; }; // struct ComposeSource diff --git a/include/miniocpp/rdma.h b/include/miniocpp/rdma.h index 47266de..083e26b 100644 --- a/include/miniocpp/rdma.h +++ b/include/miniocpp/rdma.h @@ -50,11 +50,13 @@ namespace minio::rdma { // Per-request state the RDMA control plane needs to build and sign the S3 // request that carries the token. struct ClientCtx { - // All members carry explicit in-class defaults so designated-initializer - // construction (e.g. `ClientCtx{.bucket=...}`) does not trip - // -Wmissing-field-initializers for the std::string fields we leave - // unspecified at single-shot Put/Get call sites (uploadId/partNumber for - // non-multipart paths, etag/checksum for fields populated by the callee). + // All members carry explicit in-class defaults so aggregate initialization + // can omit trailing members (and spell skipped ones as {}/std::nullopt) + // without tripping -Wmissing-field-initializers for the std::string fields + // left unspecified at single-shot Put/Get call sites (uploadId/partNumber + // for non-multipart paths, etag/checksum populated by the callee). Positional + // init is used rather than C++20 designated initializers so the library + // keeps building under the default C++17 standard. minio::creds::Provider* const provider = nullptr; std::string bucket = {}; std::string object = {}; diff --git a/include/miniocpp/types.h b/include/miniocpp/types.h index 5be1ae4..98e2910 100644 --- a/include/miniocpp/types.h +++ b/include/miniocpp/types.h @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -293,17 +294,18 @@ struct SelectRequest { struct SelectResult { error::Error err = error::SUCCESS; bool ended = false; - long int bytes_scanned = -1; - long int bytes_processed = -1; - long int bytes_returned = -1; + std::optional bytes_scanned; + std::optional bytes_processed; + std::optional bytes_returned; std::string records; SelectResult() : ended(true) {} explicit SelectResult(error::Error err) : err(std::move(err)), ended(true) {} - SelectResult(long int bytes_scanned, long int bytes_processed, - long int bytes_returned) + SelectResult(std::optional bytes_scanned, + std::optional bytes_processed, + std::optional bytes_returned) : bytes_scanned(bytes_scanned), bytes_processed(bytes_processed), bytes_returned(bytes_returned) {} diff --git a/src/args.cc b/src/args.cc index deb8e02..e4a89eb 100644 --- a/src/args.cc +++ b/src/args.cc @@ -381,7 +381,7 @@ error::Error ComposeSource::BuildHeaders(size_t object_size, } } - object_size_ = static_cast(object_size); + object_size_ = object_size; headers_ = CopyHeaders(); if (!headers_.Contains("x-amz-copy-source-if-match")) { headers_.Add("x-amz-copy-source-if-match", etag); @@ -391,14 +391,14 @@ error::Error ComposeSource::BuildHeaders(size_t object_size, } size_t ComposeSource::ObjectSize() const { - if (object_size_ == -1) { + if (!object_size_.has_value()) { std::cerr << "ABORT: ComposeSource::BuildHeaders() must be called prior to " "this method invocation. This should not happen." << std::endl; std::terminate(); } - return object_size_; + return *object_size_; } utils::Multimap ComposeSource::Headers() const { diff --git a/src/baseclient.cc b/src/baseclient.cc index 1a7d5ce..ad2247c 100644 --- a/src/baseclient.cc +++ b/src/baseclient.cc @@ -1446,13 +1446,9 @@ Result BaseClient::PutObject(PutObjectApiArgs args) { #ifdef MINIO_CPP_RDMA if (args.rdmaclient != nullptr && args.rdmaclient->Ready()) { - minio::rdma::ClientCtx putCtx = { - .provider = provider_, - .bucket = args.bucket, - .object = args.object, - .url = base_url_, - .region = region, - }; + minio::rdma::ClientCtx putCtx = {provider_, args.bucket, args.object, + {}, std::nullopt, {}, + base_url_, region}; ssize_t ret = rdmaPutWithRetry(args.rdmaclient, &putCtx, args.buf, args.size); @@ -2048,7 +2044,7 @@ Result BaseClient::StatObject(StatObjectArgs args) { resp.etag = utils::Trim(response->headers.GetFront("etag"), '"'); std::string value = response->headers.GetFront("content-length"); - if (!value.empty()) resp.size = std::stol(value); + if (!value.empty()) resp.size = std::stoll(value); value = response->headers.GetFront("last-modified"); if (!value.empty()) { @@ -2100,14 +2096,9 @@ Result BaseClient::UploadPart(UploadPartArgs args) { } minio::rdma::ClientCtx putCtx = { - .provider = provider_, - .bucket = args.bucket, - .object = args.object, - .uploadId = args.upload_id, - .partNumber = args.part_number, - .url = base_url_, - .region = region, - .checksum = args.checksum_crc64nvme, + provider_, args.bucket, args.object, + args.upload_id, args.part_number, {}, + base_url_, region, args.checksum_crc64nvme, }; ssize_t ret = diff --git a/src/client.cc b/src/client.cc index 90d9187..b8474f2 100644 --- a/src/client.cc +++ b/src/client.cc @@ -705,13 +705,9 @@ Result Client::GetObject(GetObjectArgs args) { size <= kRDMAMaxMemoryRegSize && rdma_client.Register(args.buf, size); if (use_rdma) { - minio::rdma::ClientCtx getCtx = { - .provider = provider_, - .bucket = args.bucket, - .object = args.object, - .url = base_url_, - .region = region, - }; + minio::rdma::ClientCtx getCtx = {provider_, args.bucket, args.object, + {}, 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 @@ -1161,7 +1157,8 @@ Result Client::DownloadObject(DownloadObjectArgs args) { std::string temp_filename = args.filename + "." + curlpp::escape(etag) + ".part.minio"; - std::ofstream fout(temp_filename, std::ios::trunc | std::ios::out); + std::ofstream fout(temp_filename, + std::ios::trunc | std::ios::out | std::ios::binary); if (!fout.is_open()) { return error::make("unable to open file " + temp_filename); @@ -1236,13 +1233,9 @@ Result Client::PutObject(PutObjectArgs args) { size <= kRDMAMaxMemoryRegSize && rdma_client.Register(args.buf, size); if (use_rdma) { - minio::rdma::ClientCtx putCtx = { - .provider = provider_, - .bucket = args.bucket, - .object = args.object, - .url = base_url_, - .region = region, - }; + minio::rdma::ClientCtx putCtx = {provider_, args.bucket, args.object, + {}, std::nullopt, {}, + base_url_, region}; // RAII, matching the multipart paths below -- see the GET path for why // a manual Deregister after the call is not enough. diff --git a/src/select.cc b/src/select.cc index ce6c4db..6851b35 100644 --- a/src/select.cc +++ b/src/select.cc @@ -195,21 +195,21 @@ bool SelectHandler::process(const http::DataFunctionArgs& /* args */, auto root = xdoc.select_node(xpath.c_str()); pugi::xpath_node text; std::string value; - long int bytes_scanned = -1; - long int bytes_processed = -1; - long int bytes_returned = -1; + std::optional bytes_scanned; + std::optional bytes_processed; + std::optional bytes_returned; text = root.node().select_node("BytesScanned/text()"); value = text.node().value(); - if (!value.empty()) bytes_scanned = std::stol(value); + if (!value.empty()) bytes_scanned = std::stoll(value); text = root.node().select_node("BytesProcessed/text()"); value = text.node().value(); - if (!value.empty()) bytes_processed = std::stol(value); + if (!value.empty()) bytes_processed = std::stoll(value); text = root.node().select_node("BytesReturned/text()"); value = text.node().value(); - if (!value.empty()) bytes_returned = std::stol(value); + if (!value.empty()) bytes_returned = std::stoll(value); cont = result_func_( SelectResult(bytes_scanned, bytes_processed, bytes_returned)); diff --git a/tests/tests.cc b/tests/tests.cc index bc5c75d..e0f2e68 100644 --- a/tests/tests.cc +++ b/tests/tests.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include using minio::Result; @@ -38,6 +39,7 @@ using minio::Result; #include #include #include +#include #include #include #include @@ -102,6 +104,44 @@ std::string RandBucketName() { std::string RandObjectName() { return RandomString(charset, 8); } +std::string PutUint32BigEndian(unsigned int v) { + std::string s(4, '\0'); + s[0] = static_cast((v >> 24) & 0xFF); + s[1] = static_cast((v >> 16) & 0xFF); + s[2] = static_cast((v >> 8) & 0xFF); + s[3] = static_cast(v & 0xFF); + return s; +} + +// Build a single S3 Select protocol frame (prelude + prelude CRC + headers +// + payload + message CRC) for the given event headers and XML payload. The +// headers section is exactly the encoded headers: the wire format (AWS S3 and +// MinIO) has no terminator byte, the prelude's headers-length is the boundary. +std::string MakeSelectFrame(const std::map& headers, + const std::string& payload) { + std::string headerdata; + for (const auto& [name, value] : headers) { + headerdata += static_cast(name.length()); + headerdata += name; + headerdata += static_cast(7); // header value type: string + headerdata += static_cast((value.length() >> 8) & 0xFF); + headerdata += static_cast(value.length() & 0xFF); + headerdata += value; + } + + std::string data = headerdata + payload; + unsigned int total_length = 16 + static_cast(data.length()); + std::string prelude = + PutUint32BigEndian(total_length) + + PutUint32BigEndian(static_cast(headerdata.length())); + std::string prelude_crc = PutUint32BigEndian( + static_cast(minio::utils::CRC32(prelude))); + std::string message = prelude + prelude_crc + data; + std::string message_crc = PutUint32BigEndian( + static_cast(minio::utils::CRC32(message))); + return message + message_crc; +} + struct MakeBucketError : public std::runtime_error { MakeBucketError(std::string err) : runtime_error(err) {} }; @@ -332,7 +372,12 @@ class Tests { std::string object_name = RandObjectName(); - std::string data = "DownloadObject()"; + // Binary-safe round-trip: a newline, the Windows text-mode EOF byte + // (0x1A / Ctrl-Z), and a NUL byte must survive download byte-for-byte. + std::string data = "DownloadObject()\n"; + data += static_cast(0x1A); + data += '\0'; + data += "binary-tail"; std::stringstream ss(data); minio::s3::PutObjectArgs args(ss, static_cast(data.length()), 0); args.bucket = bucket_name_; @@ -353,7 +398,7 @@ class Tests { throw std::runtime_error("DownloadObject(): " + resp.error().String()); } - std::ifstream file(filename); + std::ifstream file(filename, std::ios::binary); file.seekg(0, std::ios::end); size_t length = file.tellg(); file.seekg(0, std::ios::beg); @@ -362,8 +407,9 @@ class Tests { file.close(); if (data != std::string(buf, length)) { - throw std::runtime_error("DownloadObject(): expected: " + data + - "; got: " + buf); + throw std::runtime_error( + "DownloadObject(): expected " + std::to_string(data.length()) + + " bytes; got " + std::to_string(length) + " bytes"); } std::filesystem::remove(filename); RemoveObject(bucket_name_, object_name); @@ -1541,6 +1587,58 @@ class Tests { } } // TestAsyncOperations + // Regression test for SelectHandler Stats metric parsing: metrics larger + // than INT32_MAX must round-trip as exact long long values (any fallback + // to std::stol on 32-bit Windows LLP64 would truncate them). Uses a + // synthetic Stats event frame, independent of large objects. + void SelectStatsMetrics() { + std::cout << "SelectStatsMetrics()" << std::endl; + + const long long scanned = 5000000000LL; + const long long processed = 6000000000LL; + const long long returned = 7000000000LL; + + std::map headers = { + {":message-type", "event"}, + {":event-type", "Stats"}, + }; + std::string payload = "" + std::to_string(scanned) + + "" + + std::to_string(processed) + + "" + + std::to_string(returned) + ""; + + bool stats_delivered = false; + minio::s3::SelectHandler handler( + [&](minio::s3::SelectResult result) -> bool { + if (result.err) { + throw std::runtime_error("SelectStatsMetrics(): " + + result.err.String()); + } + if (!result.bytes_scanned.has_value() || + *result.bytes_scanned != scanned || + !result.bytes_processed.has_value() || + *result.bytes_processed != processed || + !result.bytes_returned.has_value() || + *result.bytes_returned != returned) { + throw std::runtime_error( + "SelectStatsMetrics(): unexpected metrics"); + } + stats_delivered = true; + return true; + }); + + minio::http::DataFunctionArgs args; + args.datachunk = MakeSelectFrame(headers, payload); + if (!handler.DataFunction(args)) { + throw std::runtime_error("SelectStatsMetrics(): DataFunction failed"); + } + if (!stats_delivered) { + throw std::runtime_error( + "SelectStatsMetrics(): Stats result was not delivered"); + } + } + // Issue #205 regression: a failed AssumeRoleProvider::Fetch() leaves // access_key/secret_key/session_token empty. The failure must be surfaced // via creds.err instead of silently printing empty fields; on success all @@ -1674,6 +1772,7 @@ int main(int /*argc*/, char* /*argv*/[]) { tests.SelectObjectContent(); tests.ListenBucketNotification(); tests.TestAsyncOperations(); + tests.SelectStatsMetrics(); tests.AssumeRoleProvider(); return EXIT_SUCCESS;