SteNicholas commented on code in PR #244:
URL: https://github.com/apache/paimon-cpp/pull/244#discussion_r3849088361


##########
src/paimon/rest/dlf_auth.cpp:
##########
@@ -0,0 +1,808 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "paimon/rest/dlf_auth.h"
+
+#include <openssl/evp.h>
+
+#include <array>
+#include <cctype>
+#include <climits>
+#include <ctime>
+#include <fstream>
+#include <iomanip>
+#include <limits>
+#include <regex>
+#include <set>
+#include <sstream>
+#include <string_view>
+#include <thread>
+#include <utility>
+
+#include "fmt/format.h"
+#include "paimon/catalog_options.h"
+#include "paimon/common/utils/string_utils.h"
+#include "paimon/common/utils/url_utils.h"
+#include "paimon/common/utils/uuid.h"
+#include "paimon/rest/rest_http_client.h"
+#include "rapidjson/document.h"
+
+namespace paimon {
+
+namespace {
+
+constexpr int32_t kEcsMetadataRequestTimeoutMillis = 3 * 60 * 1000;
+
+constexpr int64_t kTokenExpirationSafeTimeMillis = 60 * 60 * 1000;
+constexpr size_t kMaxTokenResponseBytes = 1024 * 1024;
+constexpr const char kDefaultEcsMetadataUrl[] =
+    "http://100.100.100.200/latest/meta-data/Ram/security-credentials/";;
+
+constexpr const char kAuthorizationHeader[] = "Authorization";
+constexpr const char kContentMd5Header[] = "Content-MD5";
+constexpr const char kContentTypeHeader[] = "Content-Type";
+constexpr const char kDlfDateHeader[] = "x-dlf-date";
+constexpr const char kDlfSecurityTokenHeader[] = "x-dlf-security-token";
+constexpr const char kDlfVersionHeader[] = "x-dlf-version";
+constexpr const char kDlfContentSha256Header[] = "x-dlf-content-sha256";
+constexpr const char kUnsignedPayload[] = "UNSIGNED-PAYLOAD";
+constexpr const char kJsonMediaType[] = "application/json";
+
+constexpr const char kOpenApiDateHeader[] = "Date";
+constexpr const char kOpenApiAcceptHeader[] = "Accept";
+constexpr const char kOpenApiHostHeader[] = "Host";
+constexpr const char kAcsSignatureMethodHeader[] = "x-acs-signature-method";
+constexpr const char kAcsSignatureNonceHeader[] = "x-acs-signature-nonce";
+constexpr const char kAcsSignatureVersionHeader[] = "x-acs-signature-version";
+constexpr const char kAcsVersionHeader[] = "x-acs-version";
+constexpr const char kAcsSecurityTokenHeader[] = "x-acs-security-token";
+
+void TrimWhitespace(std::string* value) {
+    size_t begin = 0;
+    while (begin < value->size() && std::isspace(static_cast<unsigned 
char>((*value)[begin]))) {
+        ++begin;
+    }
+    size_t end = value->size();
+    while (end > begin && std::isspace(static_cast<unsigned char>((*value)[end 
- 1]))) {
+        --end;
+    }
+    *value = value->substr(begin, end - begin);
+}
+
+std::optional<std::string> FindOption(const std::map<std::string, 
std::string>& options,
+                                      const std::string& key) {
+    auto iter = options.find(key);
+    if (iter == options.end()) {
+        return std::nullopt;
+    }
+    return iter->second;
+}
+
+Result<std::string> RequiredNonEmptyOption(const std::map<std::string, 
std::string>& options,
+                                           const std::string& key) {
+    std::optional<std::string> value = FindOption(options, key);
+    if (!value || value->empty()) {
+        return Status::Invalid(fmt::format("option '{}' must be configured for 
DLF auth", key));
+    }
+    return value.value();
+}
+
+Result<std::string> RequiredJsonString(const rapidjson::Value& object, const 
char* key) {
+    if (!object.HasMember(key) || !object[key].IsString() || 
object[key].GetStringLength() == 0) {
+        return Status::Invalid(fmt::format("DLF token field '{}' must be a 
non-empty string", key));
+    }
+    return std::string(object[key].GetString(), object[key].GetStringLength());
+}
+
+Result<std::optional<std::string>> OptionalJsonString(const rapidjson::Value& 
object,
+                                                      const char* key) {
+    if (!object.HasMember(key) || object[key].IsNull()) {
+        return std::optional<std::string>();
+    }
+    if (!object[key].IsString()) {
+        return Status::Invalid(fmt::format("DLF token field '{}' must be a 
string", key));
+    }
+    return std::optional<std::string>(
+        std::string(object[key].GetString(), object[key].GetStringLength()));
+}
+
+Result<std::tm> ToUtc(std::chrono::system_clock::time_point time) {
+    std::time_t seconds = std::chrono::system_clock::to_time_t(time);
+    std::tm utc{};
+#if defined(_WIN32)
+    if (gmtime_s(&utc, &seconds) != 0) {
+        return Status::Invalid("failed to convert DLF signing time to UTC");
+    }
+#else
+    if (gmtime_r(&seconds, &utc) == nullptr) {
+        return Status::Invalid("failed to convert DLF signing time to UTC");
+    }
+#endif
+    return utc;
+}
+
+Result<std::string> FormatDlfTime(std::chrono::system_clock::time_point time) {
+    PAIMON_ASSIGN_OR_RAISE(std::tm utc, ToUtc(time));
+    std::array<char, 32> buffer{};
+    if (std::strftime(buffer.data(), buffer.size(), "%Y%m%dT%H%M%SZ", &utc) == 
0) {
+        return Status::Invalid("failed to format DLF signing time");
+    }
+    return std::string(buffer.data());
+}
+
+Result<std::string> FormatRfc1123Time(std::chrono::system_clock::time_point 
time) {
+    PAIMON_ASSIGN_OR_RAISE(std::tm utc, ToUtc(time));
+    static constexpr std::array<const char*, 7> kWeekdays = {"Sun", "Mon", 
"Tue", "Wed",
+                                                             "Thu", "Fri", 
"Sat"};
+    static constexpr std::array<const char*, 12> kMonths = {
+        "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", 
"Nov", "Dec"};
+    if (utc.tm_wday < 0 || utc.tm_wday >= 
static_cast<int32_t>(kWeekdays.size()) ||
+        utc.tm_mon < 0 || utc.tm_mon >= static_cast<int32_t>(kMonths.size())) {
+        return Status::Invalid("failed to format DLF OpenAPI signing time");
+    }
+    return fmt::format("{}, {:02d} {} {:04d} {:02d}:{:02d}:{:02d} GMT", 
kWeekdays[utc.tm_wday],
+                       utc.tm_mday, kMonths[utc.tm_mon], utc.tm_year + 1900, 
utc.tm_hour,
+                       utc.tm_min, utc.tm_sec);
+}
+
+Result<int64_t> ParseExpiration(const std::string& expiration) {
+    static const std::regex kExpirationPattern(
+        "^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$");
+    if (!std::regex_match(expiration, kExpirationPattern)) {
+        return Status::Invalid("invalid DLF token expiration");
+    }
+    std::tm utc{};
+    std::istringstream stream(expiration);
+    stream >> std::get_time(&utc, "%Y-%m-%dT%H:%M:%SZ");
+    if (stream.fail() || stream.peek() != std::char_traits<char>::eof()) {
+        return Status::Invalid("invalid DLF token expiration");
+    }
+    int32_t year = utc.tm_year;
+    int32_t month = utc.tm_mon;
+    int32_t day = utc.tm_mday;
+    int32_t hour = utc.tm_hour;
+    int32_t minute = utc.tm_min;
+    int32_t second = utc.tm_sec;
+    std::time_t timestamp = timegm(&utc);

Review Comment:
   The `_WIN32` branches in this file are dead code and cannot work as written. 
`timegm` is called unconditionally here, but it does not exist on MSVC (the 
equivalent is `_mkgmtime`). So the `#if defined(_WIN32)` / `gmtime_s` branches 
at L128-136 and L184-192 cannot actually make this file compile on Windows — 
they only give the impression that Windows is handled.
   
   Meanwhile `docs/source/building.rst` states "Windows is not supported for 
now", and `include/paimon/visibility.h` is the only other `_WIN32` user in the 
tree, so this is not an established pattern here either.
   
   Suggest dropping the `_WIN32` branches and just using `gmtime_r` + `timegm`, 
consistent with the rest of the repository. (Or complete them with `_mkgmtime`, 
if Windows support is actually intended — but that seems out of scope for this 
PR.)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to