sunchao commented on code in PR #5872:
URL: https://github.com/apache/datafusion-comet/pull/5872#discussion_r3997395231
##########
native/core/src/parquet/objectstore/s3.rs:
##########
@@ -239,24 +240,25 @@ fn extract_s3_config_options(
s3_configs.insert(AmazonS3ConfigKey::Region, region.to_string());
}
- // Extract and handle path style access (virtual hosted style)
- let mut virtual_hosted_style_request = false;
- if let Some(path_style) = get_config_trimmed(configs, bucket,
"path.style.access") {
- virtual_hosted_style_request = path_style.to_lowercase() == "true";
- s3_configs.insert(
- AmazonS3ConfigKey::VirtualHostedStyleRequest,
- virtual_hosted_style_request.to_string(),
- );
- }
+ // Hadoop defaults fs.s3a.path.style.access to false, which means
virtual-hosted addressing,
+ // and treats non-boolean text as that default. object_store expects the
inverse flag.
+ let path_style_access = get_config_trimmed(configs, bucket,
"path.style.access")
+ .is_some_and(|value| value.eq_ignore_ascii_case("true"));
+ let mut virtual_hosted_style_request = !path_style_access;
Review Comment:
### Correctness
[P2] Preserve path-style addressing for dotted HTTPS buckets
Could we apply the AWS SDK's virtual-host eligibility rules before enabling
this flag? With `fs.s3a.endpoint.region=us-east-1` and `path.style.access`
unset or false, a bucket such as `review.dotted.bucket` now becomes
`https://review.dotted.bucket.s3.us-east-1.amazonaws.com`. BASE used path-style
addressing, and the AWS SDK endpoint resolver still selects
`https://s3.us-east-1.amazonaws.com/review.dotted.bucket` for this case. The
dotted host does not match S3's wildcard TLS certificate, so this breaks native
reads of otherwise valid buckets. The same eligibility issue exists for custom
HTTPS endpoints. Please retain path-style addressing for dotted HTTPS buckets,
including when no custom endpoint is configured, and add assertions on the
resulting request URL. The current dotted-bucket test asserts the
virtual-hosted string, which misses this regression.
--
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]
