Exist: Hangup in getting permissions in an AVT?

[xatapult]

I run the following XQuery script (I minimized it as much as possible). eXist eats my CPU up to 100%. I have to force shutdown the database.

xquery version "3.1" encoding "UTF-8";

declare option exist:serialize "method=xml media-type=text/xml indent=yes";

<TEST>
{
  let $path as xs:string :='/db/TEST' 
  return
    <result>
      <collection name="{$path}" created="{xmldb:created($path)}" 
        owner-name="{sm:get-permissions(xs:anyURI($path))/sm:permission/@owner}"/>
    </result>       
}    
</TEST>

The cause is in the AVT for the owner-name attribute. But when I put that expression outside an AVT it simply works.

Expected behavior
The requested owner name in the attribute. Or: some error message about what I did wrong. But not a hangup

To Reproduce
Create a /db/TEST collection. Run the script above as admin.

Context (please always complete the following information):

• OS: Windows 10
• eXist-db version: 5.2.0
• Java Version 1.8.0_202-b08

Additional context

• How is eXist-db installed? jar
• Any custom changes in e.g. conf.xml? no

[xatapult]

I tried doing this through REST and and through oXygen, both the same results

[adamretter]

@xatapult Thanks Erik, I can see and reproduce the issue.

It's quite impressive! I have managed to reduce it to this:

<outer>
    <inner perm="{sm:get-permissions(xs:anyURI("/db"))/sm:permission/@owner}"/>
</outer>

If you remove the <outer> element then it all works fine. I suspect an issue in the construction of the inner memtree.

This issue is in:

• 5.2.0-SNAPSHOT
• 5.1.1
• 4.8.0-SNAPSHOT

[xatapult]

I have reasons to suspect it was in 4.7 already...

[joewiz]

Sorry, what's an AVT?

[xatapult]

ATtribute Value Template, informally speaking the {...} to set an attribute's value with an XPath expression

[joewiz]

Ah, I see - I found the term defined in the XSLT spec. How strange that it's significant for this bug that the expression is enclosed!

[adamretter]

I have a fix which involves not reusing the XQueryContext's Memtree builder:

diff --git a/exist-core/src/main/java/org/exist/xquery/functions/securitymanager/PermissionsFunction.java b/exist-core/src/main/java/org/exist/xquery/functions/securitymanager/PermissionsFunction.java
index 185ab61d8f..b588cb0beb 100644
--- a/exist-core/src/main/java/org/exist/xquery/functions/securitymanager/PermissionsFunction.java
+++ b/exist-core/src/main/java/org/exist/xquery/functions/securitymanager/PermissionsFunction.java
@@ -53,7 +53,7 @@ import java.util.Optional;
  * @author <a href="mailto:[email protected]">Adam Retter</a>
  */
 public class PermissionsFunction extends BasicFunction {
-
+
     private final static QName qnGetPermissions = new QName("get-permissions", SecurityManagerModule.NAMESPACE_URI, SecurityManagerModule.PREFIX);
     private final static QName qnAddUserACE = new QName("add-user-ace", SecurityManagerModule.NAMESPACE_URI, SecurityManagerModule.PREFIX);
     private final static QName qnAddGroupACE = new QName("add-group-ace", SecurityManagerModule.NAMESPACE_URI, SecurityManagerModule.PREFIX);
@@ -424,7 +424,7 @@ public class PermissionsFunction extends BasicFunction {
     }

     private org.exist.dom.memtree.DocumentImpl permissionsToXml(final Permission permission) {
-        final MemTreeBuilder builder = context.getDocumentBuilder();
+        final MemTreeBuilder builder = new MemTreeBuilder(context);
         builder.startDocument();

         builder.startElement(new QName("permission", SecurityManagerModule.NAMESPACE_URI, SecurityManagerModule.PREFIX), null);

But... first I would like to understand the rules for when one should use either: XQueryContext#getDocumentBuilder() or new MemTreeBuilder(context). As I can see other functions that might also have the same issue. Perhaps @wolfgangmm could comment?

[wolfgangmm]

If you want to establish a new builder context, you should call XQueryContext#pushDocumentContext / XQueryContext#popDocumentContext. See e.g. EnclosedExpr.

Creating a new builder for every element/attribute/text constructor would be extremely expensive, so we're writing to the same builder for all constructors. Functions should push the builder in most cases though.

[adamretter]

@wolfgangmm okay cool thanks for clarifying, I have just found (and fixed) many more incorrect uses.