CalvinKirs commented on code in PR #64695:
URL: https://github.com/apache/doris/pull/64695#discussion_r3586489434
##########
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)) {
+ throw new IllegalArgumentException("Unsupported scheme '" +
scheme
Review Comment:
All four confirmed and fixed in 3a8448e9fd6, with one correction on the
broker-load framing: the legacy non-Nereids producer no longer exists
(`BulkLoadJob.fromLoadCommand` is the only entry), so a fresh submission is
fine — the real gap is the FE-restart/master-failover replay:
`fileGroupAggInfo` is not persisted, `BulkLoadJob.analyze()` re-parses
originStmt and skips the normalization loop in `LoadCommand.run()`, so a
pending job with a compatibility URI was rescheduled raw and cancelled.
Normalization is now re-applied in `analyzeCommand` (the replay funnel).
The rest as suggested: `InsertIntoTVFCommand.deleteExistingFilesInFE`
normalizes `file_path` via the `StorageProperties` it already builds;
`Repository.ping()` uses `getLocation()` like every other repo path assembly
(the raw field also made the periodic `RepositoryMgr` ping throw unchecked and
skip remaining repos); `AcidUtil.getAcidState` now globs on the partition
location normalized by the same `LocationPath` resolution that selected the
filesystem — via a new overload so the 10 existing test call sites keep the old
signature — which also keeps BE-facing `AcidInfo` locations consistent with the
non-transactional listing path (which already passed `getNormalizedLocation()`).
--
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]
