Tuesday 15 July 2014

Query.cfc et al functions have file-reads and server-wide exclusive locks buried in their code

G'day:
Today's muse is Henry, with this Twitter status message:


My reaction to that was along the lines of "WTF?", so as soon as I got into the office I had a look in base.cfc ([ColdFusion dir]\cfusion\CustomTags\com\adobe\coldfusion\base.cfc), and low-and behold, I see this code:

private string function getSupportedTagAttributes(string tagName)
{
    //store all service tag attributes in Server scope for faster access. 
    if(not isdefined("server.coldfusion.serviceTagAttributes.#tagName#"))
    {    
        lock scope="server" timeout="30" throwontimeout="yes" type="exclusive"
        {
            if(not isdefined("server.coldfusion.serviceTagAttributes.#tagName#"))
            {    
                var webinf = getPageContext().getServletContext().getRealPath("/WEB-INF");
                var sep = server.os.name contains "windows"? "\" : "/";
                var cftldpath = webinf & sep & "cftags" & sep & "META-INF" & sep & "taglib.cftld";
                var xpath = "/taglib/tag[name='#lcase(Right(tagName,len(tagName)-2))#']/attribute/name";
                var cftagsXml = XmlParse(FileRead(cftldpath));
                var tagAttributes = xmlsearch(cftagsXml,xpath);
                var attributeslist = "";
                var i = 0;
                for(i=1;i lte arraylen(tagAttributes); i++)
                {
                    attributeslist = listappend(attributeslist,trim(tagAttributes[i].xmltext));
                }
                server.coldfusion.serviceTagAttributes[tagName] = listsort(attributeslist,"textnocase","ASC");
            }
        }
    }

    //use a local variable for faster access
    lock scope="server" timeout="30" throwontimeout="yes" type="readonly"
    {
        var attributeslist = server.coldfusion.serviceTagAttributes[tagName];
    }
    
    return attributeslist;
}

getSupportedTagAttributes() is called in the pseudo-constructor of query.cfc (and all the other CFCs in that directory as well).

So the first time one uses one of these CFCs during the life of the server instance, it locks the server scope, reads-in the tag definition file, XML-parses it, finds the attributes for the tag version of the functionality, and stores that stuff in the server scope. At least it's only doing it the once.

But that's a really shit way of going about things. Mindnumbingly shit.

Query.cfc is specifically for emulating <cfquery>. It has <cfquery>-specific values throughout its code. So why the hell does it need to make a generic all to the filesystem, locking the entire server along the way, simply to read in which attributes to support? It's already got reference to most of these hard-coded immediately above in the same pseudo-constructor:

component extends="base"
{
    property string name; 
    property numeric blockfactor;
    property string cachedafter;
    property string cachedwithin;
    property string dataSource;
    property string dbtype;
    property boolean debug;
    property numeric maxRows;
    property string password;
    property string result;
    property numeric timeout;
    property string username;
    property string sql;
    property string fetchclientinfo;
    property struct clientinfo;
    
    //array to store cfqueryparams
    variables.parameters = [];

    //service tag to invoke
    variables.tagName = "CFQUERY";

    //list of valid cfquery tag attributes
    variables.tagAttributes = getSupportedTagAttributes(getTagName());
So just hard code the rest of them! especially if "store all service tag attributes in Server scope for faster access". You're only having to worry about that because of your shit-house approach to getting the data you need anyhow.

I had previously noted how abjectly shite the code in these CFCs was, but this takes the cake.

Whoever wrote this code should not be allowed to write code ever again.  And I hate to think what the quality of the Java code is under the hood of ColdFusion, if this is an example of how they implement stuff. Scary.

--
Adam




at