Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/miniocpp/args.h
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ struct ComposeSource : public ObjectConditionalReadArgs {
utils::Multimap Headers() const;

private:
long object_size_ = -1;
std::optional<size_t> object_size_;
utils::Multimap headers_;
}; // struct ComposeSource

Expand Down
12 changes: 7 additions & 5 deletions include/miniocpp/rdma.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand Down
12 changes: 7 additions & 5 deletions include/miniocpp/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <map>
#include <memory>
#include <nlohmann/json_fwd.hpp>
#include <optional>
#include <ostream>
#include <string>
#include <type_traits>
Expand Down Expand Up @@ -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<long long> bytes_scanned;
std::optional<long long> bytes_processed;
std::optional<long long> 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<long long> bytes_scanned,
std::optional<long long> bytes_processed,
std::optional<long long> bytes_returned)
: bytes_scanned(bytes_scanned),
bytes_processed(bytes_processed),
bytes_returned(bytes_returned) {}
Expand Down
6 changes: 3 additions & 3 deletions src/args.cc
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ error::Error ComposeSource::BuildHeaders(size_t object_size,
}
}

object_size_ = static_cast<long>(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);
Expand All @@ -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 {
Expand Down
23 changes: 7 additions & 16 deletions src/baseclient.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1446,13 +1446,9 @@ Result<PutObjectResponse> 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);
Expand Down Expand Up @@ -2048,7 +2044,7 @@ Result<StatObjectResponse> 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()) {
Expand Down Expand Up @@ -2100,14 +2096,9 @@ Result<UploadPartResponse> 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 =
Expand Down
23 changes: 8 additions & 15 deletions src/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -705,13 +705,9 @@ Result<GetObjectResponse> 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
Expand Down Expand Up @@ -1161,7 +1157,8 @@ Result<DownloadObjectResponse> 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<DownloadObjectResponse>("unable to open file " +
temp_filename);
Expand Down Expand Up @@ -1236,13 +1233,9 @@ Result<PutObjectResponse> 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.
Expand Down
12 changes: 6 additions & 6 deletions src/select.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<long long> bytes_scanned;
std::optional<long long> bytes_processed;
std::optional<long long> 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));
Expand Down
107 changes: 103 additions & 4 deletions tests/tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <miniocpp/request.h>
#include <miniocpp/response.h>
#include <miniocpp/result.h>
#include <miniocpp/select.h>
#include <miniocpp/types.h>

using minio::Result;
Expand All @@ -38,6 +39,7 @@ using minio::Result;
#include <iosfwd>
#include <iostream>
#include <list>
#include <map>
#include <ostream>
#include <random>
#include <sstream>
Expand Down Expand Up @@ -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<char>((v >> 24) & 0xFF);
s[1] = static_cast<char>((v >> 16) & 0xFF);
s[2] = static_cast<char>((v >> 8) & 0xFF);
s[3] = static_cast<char>(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<std::string, std::string>& headers,
const std::string& payload) {
std::string headerdata;
for (const auto& [name, value] : headers) {
headerdata += static_cast<char>(name.length());
headerdata += name;
headerdata += static_cast<char>(7); // header value type: string
headerdata += static_cast<char>((value.length() >> 8) & 0xFF);
headerdata += static_cast<char>(value.length() & 0xFF);
headerdata += value;
}

std::string data = headerdata + payload;
unsigned int total_length = 16 + static_cast<unsigned int>(data.length());
std::string prelude =
PutUint32BigEndian(total_length) +
PutUint32BigEndian(static_cast<unsigned int>(headerdata.length()));
std::string prelude_crc = PutUint32BigEndian(
static_cast<unsigned int>(minio::utils::CRC32(prelude)));
std::string message = prelude + prelude_crc + data;
std::string message_crc = PutUint32BigEndian(
static_cast<unsigned int>(minio::utils::CRC32(message)));
return message + message_crc;
}

struct MakeBucketError : public std::runtime_error {
MakeBucketError(std::string err) : runtime_error(err) {}
};
Expand Down Expand Up @@ -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<char>(0x1A);
data += '\0';
data += "binary-tail";
std::stringstream ss(data);
minio::s3::PutObjectArgs args(ss, static_cast<uint64_t>(data.length()), 0);
args.bucket = bucket_name_;
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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<std::string, std::string> headers = {
{":message-type", "event"},
{":event-type", "Stats"},
};
std::string payload = "<Stats><BytesScanned>" + std::to_string(scanned) +
"</BytesScanned><BytesProcessed>" +
std::to_string(processed) +
"</BytesProcessed><BytesReturned>" +
std::to_string(returned) + "</BytesReturned></Stats>";

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
Expand Down Expand Up @@ -1674,6 +1772,7 @@ int main(int /*argc*/, char* /*argv*/[]) {
tests.SelectObjectContent();
tests.ListenBucketNotification();
tests.TestAsyncOperations();
tests.SelectStatsMetrics();
tests.AssumeRoleProvider();

return EXIT_SUCCESS;
Expand Down
Loading