github-actions[bot] commented on code in PR #64695:
URL: https://github.com/apache/doris/pull/64695#discussion_r3555929334
##########
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:
This scheme check breaks the legacy mixed-scheme fallback that
`LocationPath` still implements. For example, if a catalog has only an
S3-compatible storage config but a path comes in as `cos://bucket/key`,
`LocationPath.findStorageProperties` documents that it should fall back to an
available storage whose name is `s3`, and
`S3PropertyUtils.validateAndNormalizeUri` can normalize compatible schemes to
`s3://...`. However `SpiSwitchingFileSystem` only uses that result to
pick/cache the filesystem; the actual operation still passes the original
`Location` URI to the selected S3 filesystem. Before this change the parser
accepted that URI and extracted the same bucket/key. Now the S3 filesystem
passes `Set.of("s3", "s3a", "s3n")` here, so line 99 throws `Unsupported scheme
'cos'` before the request reaches the client. The same applies to the
documented `oss://`/`obs://` compatibility cases. Please either keep this
parser lenient for delegated S3-compatible filesystems or make the fallb
ack rewrite the `Location` URI before the filesystem sees it.
--
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]
