github-actions[bot] commented on code in PR #68117:
URL: https://github.com/apache/doris/pull/68117#discussion_r4043840115


##########
be/src/util/s3_util.cpp:
##########
@@ -165,8 +234,391 @@ 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;
+}
+
+// Native protocol only: resolves the documented AZURE_ENDPOINT shorthands
+// (bare account name, official DFS host) into the Blob transport origin. 
Legacy
+// SharedKey endpoints are literal and never pass through here.
+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;
+}
+
+#ifdef USE_AZURE
+// The endpoint handed to the SDK. Native endpoints were already normalized
+// while parsing the native protocol. Legacy SharedKey producers hand over an
+// endpoint literal that only ever received a default scheme, so keep that
+// contract here: no account-name inference, no DFS-to-Blob rewrite and no path
+// rewriting, which would silently redirect single-label proxy hosts or custom
+// reverse-proxy routes after an upgrade. Only the endpoint/container join
+// boundary is normalized. Only _create_azure_client() consumes it, so keep it
+// under the same guard or a BUILD_AZURE=OFF build fails on -Wunused-function.
+std::string azure_transport_endpoint(std::string endpoint) {
+    if (endpoint.find("://") == std::string::npos) {
+        endpoint = "https://"; + endpoint;
+    }
+    while (endpoint.ends_with('/')) {
+        endpoint.pop_back();
+    }
+    return endpoint;
+}
+#endif
+
+// Only established SharedKey wire producers use AWS fields for Azure. Once
+// translated here the native factory never inspects these fields again. The
+// endpoint stays byte-for-byte as configured, exactly like the old factory.
+void import_legacy_azure_shared_key(S3ClientConf* conf) {
+    conf->azure_credentials = {};
+    conf->azure_credentials.type = AzureCredentialType::SHARED_KEY;
+    conf->azure_credentials.account_name = std::move(conf->ak);
+    conf->azure_credentials.account_key = std::move(conf->sk);
+    conf->ak.clear();
+    conf->sk.clear();
+    conf->token.clear();
+    conf->region.clear();
+    conf->role_arn.clear();
+    conf->external_id.clear();
+    conf->cred_provider_type = CredProviderType::Default;
+}
+
+Status convert_legacy_azure_properties(const StringCaseMap<std::string>& 
properties,
+                                       S3ClientConf* client_conf) {
+    auto& client = *client_conf;
+    // Compatibility is deliberately limited to the old SharedKey map. An
+    // incomplete native map must not be mistaken for that old protocol.
+    for (const auto& [key, value] : properties) {
+        const auto lower = to_lower(key);
+        if (lower.starts_with("azure") || (lower == "aws_token" && 
!value.empty())) {
+            return Status::InvalidArgument("Azure native credentials require 
AZURE_AUTH_TYPE");
+        }
+    }
+    if (!has_property(properties, {S3_ENDPOINT}) || !has_property(properties, 
{S3_AK}) ||
+        !has_property(properties, {S3_SK})) {
+        return Status::InvalidArgument("Azure native credentials require 
AZURE_AUTH_TYPE");
+    }
+    client.endpoint = *find_property(properties, {S3_ENDPOINT});
+    client.ak = *find_property(properties, {S3_AK});
+    client.sk = *find_property(properties, {S3_SK});
+    import_legacy_azure_shared_key(&client);
+    for (const auto& [name, target] :
+         {std::pair {S3_MAX_CONN_SIZE, &client.max_connections},
+          std::pair {S3_REQUEST_TIMEOUT_MS, &client.request_timeout_ms},
+          std::pair {S3_CONN_TIMEOUT_MS, &client.connect_timeout_ms}}) {
+        if (const auto* value = find_property(properties, {name}); value != 
nullptr) {
+            if (!to_int(*value, *target)) {
+                return Status::InvalidArgument("invalid Azure connection 
option {}", name);
+            }
+        }
+    }
+    return Status::OK();
+}
+
+Status validate_native_azure_shared_key_compatibility(const 
StringCaseMap<std::string>& properties,
+                                                      const 
AzureCredentialOptions& credential) {
+    const auto* legacy_endpoint = find_property(properties, {S3_ENDPOINT});
+    const auto* legacy_account = find_property(properties, {S3_AK});
+    const auto* legacy_key = find_property(properties, {S3_SK});
+    const auto* native_endpoint = find_property(properties, {AZURE_ENDPOINT});
+    const auto* legacy_override = find_property(properties, 
{"AWS_NEED_OVERRIDE_ENDPOINT"});
+    const bool has_legacy_fields =
+            legacy_endpoint != nullptr || legacy_account != nullptr || 
legacy_key != nullptr ||
+            find_property(properties, {S3_REGION}) != nullptr || 
legacy_override != nullptr;
+    if (has_legacy_fields &&
+        (legacy_endpoint == nullptr || legacy_account == nullptr || legacy_key 
== nullptr ||
+         native_endpoint == nullptr || *legacy_endpoint != *native_endpoint ||
+         *legacy_account != credential.account_name || *legacy_key != 
credential.account_key)) {
+        return Status::InvalidArgument(
+                "Azure native SharedKey compatibility fields conflict with 
native fields");
+    }
+    return Status::OK();
+}
+
+Status convert_native_azure_properties(const StringCaseMap<std::string>& 
properties,
+                                       const S3URI& uri, const std::string& 
auth_type,
+                                       S3ClientConf* client_conf) {
+    auto& client = *client_conf;
+    const auto is_legacy_shared_key_field = [&](const std::string& key) {
+        return auth_type == "SHARED_KEY" &&
+               (iequal(key, S3_ENDPOINT) || iequal(key, S3_REGION) || 
iequal(key, S3_AK) ||
+                iequal(key, S3_SK) || iequal(key, 
"AWS_NEED_OVERRIDE_ENDPOINT"));
+    };
+    for (const auto& [key, value] : properties) {
+        const auto lower = to_lower(key);
+        if ((lower.starts_with("aws_") || lower.starts_with("azure.")) &&
+            !is_legacy_shared_key_field(key)) {
+            return Status::InvalidArgument(
+                    "Azure native credentials cannot use AWS or catalog 
property aliases");
+        }
+    }
+    // Older FE versions attach a Hadoop configuration view for OneLake to
+    // this map. Ignore those extra keys; they must never supply or override
+    // any native authentication field. FE routing separates the two views.
+    auto& credential = client.azure_credentials;
+    if (auth_type == "SHARED_KEY") {
+        credential.type = AzureCredentialType::SHARED_KEY;
+    } else if (auth_type == "SAS") {
+        credential.type = AzureCredentialType::SAS;
+    } else if (auth_type == "OAUTH2") {
+        credential.type = AzureCredentialType::OAUTH2;
+    } else {
+        return Status::InvalidArgument("unsupported AZURE_AUTH_TYPE in native 
credentials");
+    }
+    auto set = [&](const char* key, std::string* target) {
+        if (const auto* value = find_property(properties, {key}); value != 
nullptr) {
+            *target = *value;
+        }
+    };
+    set(AZURE_ENDPOINT, &client.endpoint);
+    set(AZURE_ACCOUNT_NAME, &credential.account_name);
+    set(AZURE_ACCOUNT_KEY, &credential.account_key);
+    set(AZURE_SAS_TOKEN, &credential.sas_token);
+    set(AZURE_CLIENT_ID, &credential.oauth_client_id);
+    set(AZURE_CLIENT_SECRET, &credential.oauth_client_secret);
+    set(AZURE_TENANT_ID, &credential.oauth_tenant_id);
+    set(AZURE_OAUTH_SERVER_URI, &credential.oauth_server_uri);
+    if (auth_type == "SHARED_KEY") {
+        
RETURN_IF_ERROR(validate_native_azure_shared_key_compatibility(properties, 
credential));
+    }
+    if (const auto* expiry = find_property(properties, {AZURE_SAS_EXPIRY_MS}); 
expiry != nullptr) {
+        if (!to_int64(*expiry, credential.sas_expiration_time_ms) ||
+            credential.sas_expiration_time_ms <= 0) {
+            return Status::InvalidArgument("invalid Azure SAS expiry value");
+        }
+    }
+    if (client.endpoint.empty() || credential.account_name.empty()) {
+        return Status::InvalidArgument(
+                "Azure native credentials require endpoint and account name");
+    }
+    if (credential.type == AzureCredentialType::SHARED_KEY && 
credential.account_key.empty()) {
+        return Status::InvalidArgument("Azure native SharedKey requires an 
account key");
+    }
+    // Existing Azure SharedKey catalogs also contain S3-spelled locations.
+    // This explicit compatibility case carries no account in its URI;
+    // SAS/OAuth2 native data locations must carry their Azure authority.
+    if (uri.get_scheme().empty() ||
+        (uri.get_scheme() == "s3" && credential.type != 
AzureCredentialType::SHARED_KEY)) {
+        return Status::InvalidArgument("Azure native credentials require an 
Azure data URI");
+    }
+    if (client.endpoint.find_first_of("?#@\r\n") != std::string::npos) {
+        return Status::InvalidArgument(
+                "Azure endpoint must not contain credentials, query or 
fragment");
+    }
+    client.endpoint = normalize_azure_endpoint(client.endpoint);
+    if (!client.endpoint.starts_with("https://";) && 
!client.endpoint.starts_with("http://";)) {
+        return Status::InvalidArgument("Azure endpoint must use HTTP or 
HTTPS");
+    }
+    // A container property may constrain legacy callers, but is never a
+    // fallback location. Every native data URI identifies its container.
+    if (const auto* container = find_property(properties, {AZURE_CONTAINER});
+        container != nullptr && *container != uri.get_bucket()) {
+        return Status::InvalidArgument("Azure URI container conflicts with the 
storage binding");
+    }
+    return Status::OK();
+}
+
+Status convert_azure_properties(const StringCaseMap<std::string>& properties, 
const S3URI& uri,
+                                S3Conf* conf) {
+    auto& client = conf->client_conf;
+    client.provider = io::ObjStorageProvider::AZURE;
+    const auto* auth_type = find_property(properties, {AZURE_AUTH_TYPE});
+    if (auth_type == nullptr) {
+        RETURN_IF_ERROR(convert_legacy_azure_properties(properties, &client));
+    } else {
+        RETURN_IF_ERROR(convert_native_azure_properties(properties, uri, 
*auth_type, &client));
+    }
+    if (uri.get_bucket().empty()) {
+        return Status::InvalidArgument("Azure data URI requires a container");
+    }
+    conf->bucket = uri.get_bucket();
+    client.bucket = conf->bucket;
+    if (auth_type != nullptr) {
+        RETURN_IF_ERROR(S3ClientFactory::validate_azure_uri(uri, client));
+    }
+    return is_s3_conf_valid(client);
+}
 } // namespace
 
+Status S3ClientFactory::validate_azure_uri(const S3URI& uri, const 
S3ClientConf& conf) {
+    if (uri.get_scheme().empty()) {
+        return Status::OK(); // Internal file-system callers may pass a raw 
object key.
+    }
+    if (uri.get_bucket() != conf.bucket) {
+        return Status::InvalidArgument("Azure URI container conflicts with the 
storage binding");
+    }
+    if (uri.get_scheme() == "s3") {
+        if (conf.azure_credentials.type != AzureCredentialType::SHARED_KEY) {
+            return Status::InvalidArgument("Azure SAS/OAuth2 data access 
requires an Azure URI");
+        }
+        return Status::OK(); // Old SharedKey file-system paths use the S3 
wire spelling.
+    }
+    const auto uri_host = to_lower(uri.get_endpoint());
+    if (uri_host.ends_with(".dfs.fabric.microsoft.com") ||

Review Comment:
   [P2] Canonicalize the OneLake host before applying this fence
   
   `S3URI` retains an explicit port in `get_endpoint()`, so 
`abfss://[email protected]:443/file` makes both suffix 
checks false. With a matching native endpoint (or a custom transport), the 
later authority checks accept it and route a genuine Fabric location through 
the native Azure client even though this branch requires OneLake to stay on its 
Hadoop binding; the HTTPS Blob spelling has the same issue. Extract the host 
without an optional port (and account for a DNS trailing dot) before the Fabric 
suffix check, and cover both ABFS and HTTPS OneLake authorities with `:443`.



##########
be/src/util/s3_uri.cpp:
##########
@@ -37,53 +45,152 @@ const std::string S3URI::_FRAGMENT_DELIM = "#";
 /// _schema: s3
 /// _bucket: bucket1
 /// _key:    path/to/file.txt
-Status S3URI::parse() {
+bool S3URI::is_azure_endpoint(std::string_view authority) {
+    auto host = absl::AsciiStrToLower(authority.substr(0, 
authority.find(':')));
+    static constexpr std::string_view suffixes[] = {
+            ".blob.core.windows.net",       ".dfs.core.windows.net",
+            ".blob.core.chinacloudapi.cn",  ".dfs.core.chinacloudapi.cn",
+            ".blob.core.usgovcloudapi.net", ".dfs.core.usgovcloudapi.net",
+            ".blob.core.cloudapi.de",       ".dfs.core.cloudapi.de"};
+    return std::ranges::any_of(suffixes, [&host](std::string_view suffix) {
+        return host.ends_with(suffix) && host.size() > suffix.size();
+    });
+}
+
+Status S3URI::_parsing_error(std::string_view message, bool azure_provider) 
const {
+    if (azure_provider || _is_azure) {
+        return Status::InvalidArgument("{}", message);
+    }
+    // The first HTTP parse may precede provider selection. Never echo a
+    // signed query, including for malformed or custom Azure endpoints.
+    const auto safe_location = std::string_view(_location).substr(0, 
_location.find_first_of("?#"));
+    return Status::InvalidArgument("{}: {}", message, safe_location);
+}
+
+Status S3URI::_parse_authority(const std::string& scheme, const std::string& 
rest,
+                               bool azure_provider) {
+    if (scheme == _SCHEME_S3) {
+        // has scheme, eg: s3://bucket1/path/to/file.txt
+        std::vector<std::string> authority_split =
+                absl::StrSplit(rest, absl::MaxSplits(_PATH_DELIM, 1));
+        if (authority_split.empty() || authority_split[0].empty()) {
+            return _parsing_error("Invalid S3 URI", azure_provider);
+        }
+        _bucket = authority_split[0];
+        // support s3://bucket1
+        _key = authority_split.size() == 1 ? "/" : authority_split[1];
+    } else if (absl::EqualsIgnoreCase(scheme, _SCHEME_ABFS) ||
+               absl::EqualsIgnoreCase(scheme, _SCHEME_ABFSS) ||
+               absl::EqualsIgnoreCase(scheme, _SCHEME_WASB) ||
+               absl::EqualsIgnoreCase(scheme, _SCHEME_WASBS)) {
+        // Azure Data Lake paths use container@account-host as the
+        // authority. Keep the account host so the native Azure client
+        // can derive its endpoint without consulting Hadoop settings.
+        _is_azure = true;
+        std::vector<std::string> authority_split =
+                absl::StrSplit(rest, absl::MaxSplits(_PATH_DELIM, 1));
+        if (authority_split.empty() || authority_split[0].empty()) {
+            return _parsing_error("Invalid Azure URI", azure_provider);
+        }
+        const auto at = authority_split[0].find('@');
+        if (at == std::string::npos || at == 0 || at + 1 == 
authority_split[0].size() ||
+            authority_split[0].find('@', at + 1) != std::string::npos) {
+            return _parsing_error("Invalid Azure URI authority", 
azure_provider);
+        }
+        _bucket = authority_split[0].substr(0, at);
+        _endpoint = authority_split[0].substr(at + 1);
+        if (_endpoint.empty()) {
+            return _parsing_error("Invalid Azure URI authority", 
azure_provider);
+        }
+        const auto dot = _endpoint.find('.');
+        _account = dot == std::string::npos ? _endpoint : _endpoint.substr(0, 
dot);
+        _key = authority_split.size() == 1 ? "/" : authority_split[1];

Review Comment:
   [P2] Keep `?` and `#` literal in ABFS/WASB object paths
   
   This new branch captures the complete ADLS suffix here, but the common 
parsing block below still truncates `_key` at `?` and `#`. The pinned [Iceberg 
`ADLSLocation`](https://github.com/apache/iceberg/blob/apache-iceberg-1.11.0/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSLocation.java#L44-L74)
 treats everything after the authority as the literal object path, and Azure 
client libraries escape reserved blob-name characters. Consequently, 
`abfss://[email protected]/path/a?b#c.parquet` is 
currently opened as `path/a`, which can fail or read a different blob. Skip 
query/fragment stripping for the ABFS/WASB schemes and add direct-parser plus 
FileFactory coverage for both characters.



##########
be/src/util/s3_util.cpp:
##########
@@ -165,8 +234,391 @@ 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;
+}
+
+// Native protocol only: resolves the documented AZURE_ENDPOINT shorthands
+// (bare account name, official DFS host) into the Blob transport origin. 
Legacy
+// SharedKey endpoints are literal and never pass through here.
+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;
+}
+
+#ifdef USE_AZURE
+// The endpoint handed to the SDK. Native endpoints were already normalized
+// while parsing the native protocol. Legacy SharedKey producers hand over an
+// endpoint literal that only ever received a default scheme, so keep that
+// contract here: no account-name inference, no DFS-to-Blob rewrite and no path
+// rewriting, which would silently redirect single-label proxy hosts or custom
+// reverse-proxy routes after an upgrade. Only the endpoint/container join
+// boundary is normalized. Only _create_azure_client() consumes it, so keep it
+// under the same guard or a BUILD_AZURE=OFF build fails on -Wunused-function.
+std::string azure_transport_endpoint(std::string endpoint) {
+    if (endpoint.find("://") == std::string::npos) {
+        endpoint = "https://"; + endpoint;
+    }
+    while (endpoint.ends_with('/')) {
+        endpoint.pop_back();
+    }
+    return endpoint;
+}
+#endif
+
+// Only established SharedKey wire producers use AWS fields for Azure. Once
+// translated here the native factory never inspects these fields again. The
+// endpoint stays byte-for-byte as configured, exactly like the old factory.
+void import_legacy_azure_shared_key(S3ClientConf* conf) {
+    conf->azure_credentials = {};
+    conf->azure_credentials.type = AzureCredentialType::SHARED_KEY;
+    conf->azure_credentials.account_name = std::move(conf->ak);
+    conf->azure_credentials.account_key = std::move(conf->sk);
+    conf->ak.clear();
+    conf->sk.clear();
+    conf->token.clear();
+    conf->region.clear();
+    conf->role_arn.clear();
+    conf->external_id.clear();
+    conf->cred_provider_type = CredProviderType::Default;
+}
+
+Status convert_legacy_azure_properties(const StringCaseMap<std::string>& 
properties,
+                                       S3ClientConf* client_conf) {
+    auto& client = *client_conf;
+    // Compatibility is deliberately limited to the old SharedKey map. An
+    // incomplete native map must not be mistaken for that old protocol.
+    for (const auto& [key, value] : properties) {
+        const auto lower = to_lower(key);
+        if (lower.starts_with("azure") || (lower == "aws_token" && 
!value.empty())) {
+            return Status::InvalidArgument("Azure native credentials require 
AZURE_AUTH_TYPE");
+        }
+    }
+    if (!has_property(properties, {S3_ENDPOINT}) || !has_property(properties, 
{S3_AK}) ||
+        !has_property(properties, {S3_SK})) {
+        return Status::InvalidArgument("Azure native credentials require 
AZURE_AUTH_TYPE");
+    }
+    client.endpoint = *find_property(properties, {S3_ENDPOINT});
+    client.ak = *find_property(properties, {S3_AK});
+    client.sk = *find_property(properties, {S3_SK});
+    import_legacy_azure_shared_key(&client);
+    for (const auto& [name, target] :
+         {std::pair {S3_MAX_CONN_SIZE, &client.max_connections},
+          std::pair {S3_REQUEST_TIMEOUT_MS, &client.request_timeout_ms},
+          std::pair {S3_CONN_TIMEOUT_MS, &client.connect_timeout_ms}}) {
+        if (const auto* value = find_property(properties, {name}); value != 
nullptr) {
+            if (!to_int(*value, *target)) {
+                return Status::InvalidArgument("invalid Azure connection 
option {}", name);
+            }
+        }
+    }
+    return Status::OK();
+}
+
+Status validate_native_azure_shared_key_compatibility(const 
StringCaseMap<std::string>& properties,
+                                                      const 
AzureCredentialOptions& credential) {
+    const auto* legacy_endpoint = find_property(properties, {S3_ENDPOINT});
+    const auto* legacy_account = find_property(properties, {S3_AK});
+    const auto* legacy_key = find_property(properties, {S3_SK});
+    const auto* native_endpoint = find_property(properties, {AZURE_ENDPOINT});
+    const auto* legacy_override = find_property(properties, 
{"AWS_NEED_OVERRIDE_ENDPOINT"});
+    const bool has_legacy_fields =
+            legacy_endpoint != nullptr || legacy_account != nullptr || 
legacy_key != nullptr ||
+            find_property(properties, {S3_REGION}) != nullptr || 
legacy_override != nullptr;
+    if (has_legacy_fields &&
+        (legacy_endpoint == nullptr || legacy_account == nullptr || legacy_key 
== nullptr ||
+         native_endpoint == nullptr || *legacy_endpoint != *native_endpoint ||
+         *legacy_account != credential.account_name || *legacy_key != 
credential.account_key)) {
+        return Status::InvalidArgument(
+                "Azure native SharedKey compatibility fields conflict with 
native fields");
+    }
+    return Status::OK();
+}
+
+Status convert_native_azure_properties(const StringCaseMap<std::string>& 
properties,
+                                       const S3URI& uri, const std::string& 
auth_type,
+                                       S3ClientConf* client_conf) {
+    auto& client = *client_conf;
+    const auto is_legacy_shared_key_field = [&](const std::string& key) {
+        return auth_type == "SHARED_KEY" &&
+               (iequal(key, S3_ENDPOINT) || iequal(key, S3_REGION) || 
iequal(key, S3_AK) ||
+                iequal(key, S3_SK) || iequal(key, 
"AWS_NEED_OVERRIDE_ENDPOINT"));
+    };
+    for (const auto& [key, value] : properties) {
+        const auto lower = to_lower(key);
+        if ((lower.starts_with("aws_") || lower.starts_with("azure.")) &&
+            !is_legacy_shared_key_field(key)) {
+            return Status::InvalidArgument(
+                    "Azure native credentials cannot use AWS or catalog 
property aliases");
+        }
+    }
+    // Older FE versions attach a Hadoop configuration view for OneLake to
+    // this map. Ignore those extra keys; they must never supply or override
+    // any native authentication field. FE routing separates the two views.
+    auto& credential = client.azure_credentials;
+    if (auth_type == "SHARED_KEY") {
+        credential.type = AzureCredentialType::SHARED_KEY;
+    } else if (auth_type == "SAS") {
+        credential.type = AzureCredentialType::SAS;
+    } else if (auth_type == "OAUTH2") {
+        credential.type = AzureCredentialType::OAUTH2;
+    } else {
+        return Status::InvalidArgument("unsupported AZURE_AUTH_TYPE in native 
credentials");
+    }
+    auto set = [&](const char* key, std::string* target) {
+        if (const auto* value = find_property(properties, {key}); value != 
nullptr) {
+            *target = *value;
+        }
+    };
+    set(AZURE_ENDPOINT, &client.endpoint);
+    set(AZURE_ACCOUNT_NAME, &credential.account_name);
+    set(AZURE_ACCOUNT_KEY, &credential.account_key);
+    set(AZURE_SAS_TOKEN, &credential.sas_token);
+    set(AZURE_CLIENT_ID, &credential.oauth_client_id);
+    set(AZURE_CLIENT_SECRET, &credential.oauth_client_secret);
+    set(AZURE_TENANT_ID, &credential.oauth_tenant_id);
+    set(AZURE_OAUTH_SERVER_URI, &credential.oauth_server_uri);
+    if (auth_type == "SHARED_KEY") {
+        
RETURN_IF_ERROR(validate_native_azure_shared_key_compatibility(properties, 
credential));
+    }
+    if (const auto* expiry = find_property(properties, {AZURE_SAS_EXPIRY_MS}); 
expiry != nullptr) {
+        if (!to_int64(*expiry, credential.sas_expiration_time_ms) ||
+            credential.sas_expiration_time_ms <= 0) {
+            return Status::InvalidArgument("invalid Azure SAS expiry value");
+        }
+    }
+    if (client.endpoint.empty() || credential.account_name.empty()) {
+        return Status::InvalidArgument(
+                "Azure native credentials require endpoint and account name");
+    }
+    if (credential.type == AzureCredentialType::SHARED_KEY && 
credential.account_key.empty()) {
+        return Status::InvalidArgument("Azure native SharedKey requires an 
account key");
+    }
+    // Existing Azure SharedKey catalogs also contain S3-spelled locations.
+    // This explicit compatibility case carries no account in its URI;
+    // SAS/OAuth2 native data locations must carry their Azure authority.
+    if (uri.get_scheme().empty() ||
+        (uri.get_scheme() == "s3" && credential.type != 
AzureCredentialType::SHARED_KEY)) {
+        return Status::InvalidArgument("Azure native credentials require an 
Azure data URI");
+    }
+    if (client.endpoint.find_first_of("?#@\r\n") != std::string::npos) {
+        return Status::InvalidArgument(
+                "Azure endpoint must not contain credentials, query or 
fragment");
+    }
+    client.endpoint = normalize_azure_endpoint(client.endpoint);
+    if (!client.endpoint.starts_with("https://";) && 
!client.endpoint.starts_with("http://";)) {
+        return Status::InvalidArgument("Azure endpoint must use HTTP or 
HTTPS");
+    }
+    // A container property may constrain legacy callers, but is never a
+    // fallback location. Every native data URI identifies its container.
+    if (const auto* container = find_property(properties, {AZURE_CONTAINER});
+        container != nullptr && *container != uri.get_bucket()) {
+        return Status::InvalidArgument("Azure URI container conflicts with the 
storage binding");
+    }
+    return Status::OK();
+}
+
+Status convert_azure_properties(const StringCaseMap<std::string>& properties, 
const S3URI& uri,
+                                S3Conf* conf) {
+    auto& client = conf->client_conf;
+    client.provider = io::ObjStorageProvider::AZURE;
+    const auto* auth_type = find_property(properties, {AZURE_AUTH_TYPE});
+    if (auth_type == nullptr) {
+        RETURN_IF_ERROR(convert_legacy_azure_properties(properties, &client));
+    } else {
+        RETURN_IF_ERROR(convert_native_azure_properties(properties, uri, 
*auth_type, &client));
+    }
+    if (uri.get_bucket().empty()) {
+        return Status::InvalidArgument("Azure data URI requires a container");
+    }
+    conf->bucket = uri.get_bucket();
+    client.bucket = conf->bucket;
+    if (auth_type != nullptr) {
+        RETURN_IF_ERROR(S3ClientFactory::validate_azure_uri(uri, client));
+    }
+    return is_s3_conf_valid(client);
+}
 } // namespace
 
+Status S3ClientFactory::validate_azure_uri(const S3URI& uri, const 
S3ClientConf& conf) {
+    if (uri.get_scheme().empty()) {
+        return Status::OK(); // Internal file-system callers may pass a raw 
object key.
+    }
+    if (uri.get_bucket() != conf.bucket) {
+        return Status::InvalidArgument("Azure URI container conflicts with the 
storage binding");
+    }
+    if (uri.get_scheme() == "s3") {
+        if (conf.azure_credentials.type != AzureCredentialType::SHARED_KEY) {
+            return Status::InvalidArgument("Azure SAS/OAuth2 data access 
requires an Azure URI");
+        }
+        return Status::OK(); // Old SharedKey file-system paths use the S3 
wire spelling.
+    }
+    const auto uri_host = to_lower(uri.get_endpoint());
+    if (uri_host.ends_with(".dfs.fabric.microsoft.com") ||
+        uri_host.ends_with(".blob.fabric.microsoft.com")) {
+        return Status::NotSupported("OneLake data access requires its Hadoop 
storage binding");
+    }
+    const auto endpoint = normalize_azure_endpoint(conf.endpoint);
+    const bool http_uri = uri.get_scheme() == "http" || uri.get_scheme() == 
"https";
+    const auto uri_endpoint = normalize_azure_endpoint(
+            http_uri ? uri.get_scheme() + "://" + uri.get_endpoint() : 
uri.get_endpoint());
+    // ABFS/WASB authorities identify the logical Azure account, while a 
configured custom
+    // endpoint may be a proxy or emulator that intentionally has a different 
HTTP authority.
+    // The native client uses conf.endpoint as the transport origin; retain 
account validation
+    // below, but do not reject this valid proxy form. HTTP(S) locations carry 
their transport
+    // origin directly and must still match exactly.
+    const bool custom_transport = 
!S3URI::is_azure_endpoint(endpoint_authority(endpoint));
+    if (endpoint_authority(endpoint) != endpoint_authority(uri_endpoint) &&
+        !(custom_transport && !http_uri)) {
+        return Status::InvalidArgument(
+                "Azure URI account host conflicts with the storage endpoint");
+    }
+    if (http_uri && !endpoint.starts_with(uri.get_scheme() + "://")) {
+        return Status::InvalidArgument("Azure URI scheme conflicts with the 
storage endpoint");
+    }
+    if (S3URI::is_azure_endpoint(uri.get_endpoint()) &&

Review Comment:
   [P2] Allow the documented Azure secondary account endpoint
   
   For a native binding with `AZURE_ACCOUNT_NAME=myaccount` and 
`AZURE_ENDPOINT=https://myaccount-secondary.blob.core.windows.net`, this 
compares the URI label `myaccount-secondary` with the credential identity 
`myaccount` and rejects the request. Azure's [RA-GRS/RA-GZRS secondary 
endpoint](https://learn.microsoft.com/en-us/azure/storage/common/storage-redundancy#read-access-to-data-in-the-secondary-region)
 deliberately appends `-secondary` while using the same primary account 
identity and keys, so this prevents valid secondary reads. Canonicalize that 
documented suffix only for recognized Azure service authorities before this 
comparison, and add a secondary Blob-endpoint test.



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