morningman commented on code in PR #68117:
URL: https://github.com/apache/doris/pull/68117#discussion_r4037538848


##########
be/src/util/s3_util.cpp:
##########
@@ -165,8 +234,370 @@ constexpr char S3_NEED_OVERRIDE_ENDPOINT[] = 
"AWS_NEED_OVERRIDE_ENDPOINT";
 constexpr char S3_ROLE_ARN[] = "AWS_ROLE_ARN";
 constexpr char S3_EXTERNAL_ID[] = "AWS_EXTERNAL_ID";
 constexpr char S3_CREDENTIALS_PROVIDER_TYPE[] = 
"AWS_CREDENTIALS_PROVIDER_TYPE";
+
+// Native Azure binding keys.  The AWS_* aliases above remain accepted for
+// existing object-storage callers, but Azure scans use these provider-owned
+// names so their meaning does not depend on the S3 adapter.
+constexpr char AZURE_AUTH_TYPE[] = "AZURE_AUTH_TYPE";
+constexpr char AZURE_ENDPOINT[] = "AZURE_ENDPOINT";
+constexpr char AZURE_ACCOUNT_NAME[] = "AZURE_ACCOUNT_NAME";
+constexpr char AZURE_ACCOUNT_KEY[] = "AZURE_ACCOUNT_KEY";
+constexpr char AZURE_CONTAINER[] = "AZURE_CONTAINER";
+constexpr char AZURE_SAS_TOKEN[] = "AZURE_SAS_TOKEN";
+constexpr char AZURE_SAS_EXPIRY_MS[] = "AZURE_SAS_EXPIRY_MS";
+constexpr char AZURE_CLIENT_ID[] = "AZURE_CLIENT_ID";
+constexpr char AZURE_CLIENT_SECRET[] = "AZURE_CLIENT_SECRET";
+constexpr char AZURE_TENANT_ID[] = "AZURE_TENANT_ID";
+constexpr char AZURE_OAUTH_SERVER_URI[] = "AZURE_OAUTH_SERVER_URI";
+
+const std::string* find_property(const StringCaseMap<std::string>& properties,
+                                 std::initializer_list<const char*> names) {
+    for (const auto* name : names) {
+        auto it = properties.find(name);
+        if (it != properties.end()) {
+            return &it->second;
+        }
+    }
+    return nullptr;
+}
+
+bool has_property(const StringCaseMap<std::string>& properties,
+                  std::initializer_list<const char*> names) {
+    return find_property(properties, names) != nullptr;
+}
+
+std::string normalize_azure_endpoint(std::string endpoint) {
+    if (endpoint.empty()) {
+        return endpoint;
+    }
+    const bool has_scheme = endpoint.find("://") != std::string::npos;
+    if (!has_scheme) {
+        endpoint = "https://"; + endpoint;
+    }
+    const auto scheme_end = endpoint.find("://");
+    endpoint.replace(0, scheme_end, to_lower(endpoint.substr(0, scheme_end)));
+    const auto authority_begin = scheme_end == std::string::npos ? 0 : 
scheme_end + 3;
+    const auto authority_end = endpoint.find('/', authority_begin);
+    const auto authority_length = authority_end == std::string::npos
+                                          ? endpoint.size() - authority_begin
+                                          : authority_end - authority_begin;
+    const auto authority = endpoint.substr(authority_begin, authority_length);
+    if (authority.empty()) {
+        return endpoint;
+    }
+
+    auto lower_authority = to_lower(authority);
+    endpoint.replace(authority_begin, authority_length, lower_authority);
+    // Match the host, not host:port, so explicit transport ports do not 
disable
+    // the official DFS-to-Blob conversion. Custom proxy hosts stay unchanged.
+    const auto host = lower_authority.substr(0, lower_authority.find(':'));
+    const auto dfs_pos = host.find(".dfs.");
+    const bool official_dfs =
+            dfs_pos != std::string::npos && 
(host.ends_with(".dfs.core.windows.net") ||
+                                             
host.ends_with(".dfs.core.chinacloudapi.cn") ||
+                                             
host.ends_with(".dfs.core.usgovcloudapi.net") ||
+                                             
host.ends_with(".dfs.core.cloudapi.de"));
+    if (official_dfs) {
+        endpoint.replace(authority_begin + dfs_pos, 5, ".blob.");
+    } else if (!has_scheme && authority.find('.') == std::string::npos &&

Review Comment:
   Fixed in 573f12c86da. Account-name suffix inference now only runs where the 
native protocol is parsed (`convert_native_azure_properties`); 
`_create_azure_client()` applies just the old factory's transport rule (default 
scheme + endpoint/container join boundary), so `AWS_ENDPOINT=storage-proxy` 
still targets `https://storage-proxy/container`. 
`S3ClientFactoryTest.LegacyAzureEndpointsKeepSingleLabelHosts` covers the 
property / `ObjectStoreInfoPB` / `TS3StorageParam` producers with unschemed 
single-label hosts (with and without port) and asserts the container base the 
SDK signs against.



##########
be/src/util/s3_util.cpp:
##########
@@ -165,8 +234,370 @@ constexpr char S3_NEED_OVERRIDE_ENDPOINT[] = 
"AWS_NEED_OVERRIDE_ENDPOINT";
 constexpr char S3_ROLE_ARN[] = "AWS_ROLE_ARN";
 constexpr char S3_EXTERNAL_ID[] = "AWS_EXTERNAL_ID";
 constexpr char S3_CREDENTIALS_PROVIDER_TYPE[] = 
"AWS_CREDENTIALS_PROVIDER_TYPE";
+
+// Native Azure binding keys.  The AWS_* aliases above remain accepted for
+// existing object-storage callers, but Azure scans use these provider-owned
+// names so their meaning does not depend on the S3 adapter.
+constexpr char AZURE_AUTH_TYPE[] = "AZURE_AUTH_TYPE";
+constexpr char AZURE_ENDPOINT[] = "AZURE_ENDPOINT";
+constexpr char AZURE_ACCOUNT_NAME[] = "AZURE_ACCOUNT_NAME";
+constexpr char AZURE_ACCOUNT_KEY[] = "AZURE_ACCOUNT_KEY";
+constexpr char AZURE_CONTAINER[] = "AZURE_CONTAINER";
+constexpr char AZURE_SAS_TOKEN[] = "AZURE_SAS_TOKEN";
+constexpr char AZURE_SAS_EXPIRY_MS[] = "AZURE_SAS_EXPIRY_MS";
+constexpr char AZURE_CLIENT_ID[] = "AZURE_CLIENT_ID";
+constexpr char AZURE_CLIENT_SECRET[] = "AZURE_CLIENT_SECRET";
+constexpr char AZURE_TENANT_ID[] = "AZURE_TENANT_ID";
+constexpr char AZURE_OAUTH_SERVER_URI[] = "AZURE_OAUTH_SERVER_URI";
+
+const std::string* find_property(const StringCaseMap<std::string>& properties,
+                                 std::initializer_list<const char*> names) {
+    for (const auto* name : names) {
+        auto it = properties.find(name);
+        if (it != properties.end()) {
+            return &it->second;
+        }
+    }
+    return nullptr;
+}
+
+bool has_property(const StringCaseMap<std::string>& properties,
+                  std::initializer_list<const char*> names) {
+    return find_property(properties, names) != nullptr;
+}
+
+std::string normalize_azure_endpoint(std::string endpoint) {
+    if (endpoint.empty()) {
+        return endpoint;
+    }
+    const bool has_scheme = endpoint.find("://") != std::string::npos;
+    if (!has_scheme) {
+        endpoint = "https://"; + endpoint;
+    }
+    const auto scheme_end = endpoint.find("://");
+    endpoint.replace(0, scheme_end, to_lower(endpoint.substr(0, scheme_end)));
+    const auto authority_begin = scheme_end == std::string::npos ? 0 : 
scheme_end + 3;
+    const auto authority_end = endpoint.find('/', authority_begin);
+    const auto authority_length = authority_end == std::string::npos
+                                          ? endpoint.size() - authority_begin
+                                          : authority_end - authority_begin;
+    const auto authority = endpoint.substr(authority_begin, authority_length);
+    if (authority.empty()) {
+        return endpoint;
+    }
+
+    auto lower_authority = to_lower(authority);
+    endpoint.replace(authority_begin, authority_length, lower_authority);
+    // Match the host, not host:port, so explicit transport ports do not 
disable
+    // the official DFS-to-Blob conversion. Custom proxy hosts stay unchanged.
+    const auto host = lower_authority.substr(0, lower_authority.find(':'));
+    const auto dfs_pos = host.find(".dfs.");
+    const bool official_dfs =
+            dfs_pos != std::string::npos && 
(host.ends_with(".dfs.core.windows.net") ||
+                                             
host.ends_with(".dfs.core.chinacloudapi.cn") ||
+                                             
host.ends_with(".dfs.core.usgovcloudapi.net") ||
+                                             
host.ends_with(".dfs.core.cloudapi.de"));
+    if (official_dfs) {
+        endpoint.replace(authority_begin + dfs_pos, 5, ".blob.");
+    } else if (!has_scheme && authority.find('.') == std::string::npos &&
+               authority.find(':') == std::string::npos) {
+        endpoint.insert(authority_begin + authority.size(), 
".blob.core.windows.net");
+    }
+    while (endpoint.ends_with('/')) {
+        endpoint.pop_back();
+    }
+    return endpoint;
+}
+
+std::string endpoint_authority(const std::string& endpoint) {
+    const auto begin = endpoint.find("://") + 3;
+    auto authority = endpoint.substr(begin, endpoint.find('/', begin) - begin);
+    if (endpoint.starts_with("https://";) && authority.ends_with(":443")) {
+        authority.resize(authority.size() - 4);
+    } else if (endpoint.starts_with("http://";) && authority.ends_with(":80")) {
+        authority.resize(authority.size() - 3);
+    }
+    return authority;
+}
+
+// Only established SharedKey wire producers use AWS fields for Azure. Once
+// translated here the native factory never inspects these fields again.
+void import_legacy_azure_shared_key(S3ClientConf* conf) {
+    // Keep the established SharedKey endpoint normalization at the legacy
+    // boundary. Native endpoints and object keys retain their internal 
slashes.
+    conf->endpoint = normalize_http_uri(conf->endpoint);

Review Comment:
   Fixed in 573f12c86da. `import_legacy_azure_shared_key()` no longer calls 
`normalize_http_uri()`; the persisted endpoint stays byte-for-byte and only the 
join boundary with the container is normalized, so 
`https://proxy.example/gateway//tenant` still becomes 
`https://proxy.example/gateway//tenant/container`. 
`S3ClientFactoryTest.LegacyAzureEndpointsKeepCustomBasePathsBytePreserving` 
covers all three producers with repeated separators, a trailing slash and an 
encoded base path.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to