[bugfix] Cyclic dependency import detection for XQuery 1.0 (i.e. XQST0093)

adamretter · adamretter · commit 15e62ea62955 · 2023-10-14T14:43:02.000+02:00
diff --git a/exist-core/pom.xml b/exist-core/pom.xml
@@ -514,6 +514,17 @@
             <artifactId>ant</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>org.jgrapht</groupId>
+            <artifactId>jgrapht-core</artifactId>
+            <version>1.5.2</version>
+        </dependency>
+
+	<dependency>
+            <groupId>org.jgrapht</groupId>
+            <artifactId>jgrapht-opt</artifactId>
+            <version>1.5.2</version>
+        </dependency>
 
         <dependency>
             <groupId>junit</groupId>
diff --git a/exist-core/src/main/java/org/exist/xquery/ModuleContext.java b/exist-core/src/main/java/org/exist/xquery/ModuleContext.java
@@ -48,10 +48,10 @@
 
 
 /**
- * Subclass of {@link org.exist.xquery.XQueryContext} for
- * imported modules.
+ * Subclass of {@link org.exist.xquery.XQueryContext} for imported modules.
  *
  * @author wolf
+ * @author <a href="mailto:adam@evolvedbinary.com">Adam Retter</a>
  */
 public class ModuleContext extends XQueryContext {
 
@@ -66,7 +66,8 @@ public ModuleContext(final XQueryContext parentContext, final String moduleNames
         super(parentContext != null ? parentContext.db : null,
                 parentContext != null ? parentContext.getConfiguration() : null,
                 null,
-                false);
+                false,
+                null);
         this.moduleNamespace = moduleNamespace;
         this.modulePrefix = modulePrefix;
         this.location = location;
@@ -95,6 +96,52 @@ public void setModuleNamespace(final String prefix, final String namespaceURI) {
         this.moduleNamespace = namespaceURI;
     }
 
+    @Override
+    protected void addModuleVertex(final ModuleVertex moduleVertex) {
+        getRootContext().addModuleVertex(moduleVertex);
+    }
+
+    protected boolean hasModuleVertex(final ModuleVertex moduleVertex) {
+        return getRootContext().hasModuleVertex(moduleVertex);
+    }
+
+    @Override
+    protected void addModuleEdge(final ModuleVertex source, final ModuleVertex sink) {
+        getRootContext().addModuleEdge(source, sink);
+    }
+
+    @Override
+    protected boolean hasModulePath(final ModuleVertex source, final ModuleVertex sink) {
+        return getRootContext().hasModulePath(source, sink);
+    }
+
+    @Override
+    public @Nullable Module[] importModule(@Nullable String namespaceURI, @Nullable String prefix, @Nullable AnyURIValue[] locationHints) throws XPathException {
+        final ModuleVertex thisModuleVertex = new ModuleVertex(moduleNamespace, location);
+
+        for (final AnyURIValue locationHint : locationHints) {
+            final ModuleVertex imporedModuleVertex = new ModuleVertex(namespaceURI, locationHint.toString());
+
+            if (!hasModuleVertex(imporedModuleVertex)) {
+                addModuleVertex(imporedModuleVertex);
+            } else {
+                // Check if there is already a path from the imported module to this module
+                if (getXQueryVersion() == 10 && namespaceURI != null && locationHints != null && hasModulePath(imporedModuleVertex, thisModuleVertex)) {
+                    throw new XPathException(ErrorCodes.XQST0093, "Detected cyclic import between modules: " + getModuleNamespace() + " at: " + getLocation() + ", and: " + namespaceURI + " at: " + locationHint.toString());
+                }
+            }
+
+            if (!hasModuleVertex(thisModuleVertex)) {
+                // NOTE(AR) may occur when the actual module has a different namespace from that of the `import module namespace`... will later raise an XQST0047 error
+                addModuleVertex(thisModuleVertex);
+            }
+
+            addModuleEdge(thisModuleVertex, imporedModuleVertex);
+        }
+
+        return super.importModule(namespaceURI, prefix, locationHints);
+    }
+
     @Override
     protected @Nullable Module importModuleFromLocation(final String namespaceURI, @Nullable final String prefix, final AnyURIValue locationHint) throws XPathException {
         // guard against self-recursive import - see: https://github.com/eXist-db/exist/issues/3448
diff --git a/exist-core/src/main/java/org/exist/xquery/XQueryContext.java b/exist-core/src/main/java/org/exist/xquery/XQueryContext.java
@@ -33,6 +33,7 @@
 import java.nio.file.Paths;
 import java.util.*;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
@@ -94,6 +95,13 @@
 import org.exist.xquery.update.Modification;
 import org.exist.xquery.util.SerializerUtils;
 import org.exist.xquery.value.*;
+import org.jgrapht.alg.interfaces.ShortestPathAlgorithm;
+import org.jgrapht.alg.shortestpath.TransitNodeRoutingShortestPath;
+import org.jgrapht.graph.DefaultEdge;
+import org.jgrapht.graph.DefaultGraphType;
+import org.jgrapht.opt.graph.fastutil.FastutilMapGraph;
+import org.jgrapht.util.ConcurrencyUtil;
+import org.jgrapht.util.SupplierUtil;
 import org.w3c.dom.Node;
 
 import static com.evolvedbinary.j8fu.OptionalUtil.or;
@@ -224,6 +232,11 @@ public class XQueryContext implements BinaryValueManager, Context {
      */
     private Object2ObjectMap<String, Module[]> allModules = new Object2ObjectOpenHashMap<>();
 
+    /**
+     * Describes a graph of all the modules and how they import each other.
+     */
+    private @Nullable final FastutilMapGraph<ModuleVertex, DefaultEdge> modulesDependencyGraph;
+
     /**
      * Used to save current state when modules are imported dynamically
      */
@@ -454,6 +467,10 @@ public XQueryContext(@Nullable final Database db, @Nullable final Configuration
     }
 
     protected XQueryContext(@Nullable final Database db, @Nullable final Configuration configuration, @Nullable final Profiler profiler, final boolean loadDefaults) {
+        this(db, configuration, profiler, loadDefaults, new FastutilMapGraph<>(null, SupplierUtil.createDefaultEdgeSupplier(), DefaultGraphType.directedPseudograph().asUnweighted()));
+    }
+
+    protected XQueryContext(@Nullable final Database db, @Nullable final Configuration configuration, @Nullable final Profiler profiler, final boolean loadDefaults, final @Nullable FastutilMapGraph<ModuleVertex, DefaultEdge> modulesDependencyGraph) {
         this.db = db;
 
         // if needed, fallback to db.getConfiguration
@@ -474,6 +491,8 @@ protected XQueryContext(@Nullable final Database db, @Nullable final Configurati
             this.profiler = new Profiler(null);
         }
 
+        this.modulesDependencyGraph = modulesDependencyGraph;
+
         this.watchdog = new XQueryWatchDog(this);
 
         // load configuration defaults
@@ -1529,6 +1548,65 @@ public void addModule(final String namespaceURI, final Module module) {
         addRootModule(namespaceURI, module);
     }
 
+    /**
+     * Add a vertex to the Modules Dependency Graph.
+     *
+     * @param moduleVertex the module vertex
+     */
+    protected void addModuleVertex(final ModuleVertex moduleVertex) {
+        modulesDependencyGraph.addVertex(moduleVertex);
+    }
+
+    /**
+     * Check if a vertex exists in the Modules Dependency Graph.
+     *
+     * @param moduleVertex the module vertex to look for
+     *
+     * @return true if the module vertex exists, false otherwise
+     */
+    protected boolean hasModuleVertex(final ModuleVertex moduleVertex) {
+        return modulesDependencyGraph.containsVertex(moduleVertex);
+    }
+
+    /**
+     * Add an edge between two Modules in the Dependency Graph.
+     *
+     * @param source the importing module
+     * @param sink the imported module
+     */
+    protected void addModuleEdge(final ModuleVertex source, final ModuleVertex sink) {
+        modulesDependencyGraph.addEdge(source, sink);
+    }
+
+    /**
+     * Look for a path between two Modules in the Dependency Graph.
+     *
+     * @param source the module to start searching from
+     * @param sink the destination module to attempt to reach
+     *
+     * @return true, if there is a path between the mdoules, false otherwise
+     */
+    protected boolean hasModulePath(final ModuleVertex source, final ModuleVertex sink) {
+        if (modulesDependencyGraph == null) {
+            return false;
+        }
+
+        ThreadPoolExecutor executor = null;
+        try {
+            executor = ConcurrencyUtil.createThreadPoolExecutor(2);
+            final ShortestPathAlgorithm<ModuleVertex, DefaultEdge> spa = new TransitNodeRoutingShortestPath<>(modulesDependencyGraph, executor);
+            return spa.getPath(source, sink) != null;
+        } finally {
+            if (executor != null) {
+                try {
+                    ConcurrencyUtil.shutdownExecutionService(executor);
+                } catch (final InterruptedException e) {
+                    Thread.currentThread().interrupt();
+                }
+            }
+        }
+    }
+
     protected void setRootModules(final String namespaceURI, @Nullable final Module[] modules) {
         if (modules == null) {
             allModules.remove(namespaceURI); // unbind the module
@@ -2641,6 +2719,11 @@ private ExternalModule compileOrBorrowModule(final String namespaceURI, final St
             }
 
             final ExternalModuleImpl modExternal = new ExternalModuleImpl(namespaceURI, prefix);
+
+            // NOTE(AR) this is needed to support cyclic imports in XQuery 3.1, see: https://github.com/eXist-db/exist/pull/4996
+            addModule(namespaceURI, modExternal);
+            addModuleVertex(new ModuleVertex(namespaceURI, location));
+
             final XQueryContext modContext = new ModuleContext(this, namespaceURI, prefix, location);
             modExternal.setContext(modContext);
             final XQueryLexer lexer = new XQueryLexer(modContext, reader);
@@ -3547,4 +3630,52 @@ public String getStringValue() {
 
         return sb.toString();
     }
+
+    @Immutable
+    public static class ModuleVertex {
+        private final String namespaceURI;
+        private final String location;
+
+        public ModuleVertex(final String namespaceURI) {
+            this.namespaceURI = namespaceURI;
+            this.location = null;
+        }
+
+        public ModuleVertex(final String namespaceURI, final String location) {
+            this.namespaceURI = namespaceURI;
+            this.location = location;
+        }
+
+        @Override
+        public boolean equals(final Object o) {
+            if (this == o) {
+                return true;
+            }
+
+            if (o == null || getClass() != o.getClass()) {
+                return false;
+            }
+
+            final ModuleVertex that = (ModuleVertex) o;
+            if (!namespaceURI.equals(that.namespaceURI)) {
+                return false;
+            }
+            return location.equals(that.location);
+        }
+
+        @Override
+        public int hashCode() {
+            int result = namespaceURI.hashCode();
+            result = 31 * result + location.hashCode();
+            return result;
+        }
+
+        @Override
+        public String toString() {
+            return "Module{" +
+                    "namespaceURI='" + namespaceURI + '\'' +
+                    "location='" + location + '\'' +
+                    '}';
+        }
+    }
 }
