github-actions[bot] commented on code in PR #68879: URL: https://github.com/apache/doris/pull/68879#discussion_r4247029348
########## fe/fe-filesystem/fe-filesystem-spi/src/main/java/org/apache/doris/filesystem/spi/EndpointHosts.java: ########## @@ -0,0 +1,61 @@ +// 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; + +/** + * 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 { Review Comment: [P2] Bump the filesystem plugin API major for this new SPI class. `EndpointHosts` is supplied by the FE host because plugin zips exclude the provided `fe-filesystem-spi` jar. New S3/COS/OBS/OSS/MinIO providers call it during URI guessing, but `filesystem.plugin.api.version` remains `1.0`. An older 1.0 FE therefore admits these new plugin jars and `bindPrimary`/`bindAll` fails with `NoClassDefFoundError` on the URI fallback. Increment the API major so the existing gate rejects the incompatible deployment. ########## fe/fe-filesystem/fe-filesystem-cos/src/main/java/org/apache/doris/filesystem/cos/CosFileSystemProvider.java: ########## @@ -51,7 +52,10 @@ public boolean supports(Map<String, String> properties) { return true; } String endpoint = firstPresent(properties, ENDPOINT_NAMES); - return endpoint != null && endpoint.contains("myqcloud.com"); + // The endpoint is a DNS host: classify case-insensitively so an upper/mixed case spelling + // routes exactly like its lowercase form (the legacy COSProperties.guessIsMe compared + // case-sensitively). + return endpoint != null && StringUtils.containsIgnoreCase(endpoint, "myqcloud.com"); Review Comment: [P2] Restrict endpoint-alias markers to the endpoint host. For an S3 resource with `s3.endpoint=https://gateway.internal/proxy/MYQCLOUD.COM`, an explicit region and paired S3 credentials, `S3Resource.pingS3` uses raw `FileSystemFactory.getFileSystem(Map)`. With COS loaded before S3 (the normal alphabetic plugin-directory order), this new whole-URL check claims COS solely from the proxy path; native COS then rejects the endpoint suffix. Before this change COS declined the map and generic S3 preserved the gateway URL, including its supported base path. Apply the host extraction used by the URI fallback to endpoint aliases in `supports`/`supportsGuess` and the parallel cloud providers, and cover raw resource selection with a marker only in the endpoint path. ########## fe/fe-filesystem/fe-filesystem-cos/src/main/java/org/apache/doris/filesystem/cos/CosFileSystemProvider.java: ########## @@ -76,14 +80,15 @@ public boolean supportsGuess(Map<String, String> properties) { String value = properties.get(name); if (value != null) { if (!value.isEmpty()) { - return value.contains("myqcloud.com"); + return StringUtils.containsIgnoreCase(value, "myqcloud.com"); Review Comment: [P2] Normalize the COS endpoint before constructing the native client. With `cos.endpoint=COS.AP-SHANGHAI.MYQCLOUD.COM`, this guess now selects COS and binding derives `ap-shanghai`, but the holder preserves the uppercase endpoint. `CosObjStorage.buildCosClient` forwards it unchanged to `setEndPointSuffix`; the pinned `cos_api` 5.6.211 [SDK validator](https://raw.githubusercontent.com/tencentyun/cos-java-sdk-v5/v5.6.211/src/main/java/com/qcloud/cos/internal/UrlComponentUtils.java) rejects uppercase endpoint suffixes, so the first filesystem operation fails although lowercase works. Normalize the effective COS host and cover native client/config creation, not only `supportsGuess`. -- 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]
