github-actions[bot] commented on code in PR #68879:
URL: https://github.com/apache/doris/pull/68879#discussion_r4244469030
##########
fe/fe-filesystem/fe-filesystem-cos/src/main/java/org/apache/doris/filesystem/cos/CosFileSystemProvider.java:
##########
@@ -76,14 +79,14 @@ 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");
}
break;
}
}
for (Map.Entry<String, String> entry : properties.entrySet()) {
if ("uri".equalsIgnoreCase(entry.getKey()) && entry.getValue() !=
null) {
- return entry.getValue().contains("myqcloud.com");
+ return StringUtils.containsIgnoreCase(entry.getValue(),
"myqcloud.com");
Review Comment:
[P2] Match the cloud marker against the URI host here and in the parallel
OBS fallback. For `minio.endpoint=http://minio.internal:9000` plus
`uri=s3://bucket/archive/MYQCLOUD.COM/data`, this new check selects COS from
the key text; COS precedes MinIO in `bindPrimary` and binding fails because the
MinIO endpoint is not a COS alias and an `s3://` URI supplies no endpoint. OBS
has the same failure with `MYHUAWEICLOUD.COM`. Cover key-only markers in
provider-selection tests.
##########
fe/fe-filesystem/fe-filesystem-oss/src/main/java/org/apache/doris/filesystem/oss/OssFileSystemProvider.java:
##########
@@ -137,13 +137,14 @@ private static String firstNonNull(Map<String, String>
properties, String[] name
}
private static boolean isKnownOssLocation(String value) {
- if (value == null || value.contains("oss-dls.aliyuncs")) {
+ if (value == null || StringUtils.containsIgnoreCase(value,
"oss-dls.aliyuncs")) {
return false;
}
- if (value.startsWith("oss://")) {
+ if (StringUtils.startsWithIgnoreCase(value, "oss://")) {
return true;
}
- return value.contains("aliyuncs.com") && (value.contains("oss-") ||
value.contains("s3."));
+ return StringUtils.containsIgnoreCase(value, "aliyuncs.com")
Review Comment:
[P2] Check the URI host when guessing OSS from a location. With
`uri=https://my-bucket.s3.us-east-1.amazonaws.com/archive/OSS-CN-BEIJING.ALIYUNCS.COM/data`
and `s3.region=us-east-1`, this new case-insensitive whole-URI search sees
only the object key's OSS text, yet `bindPrimary` picks OSS before S3. OSS then
rewrites the AWS endpoint to `oss-us-east-1-internal.aliyuncs.com`, so the
storage binding points at the wrong service. Limit the marker checks to the
authority and add a key-only negative case.
##########
fe/fe-filesystem/fe-filesystem-s3/src/main/java/org/apache/doris/filesystem/s3/S3FileSystemProvider.java:
##########
@@ -125,11 +125,14 @@ public boolean supportsGuess(Map<String, String>
properties) {
}
}
if (endpoint != null && !endpoint.isBlank()) {
- return endpoint.contains("amazonaws.com");
+ // The endpoint is a DNS host: classify case-insensitively so an
upper/mixed case
+ // spelling routes exactly like its lowercase form (intentional
deviation from the
+ // legacy S3Properties.guessIsMe, which compared case-sensitively).
+ return StringUtils.containsIgnoreCase(endpoint, "amazonaws.com");
}
for (Map.Entry<String, String> entry : properties.entrySet()) {
if ("uri".equalsIgnoreCase(entry.getKey()) && entry.getValue() !=
null) {
- return entry.getValue().contains("amazonaws.com");
+ return StringUtils.containsIgnoreCase(entry.getValue(),
"amazonaws.com");
Review Comment:
[P2] Restrict the AWS marker check to the URI authority. A MinIO map with
`minio.endpoint=http://minio.internal:9000`, `AWS_REGION=us-east-1`, and
`uri=s3://bucket/archive/AMAZONAWS.COM/data` now makes this S3 guess true
solely because of the object key. `bindPrimary` checks S3 before MinIO and the
S3 binder builds an AWS endpoint, so requests leave the configured MinIO
service. Test provider selection with a key-only uppercase marker.
##########
fe/fe-filesystem/fe-filesystem-s3/src/main/java/org/apache/doris/filesystem/s3/S3FileSystemProvider.java:
##########
@@ -125,11 +125,14 @@ public boolean supportsGuess(Map<String, String>
properties) {
}
}
if (endpoint != null && !endpoint.isBlank()) {
- return endpoint.contains("amazonaws.com");
+ // The endpoint is a DNS host: classify case-insensitively so an
upper/mixed case
+ // spelling routes exactly like its lowercase form (intentional
deviation from the
+ // legacy S3Properties.guessIsMe, which compared case-sensitively).
+ return StringUtils.containsIgnoreCase(endpoint, "amazonaws.com");
Review Comment:
[P2] Make S3 region extraction locale-independent for the uppercase
endpoints admitted here. With
`s3.endpoint=HTTPS://S3.IL-CENTRAL-1.AMAZONAWS.COM` and no explicit region,
this new guess selects S3, but under JVM locale `tr_TR`,
`S3FileSystemProperties.extractRegion` lowercases `IL` to dotless `ıl`; its
ASCII region regex then fails and binding throws `Region is not set`. The
lowercase equivalent binds. Use `Locale.ROOT` in the extractor and test
endpoint-only binding under that locale.
##########
fe/fe-core/src/main/java/org/apache/doris/common/util/LocationPath.java:
##########
@@ -354,7 +354,8 @@ private static String encodedLocation(String location) {
public static boolean isHdfsOnOssEndpoint(String location) {
// example: cn-shanghai.oss-dls.aliyuncs.com contains the
"oss-dls.aliyuncs".
//
https://www.alibabacloud.com/help/en/e-mapreduce/latest/oss-kusisurumen
- return location.contains("oss-dls.aliyuncs");
+ // The marker is a DNS hostname, so the match is case-insensitive.
+ return StringUtils.containsIgnoreCase(location, "oss-dls.aliyuncs");
Review Comment:
[P2] Match the OSS-DLS marker only in the URI authority. For a normal object
such as `s3://bucket/archive/OSS-DLS.ALIYUNCS.COM/data.parquet`, the new
case-insensitive whole-location check is true even though the host is just
`bucket`. `fromSchemaWithContext` then selects `OSS_HDFS`, and
`getTFileTypeForBE` sends `FILE_HDFS` to the BE scan range, so a valid S3 key
is read through the HDFS path. Parse the host/authority before matching and
cover a marker in the key with a negative test.
##########
fe/fe-filesystem/fe-filesystem-minio/src/main/java/org/apache/doris/filesystem/minio/MinioFileSystemProvider.java:
##########
@@ -131,8 +131,11 @@ public boolean supportsGuess(Map<String, String>
properties) {
// region keys — replicate both exclusion legs.
for (Map.Entry<String, String> entry : properties.entrySet()) {
if ("uri".equalsIgnoreCase(entry.getKey()) && entry.getValue()
!= null) {
- // case-sensitive contains, byte-identical to legacy
S3Properties.guessIsMe
- if (entry.getValue().contains("amazonaws.com")) {
+ // The uri host is a DNS name: match case-insensitively,
deliberately deviating
+ // from legacy S3Properties.guessIsMe's case-sensitive
contains. With the legacy
+ // behaviour an upper-case AWS uri was not excluded here,
so MinIO silently
+ // claimed a map that belongs to S3.
+ if
(entry.getValue().toLowerCase(java.util.Locale.ROOT).contains("amazonaws.com"))
{
Review Comment:
[P2] Check the URI host in MinIO's AWS exclusion. For
`uri=https://bucket.minio.internal/archive/AMAZONAWS.COM/file` with paired
`minio.*` credentials and no endpoint alias, MinIO previously derived
`minio.internal` and bound with its default region. This new whole-URI check
rejects it solely because the object key contains uppercase AWS text; S3's
separate guess also claims and fails binding. Even after fixing the S3 guess,
MinIO would still have no claimant here. Compare the authority and add a
key-only negative test.
##########
fe/fe-filesystem/fe-filesystem-oss-hdfs/src/main/java/org/apache/doris/filesystem/hdfs/properties/OssHdfsProperties.java:
##########
@@ -189,7 +194,8 @@ public static boolean guessIsMe(Map<String, String> props) {
}
return OSS_ENDPOINT_KEY_NAME.stream()
.map(props::get)
- .anyMatch(ep -> StringUtils.isNotBlank(ep) &&
ep.endsWith(OSS_HDFS_ENDPOINT_SUFFIX));
+ .anyMatch(ep -> StringUtils.isNotBlank(ep)
+ &&
ep.toLowerCase(Locale.ROOT).endsWith(OSS_HDFS_ENDPOINT_SUFFIX));
Review Comment:
[P2] Use locale-independent folding in the region extractor as well. Under a
Turkish JVM default locale, this new `Locale.ROOT` suffix check recognizes
`oss.endpoint=CN-BEIJING.OSS-DLS.ALIYUNCS.COM`, but binding without an explicit
region calls `extractRegion`, which uses default-locale `toLowerCase()`. Its
dotless `ı` output cannot match the ASCII OSS-DLS regex, so the mixed-case
endpoint fails while the lowercase spelling works. Use `Locale.ROOT` there and
cover that locale in a 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]
