github-actions[bot] commented on code in PR #68879: URL: https://github.com/apache/doris/pull/68879#discussion_r4268647378
########## fe/fe-filesystem/fe-filesystem-spi/src/main/java/org/apache/doris/filesystem/spi/EndpointHosts.java: ########## @@ -0,0 +1,97 @@ +// 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. + +package org.apache.doris.filesystem.spi; + +import java.util.Locale; + +/** + * Host extraction for endpoint / object-storage URI marker matching. + * + * <p>Cloud dialect detection keys on host markers such as {@code oss-dls.aliyuncs.com}, + * {@code myqcloud.com} or {@code amazonaws.com}. Those markers are host names, so they may only be + * matched against the host part: a URI whose object key happens to contain the same text (for + * example {@code s3://bucket/archive/AMAZONAWS.COM/file}) does not point at that cloud's storage. + * + * <p>Deliberately string based rather than {@code java.net.URI} based: a value such as + * {@code oss://bucket/my dir/...} does not parse as a URI, and a failed parse must not silently + * turn a real endpoint into "no match". + */ +public final class EndpointHosts { + + private EndpointHosts() { + } + + /** + * Returns the host part of {@code endpointOrUri}: everything after {@code scheme://}, up to the + * first {@code /} that starts the path, with any {@code userinfo@} removed. A value without a + * scheme is a bare host, so it is returned as written (minus any path/port unaffected parts). + */ + public static String hostOf(String endpointOrUri) { + if (endpointOrUri == null) { + return ""; + } + String value = endpointOrUri.trim(); + int authorityStart = authorityStart(value); + // The authority ends at the first path, query or fragment delimiter: an '@' inside a fragment + // (e.g. host#contact@other) must not be taken for userinfo and discard the real host. + String host = value.substring(authorityStart, authorityEnd(value, authorityStart)); + int userInfoIndex = host.lastIndexOf('@'); + if (userInfoIndex >= 0) { + host = host.substring(userInfoIndex + 1); + } + return host; + } + + /** + * Returns {@code endpointOrUri} with the scheme and the authority (userinfo, host and port) folded + * to lower case, leaving the path, the query and the fragment untouched. + * + * <p>Only the host is case insensitive. A custom endpoint may carry a case sensitive base path + * (e.g. {@code https://gateway.internal/Proxy}); folding the whole value would retarget it to a + * different namespace. Callers that must hand a canonical host to a case sensitive SDK therefore + * fold the host only. + */ + public static String lowercaseSchemeAndHost(String endpointOrUri) { + if (endpointOrUri == null) { + return ""; + } + String value = endpointOrUri.trim(); + int authorityStart = authorityStart(value); + int authorityEnd = authorityEnd(value, authorityStart); + return value.substring(0, authorityStart).toLowerCase(Locale.ROOT) + + value.substring(authorityStart, authorityEnd).toLowerCase(Locale.ROOT) + + value.substring(authorityEnd); + } + + /** Index just after {@code scheme://}, or 0 when the value carries no scheme. */ + private static int authorityStart(String value) { + int schemeIndex = value.indexOf("://"); + return schemeIndex >= 0 ? schemeIndex + 3 : 0; Review Comment: [P2] Recognize a scheme only at the start of the endpoint For `INTO OUTFILE 's3://bucket/result'` using a custom gateway endpoint `s3.us-west-2.amazonaws.com.gateway.internal/Proxy/https://minio.internal`, `s3.region=us-west-2`, `s3.credentials_provider_type=INSTANCE_PROFILE`, and `use_path_style=true`, this search mistakes the path's `https://` for a leading scheme. `hostOf` returns `minio.internal`, so S3 declines the map and MinIO binding rejects its AWS-only credential mode. The base selects S3 from the actual gateway authority; the BE [SDK prefixes the schemeless value](https://github.com/aws/aws-sdk-cpp/blob/1.11.221/src/aws-cpp-sdk-core/source/endpoint/BuiltInParameters.cpp#L13-L23) and its [custom endpoint rules preserve the base path](https://github.com/aws/aws-sdk-cpp/blob/1.11.221/generated/src/aws-cpp-sdk-s3/source/S3EndpointRules.cpp), so that configuration can reach the gateway. Recognize a valid leading scheme before the first path/query/fragment delimiter, and add a binding test with an AWS-only credential mode. Unlike th e existing fragment-`@` and marker-in-path findings, this discards a real leading cloud host through authority-start detection; the nested path contains no cloud marker. ########## fe/fe-filesystem/fe-filesystem-s3/src/main/java/org/apache/doris/filesystem/s3/S3FileSystemProvider.java: ########## @@ -125,11 +126,15 @@ public boolean supportsGuess(Map<String, String> properties) { } } if (endpoint != null && !endpoint.isBlank()) { - return endpoint.contains("amazonaws.com"); + // The marker is a host name: match it in the endpoint host only (and case-insensitively), + // so marker text inside a proxy/base path cannot claim the map for S3. Legacy compared the + // whole value case-sensitively. + return StringUtils.containsIgnoreCase(EndpointHosts.hostOf(endpoint), "amazonaws.com"); Review Comment: [P2] Normalize S3 endpoint schemes before exporting them to BE For `INTO OUTFILE 's3://bucket/result'` with `s3.endpoint=HTTPS://S3.US-EAST-1.AMAZONAWS.COM` and paired S3 credentials, this guess now binds S3 and derives `us-east-1`, but `S3FileSystemProperties` leaves the effective endpoint unchanged. `StorageAdapter`/`OutFileClause` send it verbatim as `AWS_ENDPOINT`; BE enables endpoint override by default. The [pinned SDK's `OverrideEndpoint`](https://github.com/aws/aws-sdk-cpp/blob/1.11.221/src/aws-cpp-sdk-core/source/endpoint/BuiltInParameters.cpp#L13-L23) accepts only lowercase `http://` or `https://` and otherwise prepends the configured scheme, producing `https://HTTPS://S3.US-EAST-1.AMAZONAWS.COM` under the default BE configuration. Export therefore cannot reach the intended S3 endpoint although the lowercase spelling works. Normalize S3's effective scheme/host as in COS/OBS/OSS and cover backend endpoint emission, not only guessing/region output. This is a BE S3 failure distinct from the existing OBS native-SDK and S3 locale finding s. -- 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]
