Prevent infinite loop in function lookup

[joewiz]

The problem

#29 (released in https://github.com/eXist-db/shared-resources/releases/tag/v0.5.0) added the following feature:

Template functions can now accept an arbitrary number of parameters, previously limited to 15

... but this enhancement introduced a bug, reported in #33: If a template references a non-existent function, the request for this template hangs indefinitely, and the query has to be manually killed.

Diagnosis

The problem was caused by an infinite loop in templates:resolve(), which looks up a function named in a template call. It checks to see if a function of the given name and arity 2 exists (this would correspond to a basic templating function that contains 2 parameters, $node and $model); if no function at this arity is found, the check is performed again, incrementing the arity by 1, and again recursively—with no bound.

This recursive check for arity is needed, since the function called may take fewer or greater than the number of parameters supplied in the template; the templating module allows parameters to come from at least 3 other locations (URL parameters, request attributes, and session attributes), with an extension mechanism for developers to supply their own ordered set of parameter sources. So the templating module has no way to know in advance the arity to apply. Instead, it needs to find a function whose arity matches. It proceeds from least to greatest arity. But there's currently no upper bound, meaning that missing functions lead to an infinite loop.

Solutions

One solution would be to enforce an upper bound for arity in the $lookup (or $resolve) function they supply to templates:apply() in their apps' modules/view.xql query:

let $lookup := function ($functionName as xs:string, $arity as xs:int) {
    if ($arity le 20) then 
        try {
            function-lookup(xs:QName($functionName), $arity)
        } catch * {
            ()
        }
    else
        ()
}

However, that would require changes in apps that previously worked before #29, and it would impose a burden on all apps that wanted to insulate themselves from this problem.

The approach taken in this PR is to move the maximum arity value into the templating function, hard-coding the previous limit of 20 for templating function arity. This limit was adequate before #29 and could always be adjusted upward if needed in a future PR.

Result

Now, if a non-existent function is referenced, the templating module immediately returns an error:

An error has occurred
An error has been generated by the application.

templates:NotFound No template function found for call app:doh-a-non-existent-function [at line 191, column 85, source: /db/apps/shared-resources/content/templates.xql]
In function:
templates:call(item(), element(), map(*)) [137:36:/db/apps/shared-resources/content/templates.xql]

Closes #33.

[adamretter]

Excellent stuff. I wonder, could we have a custom error here that says that the max has been exceeded and that they could tweak the variable in whichever file if they need a higher limit?

[adamretter]

I have a feeling that I opened an issue somewhere because there are two versions of the templating module; One in shared resources and one elsewhere. But I now can't find the other issue.

If that is the case do we need to duppicate these changes? Or preferably put tempting in its own module?

[joewiz]

@adamretter Great idea to improve the error message. I've committed this. Thanks!

Yes, the "old" dashboard contained a copy of the templating module - see https://github.com/eXist-db/dashboard/blob/master/modules/templates.xql. But luckily the "new" dashboard dropped it - see https://github.com/eXist-db/existdb-dashboard/tree/master/modules.

[joewiz]

@adamretter Incidentally, this made me wish for a 1-arity version of fn:function-lookup which would take just a function name and return all arity variants. It also seems like the F&O 3.1 spec forgot to provide a function for probing a function’s annotations - there’s no fn:function-annotations.

These, combined with some of the functions from the inspect module and the util functions for retrieving available modules and functions might make good candidates for inclusion in an EXPath spec (: if not XQuery 3.2 :). This could fit in nicely with @lcahlander’s XQDoc project.

[adamretter]

So I think creating an EXPath reflection module is probably the best chance of success for such things. Would be happy to collaborate on it with you

[joewiz]

@adamretter Thanks for the review & merge and these comments. I wasn't familiar with the term reflection, but if that's the umbrella term that this would fall under, it sounds great. Perhaps once HTTP v2 is done and the community issues are a little more settled.