github-actions[bot] commented on code in PR #69527:
URL: https://github.com/apache/doris/pull/69527#discussion_r4370538558


##########
fe/be-java-extensions/hadoop-deps/src/main/java/org/apache/hadoop/fs/FileSystem.java:
##########
@@ -3622,16 +3640,768 @@ public static Class<? extends FileSystem> 
getFileSystemClass(String scheme,
    * @return the initialized filesystem.
    * @throws IOException problems loading or initializing the FileSystem
    */
+  // DORIS-PATCH: TVF policy is checked against the frozen configuration used 
by the consumer.
+  public static int getDorisTvfGuardVersion() {
+    return 1;
+  }
+
+  public static final class TvfGuard {
+    public static final String POLICY_KEY = "fs.doris.tvf.remote.paths";
+    public static final String NATIVE_POLICY = "native";
+    public static final String NATIVE_WRITE_POLICY = "native-write";
+    public static final String JAVA_READ_POLICY = "java-read";
+    public static final String ROOT_KEY = "fs.doris.tvf.root";
+    public static final String PATH_KEY = "fs.doris.tvf.path";
+    public static final String CONTEXT_KEY = "fs.doris.tvf.context";
+    private static final ThreadLocal<Context> LAST_NATIVE_CONTEXT = new 
ThreadLocal<>();
+
+    private TvfGuard() {
+    }
+
+    public static final class Rule {
+      private final String filesystem;
+      private final String path;
+      private final boolean descendants;
+
+      public Rule(String filesystem, String path, boolean descendants) {
+        this.filesystem = filesystem;
+        this.path = path;
+        this.descendants = descendants;
+      }
+    }
+
+    public static String encode(List<Rule> rules) {
+      if (FileSystem.getDorisTvfGuardVersion() != 1) {
+        throw new IllegalStateException("TVF Hadoop runtime policy version 
mismatch");
+      }
+      try {
+        java.io.ByteArrayOutputStream bytes = new 
java.io.ByteArrayOutputStream();
+        java.io.DataOutputStream out = new java.io.DataOutputStream(bytes);
+        out.writeInt(1);
+        out.writeInt(rules.size());
+        for (Rule rule : rules) {
+          out.writeUTF(rule.filesystem);
+          out.writeUTF(rule.path);
+          out.writeBoolean(rule.descendants);
+        }
+        out.flush();
+        return 
java.util.Base64.getEncoder().encodeToString(bytes.toByteArray());
+      } catch (IOException e) {
+        throw new IllegalArgumentException("Cannot encode TVF Hadoop route 
policy", e);
+      }
+    }
+
+    /** Literal probes and subtree prefixes touched by Globber, without 
filesystem IO. */
+    public static List<Rule> globRules(URI filesystem, String pattern) throws 
IOException {
+      List<Rule> rules = new ArrayList<>();
+      for (String expanded : GlobExpander.expand(pattern)) {
+        Path flattened = new Path(expanded.isEmpty() ? Path.CUR_DIR : 
expanded);
+        String path = flattened.toUri().getPath();
+        Path root = new Path(flattened.isAbsolute() ? "/" : Path.CUR_DIR);
+        Path candidate = root;
+        List<String> components = new ArrayList<>();
+        List<Boolean> patterned = new ArrayList<>();
+        for (String component : path.split("/")) {
+          if (component.isEmpty()) {
+            continue;
+          }
+          GlobFilter filter = new GlobFilter(component);
+          if (filter.hasPattern()) {
+            // Keep every real listing parent, even if later escaped '..' 
removes it.
+            rules.add(new Rule(filesystem.toString(),
+                globLiteralPrefix(root, components, 
patterned).toUri().getPath(), true));
+          }
+          component = component.replaceAll("\\\\(.)", "$1");
+          candidate = new Path(candidate, component);
+          if ("..".equals(component) && !components.isEmpty()
+              && !"..".equals(components.get(components.size() - 1))) {
+            components.remove(components.size() - 1);
+            patterned.remove(patterned.size() - 1);
+          } else if (!".".equals(component)) {
+            components.add(component);
+            patterned.add(filter.hasPattern());
+          }
+        }
+        boolean subtree = patterned.contains(Boolean.TRUE);
+        Path probe = subtree ? globLiteralPrefix(root, components, patterned) 
: candidate;
+        rules.add(new Rule(filesystem.toString(), probe.toUri().getPath(), 
subtree));
+      }
+      return rules;
+    }
+
+    private static Path globLiteralPrefix(Path root, List<String> components, 
List<Boolean> patterned) {
+      Path prefix = root;
+      for (int i = 0; i < components.size() && !patterned.get(i); i++) {
+        prefix = new Path(prefix, components.get(i));
+      }
+      return prefix;
+    }
+
+    /** Java requests own their mount clients; native connections keep their 
existing cache. */
+    private static final class RequestViewFileSystem extends 
org.apache.hadoop.fs.viewfs.ViewFileSystem {
+      private final Map<List<String>, FileSystem> children = new HashMap<>();
+      private final Set<FileSystem> owned = Collections.newSetFromMap(new 
java.util.IdentityHashMap<>());
+      private boolean closed;
+
+      private RequestViewFileSystem() throws IOException {
+      }
+
+      private final FsGetter getter = new FsGetter() {
+        @Override
+        public FileSystem get(URI uri, Configuration conf) throws IOException {
+          synchronized (RequestViewFileSystem.this) {
+            List<String> key = Arrays.asList(uri.getScheme(), 
uri.getRawAuthority());
+            FileSystem child = children.get(key);
+            if (child == null) {
+              child = getNewInstance(uri, conf);
+              children.put(key, child);
+            }
+            return child;
+          }
+        }
+
+        @Override
+        public FileSystem getNewInstance(URI uri, Configuration conf) throws 
IOException {
+          synchronized (RequestViewFileSystem.this) {
+            if (closed) {
+              throw new IOException("TVF ViewFS request is closed");
+            }
+            FileSystem child = FileSystem.newInstance(uri, conf);
+            owned.add(child);
+            return child;
+          }
+        }
+      };
+
+      @Override
+      protected FsGetter fsGetter() {
+        return getter;
+      }
+
+      @Override
+      public synchronized void close() throws IOException {
+        if (closed) {
+          return;
+        }
+        List<IOException> failures = new ArrayList<>();
+        try {
+          // delete-on-exit may resolve a previously unused mount, so do this 
before sealing.
+          ((FileSystem) this).closeBase();
+        } catch (IOException e) {
+          failures.add(e);
+        } finally {
+          closed = true;
+          // Enumerating ViewFS target.getChildFileSystems() misses Nfly's 
actual peers.
+          // The stable getter owns those peers directly, including partial 
initialization.
+          for (FileSystem child : owned) {
+            try {
+              child.close();
+            } catch (IOException e) {
+              failures.add(e);
+            } catch (RuntimeException e) {
+              failures.add(new IOException("Cannot close a TVF mount client", 
e));
+            }
+          }
+          owned.clear();
+          children.clear();
+        }
+        if (!failures.isEmpty()) {
+          throw MultipleIOException.createIOException(failures);
+        }
+      }
+    }
+
+    private static boolean isNative(String policy) {
+      return NATIVE_POLICY.equals(policy) || 
NATIVE_WRITE_POLICY.equals(policy);
+    }
+
+    private static boolean isJavaRequest(Configuration conf) {
+      String policy = conf.getRaw(POLICY_KEY);
+      return policy != null && !policy.isEmpty() && !isNative(policy);
+    }
+
+    private static boolean isRequestViewFs(Class<? extends FileSystem> 
implementation, Configuration conf) {
+      return isJavaRequest(conf) && implementation == 
org.apache.hadoop.fs.viewfs.ViewFileSystem.class;
+    }
+
+    private static IOException rejected() {
+      return new IOException("Hadoop TVF requires a proven remote filesystem 
route. "
+          + "Use local() with LOCAL TVF privileges and secure-path 
restrictions.");
+    }
+
+    private static Configuration freeze(Configuration conf) throws IOException 
{
+      // Service providers (notably HDFS) register their default/site 
resources on first load.
+      // Load them before copying raw values, as an ordinary FileSystem lookup 
does.
+      if (!FILE_SYSTEMS_LOADED) {
+        loadFileSystems();
+      }
+      if (conf instanceof FrozenConfiguration) {
+        return conf;
+      }
+      return new FrozenConfiguration(conf);
+    }
+
+    private static Configuration prepare(Configuration conf) throws 
IOException {
+      String policy = conf.getRaw(POLICY_KEY);
+      if (policy == null || policy.isEmpty()) {
+        return conf;
+      }
+      Configuration frozen = freeze(conf);
+      Context context = ((FrozenConfiguration) frozen).context;
+      if (isNative(policy)) {
+        context.checkNative(frozen.getRaw(ROOT_KEY), frozen.getRaw(PATH_KEY));
+      } else if (JAVA_READ_POLICY.equals(policy)) {
+        context.checkJava(new Path(frozen.getRaw(PATH_KEY)));
+      } else {
+        context.checkRules(policy);
+      }
+      return frozen;
+    }
+
+    public static void check(URI ignored, Configuration conf) throws 
IOException {
+      prepare(conf);
+    }
+
+    public static void clearNativeContext() {
+      LAST_NATIVE_CONTEXT.remove();
+    }
+
+    public static Context takeNativeContext() {
+      Context result = LAST_NATIVE_CONTEXT.get();
+      LAST_NATIVE_CONTEXT.remove();
+      return result;
+    }
+
+    public static Configuration prepareJavaRead(Configuration conf, Path path, 
String marker) throws IOException {
+      if (marker == null || marker.isEmpty()) {
+        return conf;
+      }
+      if (!NATIVE_POLICY.equals(marker)) {
+        throw rejected();
+      }
+      Configuration request = new Configuration(conf);
+      request.set(POLICY_KEY, JAVA_READ_POLICY);
+      request.set(ROOT_KEY, path.toUri().toString());
+      request.set(PATH_KEY, path.toString());
+      return prepare(request);
+    }
+
+    public static void checkJavaRead(FileSystem fs, Path path, String marker) 
throws IOException {
+      if (marker == null || marker.isEmpty()) {
+        return;
+      }
+      if (!NATIVE_POLICY.equals(marker)) {
+        throw rejected();
+      }
+      // The marker is request-owned, never inferred from an unmarked cached 
filesystem.
+      Configuration conf = fs.getConf();
+      if (JAVA_READ_POLICY.equals(conf.getRaw(POLICY_KEY))) {
+        // S3A derives a Configuration for bucket options. Its copy preserves 
our internal
+        // marker and raw snapshot values, but not the private Configuration 
subclass.
+        ((FrozenConfiguration) freeze(conf)).context.checkJava(path);
+      } else {
+        throw rejected();
+      }
+    }
+
+    private static final class FrozenConfiguration extends Configuration {
+      private final Context context;
+
+      private FrozenConfiguration(Configuration source) throws IOException {
+        super(false);
+        setClassLoader(source.getClassLoader());
+        java.util.SortedMap<String, String> values = new java.util.TreeMap<>();
+        for (Map.Entry<String, String> entry : source) {
+          String key = entry.getKey();
+          String value = source.getRaw(key);
+          set(key, value);
+          if (!PATH_KEY.equals(key) && !ROOT_KEY.equals(key) && 
!CONTEXT_KEY.equals(key)) {
+            values.put(key, value);
+          }
+        }
+        // ViewFS's private InnerCache folds authority case (A/a). Guarded 
children must
+        // instead use FileSystem's exact-authority/profile cache below.
+        set("fs.viewfs.enable.inner.cache", "false");
+        values.put("fs.viewfs.enable.inner.cache", "false");
+        try {
+          java.security.MessageDigest digest = 
java.security.MessageDigest.getInstance("SHA-256");
+          java.io.DataOutputStream output = new java.io.DataOutputStream(
+              new java.security.DigestOutputStream(new java.io.OutputStream() {
+                @Override
+                public void write(int value) {
+                }
+
+                @Override
+                public void write(byte[] values, int offset, int length) {
+                }
+              }, digest));
+          for (Map.Entry<String, String> entry : values.entrySet()) {
+            for (String value : new String[] {entry.getKey(), 
entry.getValue()}) {
+              // Preserve raw Java code units, including unpaired surrogates. 
UTF-8 replacement
+              // would give a literal '?' and an unpaired surrogate the same 
cache profile.
+              output.writeInt(value.length());
+              output.writeChars(value);
+            }
+          }
+          output.flush();
+          StringBuilder identity = new StringBuilder();
+          for (byte value : digest.digest()) {
+            identity.append(Character.forDigit((value & 255) >>> 4, 16));
+            identity.append(Character.forDigit(value & 15, 16));
+          }
+          set(CONTEXT_KEY, identity.toString());
+        } catch (java.security.NoSuchAlgorithmException e) {
+          throw new IOException("SHA-256 is required for TVF Hadoop cache 
isolation");
+        }
+        context = new Context(this);
+      }
+    }
+
+    public static final class Context {
+      private final Configuration conf;
+      private final Map<String, String> routingKeys = new HashMap<>();
+
+      private Context(Configuration conf) {
+        this.conf = conf;
+        for (Map.Entry<String, String> entry : conf) {
+          String key = entry.getKey();
+          if ("fs.defaultFS".equals(key) || "fs.default.name".equals(key)
+              || key.startsWith("fs.viewfs.") || (key.startsWith("fs.") && 
key.endsWith(".impl"))) {
+            routingKeys.put(key, entry.getValue());
+          }
+        }
+      }
+
+      private EffectiveRoute route(boolean descendants) {
+        return new EffectiveRoute(routingKeys, descendants, conf);
+      }
+
+      public String profile() {
+        return conf.getRaw(CONTEXT_KEY);
+      }
+
+      public boolean isDirectRemote() throws IOException {
+        String root = conf.getRaw(ROOT_KEY);
+        if (root == null) {
+          throw rejected();
+        }
+        try {
+          URI uri = nativeUri(root);
+          return route(false).provider(uri.getScheme()) == 
Route.Provider.REMOTE;
+        } catch (RuntimeException | URISyntaxException e) {
+          throw rejected();
+        }
+      }
+
+      private static URI nativeUri(String root) throws URISyntaxException {
+        root = root.trim();
+        if (root.isEmpty() || "default".equals(root)) {
+          throw new URISyntaxException("", "Unproven native filesystem");
+        }
+        return new URI(root.contains("://") ? root : "hdfs://" + root);
+      }
+
+      public void checkNative(String root, String path) throws IOException {
+        if (root == null || path == null || path.isEmpty()) {
+          throw rejected();
+        }
+        try {
+          URI uri = nativeUri(root);
+          Path actualPath = new Path(path);
+          if (NATIVE_WRITE_POLICY.equals(conf.getRaw(POLICY_KEY))) {
+            // ViewFS.create resolves the parent, without following the final 
component.
+            Path parent = actualPath.getParent();
+            actualPath = parent == null ? new Path("/") : parent;
+          }
+          route(false).checkRoute(uri, actualPath.toUri().getPath(), new 
HashSet<>());
+        } catch (RuntimeException | URISyntaxException e) {
+          throw rejected();
+        }
+      }
+
+      private void checkJava(Path path) throws IOException {
+        try {
+          URI uri = path.toUri();
+          route(false).checkRoute(uri, uri.getPath(), new HashSet<>());
+        } catch (RuntimeException | URISyntaxException e) {
+          throw rejected();
+        }
+      }
+
+      private void checkRules(String policy) throws IOException {
+        try {
+          byte[] bytes = java.util.Base64.getDecoder().decode(policy);
+          java.io.DataInputStream in = new java.io.DataInputStream(new 
java.io.ByteArrayInputStream(bytes));
+          int version = in.readInt();
+          int count = in.readInt();
+          if (version != 1 || count <= 0 || count > in.available() / 5) {
+            throw rejected();
+          }
+          for (int i = 0; i < count; i++) {
+            URI filesystem = new URI(in.readUTF());
+            String path = in.readUTF();
+            boolean descendants = in.readBoolean();
+            route(descendants).checkRoute(filesystem, path, new HashSet<>());
+          }
+          if (in.available() != 0) {
+            throw rejected();
+          }
+        } catch (RuntimeException | URISyntaxException e) {
+          throw rejected();
+        }
+      }
+    }
+
+    private static final class EffectiveRoute extends Route {
+      private final Configuration conf;
+
+      private EffectiveRoute(Map<String, String> properties, boolean 
descendants, Configuration conf) {
+        super(properties, descendants);
+        this.conf = conf;
+      }
+
+      private EffectiveRoute(EffectiveRoute original, boolean descendants) {
+        super(original, descendants);
+        conf = original.conf;
+      }
+
+      @Override
+      protected Route withDescendants(boolean value) {
+        return new EffectiveRoute(this, value);
+      }
+
+      @Override
+      protected String property(String key) {
+        return conf.get(key);
+      }
+
+      @Override
+      protected Provider provider(String scheme) {
+        if ("file".equalsIgnoreCase(scheme)) {
+          return Provider.LOCAL;
+        }
+        try {
+          String implementation = FileSystem.getFileSystemClass(scheme, 
conf).getName();
+          return implementationProvider(implementation);
+        } catch (IOException | RuntimeException e) {
+          return Provider.UNKNOWN;
+        }
+      }
+    }
+
+    public static class Route {
+      protected enum Provider {
+        REMOTE,
+        LOCAL,
+        MOUNT_TABLE,
+        DYNAMIC,
+        UNKNOWN
+      }
+
+      protected static final Set<String> REMOTE_SCHEMES = new 
HashSet<>(Arrays.asList(
+          "hdfs", "s3a", "s3n", "oss", "ofs", "jfs", "abfs", "abfss", "wasb", 
"wasbs", "gs", "cosn"));
+      protected static final Set<String> REMOTE_IMPLEMENTATIONS = new 
HashSet<>(Arrays.asList(
+          "org.apache.hadoop.hdfs.DistributedFileSystem",
+          "org.apache.hadoop.fs.s3a.S3AFileSystem",
+          "org.apache.hadoop.fs.azurebfs.AzureBlobFileSystem",
+          "org.apache.hadoop.fs.azurebfs.SecureAzureBlobFileSystem",
+          "org.apache.hadoop.fs.azure.NativeAzureFileSystem",
+          "org.apache.hadoop.fs.azure.NativeAzureFileSystem$Secure",
+          "io.juicefs.JuiceFileSystem",

Review Comment:
   [P1] Validate the JuiceFS backing store before declaring it remote
   
   With the supported JuiceFS 1.3.1 JAR installed, format a reachable Redis 
namespace with `--storage file --bucket /tmp/tvf-target/`. A global LOAD user 
can then run `INSERT INTO hdfs('file_path'='jfs://cluster/out_', 
'format'='csv', 'fs.defaultFS'='jfs://cluster', 
'fs.jfs.impl'='io.juicefs.JuiceFileSystem', 
'juicefs.cluster.meta'='redis://controlled-meta:6379/1') SELECT 1` while 
`enable_local_tvf=false`. Both FE and `EffectiveRoute` return REMOTE for this 
class, and `Context.isDirectRemote()` also exempts cached uses from further 
route resolution. JuiceFS [loads the namespace 
format](https://github.com/juicedata/juicefs/blob/v1.3.1/sdk/java/libjfs/main.go#L498-L549),
 [selects its Storage/Bucket 
backend](https://github.com/juicedata/juicefs/blob/v1.3.1/cmd/format.go#L256-L264),
 and its file backend [creates/writes local data 
objects](https://github.com/juicedata/juicefs/blob/v1.3.1/pkg/object/file.go#L139-L187).
 The insert therefore persists authoritative chunks below `/tmp/tvf-targ
 et/<volume>/chunks/` on the BE without LOCAL opt-in or secure-root admission. 
Validate the actual backend, including metadata reloads, or reject unproven 
JuiceFS routes in guarded TVFs; cover file-backed rejection and remote-backed 
success. This differs from the earlier JuiceFS compatibility thread: 
recognizing the provider class does not establish where its data is stored.



-- 
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]

Reply via email to