CalvinKirs commented on code in PR #64695:
URL: https://github.com/apache/doris/pull/64695#discussion_r3575896430
##########
fe/fe-filesystem/fe-filesystem-spi/src/main/java/org/apache/doris/filesystem/spi/ObjectStorageUri.java:
##########
@@ -59,6 +66,45 @@ private ObjectStorageUri(String bucket, String key) {
* original (escaped) form.
*/
public static ObjectStorageUri parse(String path, boolean pathStyleAccess)
{
+ return parse(path, pathStyleAccess, null);
+ }
+
+ /**
+ * Same as {@link #parse(String, boolean)} but validates the URI scheme
against the set of
+ * schemes the calling provider accepts. A scheme not in {@code
supportedSchemes} (matched
+ * case-insensitively) is rejected, so e.g. a COS provider refuses an
{@code oss://} URI.
+ * Passing {@code null} or an empty set skips scheme validation.
+ *
+ * <p>{@code http}/{@code https} are always accepted regardless of {@code
supportedSchemes}:
+ * they are the endpoint-URL form (e.g. a TVF such as {@code s3()} pointed
at a path-style
+ * endpoint), not a provider-identity scheme. {@code supportedSchemes}
therefore stays explicit
+ * (e.g. {@code s3}/{@code oss}/{@code cos}) for catalog scheme-to-storage
routing and omits
+ * {@code http}/{@code https} on purpose.
+ */
+ public static ObjectStorageUri parse(String path, boolean pathStyleAccess,
Set<String> supportedSchemes) {
+ if (path == null) {
+ throw new IllegalArgumentException("Object storage path must not
be null");
+ }
+ if (supportedSchemes != null && !supportedSchemes.isEmpty()) {
+ int schemeEnd = path.indexOf("://");
+ if (schemeEnd < 0) {
+ throw new IllegalArgumentException("Cannot parse object
storage URI without scheme: " + path);
+ }
+ String scheme = path.substring(0, schemeEnd).toLowerCase();
+ // http/https is the endpoint-URL form (e.g. TVF s3() with a
path-style endpoint),
+ // not a provider-identity scheme, so it bypasses the
supportedSchemes check. The set
+ // stays explicit (s3/oss/cos/...) for catalog scheme-to-storage
routing; the actual
+ // bucket/key extraction for http(s) is handled by the path-style
branch in doParse.
+ boolean httpEndpoint = scheme.equals("http") ||
scheme.equals("https");
+ if (!httpEndpoint && !supportedSchemes.contains(scheme)) {
Review Comment:
Fixed in 01a5d504072. The legacy mixed-scheme fallback is now handled at the
switching boundary instead of loosening the parser: when `LocationPath`'s
compatibility fallback fires (path scheme type != selected storage type) and
the normalization is a pure scheme-prefix swap, `SpiSwitchingFileSystem`
delegates the operation with the storage's native scheme (`cos://x` ->
`s3://x`) and translates all returned paths (listing entries,
`listDirectories`, `GlobListing`, input/output file locations) back to the
caller's original scheme. Concrete filesystems only ever see schemes they
whitelist, while callers keep comparing paths against HMS/Iceberg metadata
unchanged. Direct-match routing and same-scheme fallbacks (MinIO/Ozone) are
unaffected. Covered by new unit tests in `SpiSwitchingFileSystemTest`.
--
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]
