CalvinKirs commented on code in PR #66004: URL: https://github.com/apache/doris/pull/66004#discussion_r3654548731
########## fe/fe-core/src/main/java/org/apache/doris/datasource/storage/StorageUriUtils.java: ########## @@ -0,0 +1,370 @@ +// 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.datasource.storage; + +import org.apache.doris.common.UserException; +import org.apache.doris.common.util.S3URI; +import org.apache.doris.foundation.property.StoragePropertiesException; + +import org.apache.commons.lang3.StringUtils; + +import java.io.UnsupportedEncodingException; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URLDecoder; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; +import java.util.Map; +import java.util.Optional; +import java.util.regex.Pattern; + +/** + * fe-core-only URI validation/normalization glue for {@link StorageAdapter}. + * + * <p>Ports the URI subset of the legacy {@code S3PropertyUtils} and {@code AzurePropertyUtils} + * pure functions so the facade does not depend on the to-be-deleted + * {@code datasource.property.storage} package. Behavior is kept identical, with one deliberate + * exception: the legacy full-parse path could throw the checked {@code UserException} (from + * {@link S3URI#create}); the facade surface is unchecked, so that exception is rethrown as + * {@link StoragePropertiesException} with the same message.</p> + */ +public final class StorageUriUtils { + + private static final String URI_KEY = "uri"; + private static final String SCHEME_DELIM = "://"; + private static final String S3_SCHEME_PREFIX = "s3://"; + + // S3-compatible schemes that can be converted to s3:// with simple string replacement + // Format: scheme://bucket/key -> s3://bucket/key + private static final String[] SIMPLE_S3_COMPATIBLE_SCHEMES = { + "s3a", "s3n", "oss", "cos", "cosn", "obs", "bos", "gs" + }; + + private static final Pattern ONELAKE_PATTERN = Pattern.compile( + "abfs[s]?://([^@]+)@([^/]+)\\.dfs\\.fabric\\.microsoft\\.com(/.*)?", Pattern.CASE_INSENSITIVE); + + private StorageUriUtils() { + } + + /** + * Port of legacy {@code S3PropertyUtils.validateAndNormalizeUri}: validates and normalizes + * the given path into a standard {@code s3://bucket/key} URI. + * + * <p>Also carries the legacy {@code OSSProperties.validateAndNormalizeUri} override: OSS + * bindings first rewrote a virtual-hosted {@code scheme://bucket.endpoint/key} authority to + * {@code scheme://bucket/key} ({@code rewriteOssBucketIfNecessary}) before the base + * normalization, so an OSS bucket-domain URI under a non-http scheme never reached BE or the + * filesystem with the whole host as the bucket. The facade has no provider key at this call + * site (StorageAdapter routes every S3-compatible binding here), so the rewrite is gated on + * the URI shape instead — see {@link #rewriteOssBucketIfNecessary(String)}.</p> + */ + public static String validateAndNormalizeS3Uri(String path, + String stringUsePathStyle, + String stringForceParsingByStandardUri) { + if (StringUtils.isBlank(path)) { + throw new StoragePropertiesException("path is null"); + } + + // Legacy OSSProperties applied the bucket-domain rewrite before the base normalization. + path = rewriteOssBucketIfNecessary(path); Review Comment: Fixed. The rewrite is now provider-gated instead of shape-gated: `StorageAdapter` passes `"OSS".equals(providerKey)` into `StorageUriUtils.validateAndNormalizeS3Uri`, and the rewrite body is the legacy `rewriteOssBucketIfNecessary` verbatim (any dotted authority chopped for OSS bindings only; the aliyun-shape heuristic and its two divergence corners are gone). `StorageUriUtilsTest` freezes both directions: OSS bindings chop `bucket.endpoint` authorities (aliyun-shaped or custom), while a dotted bucket name under an explicit non-OSS binding (`oss://logs.prod/…` with `fs.s3.support=true`, `s3://my.bucket.name/…`) passes through unchanged. ########## fe/fe-filesystem/fe-filesystem-minio/src/main/java/org/apache/doris/filesystem/minio/MinioFileSystemProvider.java: ########## @@ -65,6 +65,83 @@ public FileSystem create(MinioFileSystemProperties properties) throws IOExceptio new S3ObjStorage(delegate, properties.getSupportedSchemes())); } + private static final String[] MINIO_GUESS_IDENTIFIERS = { + "minio.access_key", "AWS_ACCESS_KEY", "ACCESS_KEY", "access_key", "s3.access_key", + "minio.endpoint", "s3.endpoint", "AWS_ENDPOINT", "endpoint", "ENDPOINT"}; + private static final String[] MINIO_GUESS_ENDPOINT_NAMES = { + "minio.endpoint", "s3.endpoint", "AWS_ENDPOINT", "endpoint", "ENDPOINT"}; + // Endpoint markers of the clouds whose guessIsMe fe-core's MinioProperties.guessIsMe + // consults for mutual exclusion (Azure/COS/OSS/S3, plus GCS which the dialect split adds). + private static final String[] OTHER_CLOUD_ENDPOINT_MARKERS = { + "aliyuncs.com", "myqcloud.com", "amazonaws.com", "storage.googleapis.com", + ".blob.core.windows.net", ".dfs.core.windows.net", ".blob.core.chinacloudapi.cn"}; + private static final String[] S3_GUESS_REGION_NAMES = { + "s3.region", "glue.region", "aws.glue.region", "iceberg.rest.signing-region", + "rest.signing-region", "client.region"}; + + @Override + public boolean supportsExplicit(Map<String, String> properties) { + return S3CompatSignals.isFsSupport(properties, FS_MINIO_SUPPORT); + } + + @Override + public boolean supportsGuess(Map<String, String> properties) { + // Verbatim port of fe-core MinioProperties.guessIsMe: MinIO is the "any other + // S3-compatible" fallback — it claims the map iff none of Azure/COS/OSS/S3 would claim + // it AND at least one identifier key is present. The sibling guesses are replicated + // here (plugins cannot call each other's classes), preserving legacy control flow + // EXACTLY: legacy S3.guessIsMe SHORT-CIRCUITS on a present endpoint alias (claiming + // only an amazonaws endpoint), and consults its uri/region fallbacks ONLY when no + // endpoint alias exists. A present non-cloud endpoint (e.g. a private MinIO address) + // therefore stays MinIO's even when an s3.region key is also set. + if ("azure".equalsIgnoreCase(properties.get(S3CompatSignals.PROVIDER_KEY))) { + return false; + } + String endpoint = null; + for (String name : MINIO_GUESS_ENDPOINT_NAMES) { + String value = properties.get(name); + if (value != null && !value.isBlank()) { + endpoint = value; + break; + } + } + if (endpoint != null) { + String lower = endpoint.toLowerCase(java.util.Locale.ROOT); + for (String marker : OTHER_CLOUD_ENDPOINT_MARKERS) { Review Comment: Fixed. The Azure exclusion in MinIO's guess now goes through a shared predicate, `AzureBlobEndpointSignals` (fe-filesystem-spi — the module both providers depend on): Azure's endpoint-alias order, legacy host extraction (URI parse / path+port strip), lowercase + dot-anchored `endsWith`, and the probe-injected live `Config.azure_blob_host_suffixes` list (`_AZURE_HOST_SUFFIXES_`), with the 8-entry builtin default. `AzureFileSystemProvider.supportsGuess` uses the same predicate, so the two can no longer drift — whatever Azure claims, MinIO yields (mirroring legacy `MinioProperties.guessIsMe` consulting `AzureProperties.guessIsMe`). Tests cover the gov-cloud dfs endpoint, probe-injected custom suffixes, and a `bindAll` routing case asserting exactly one AZURE binding and no MINIO. -- 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]
