Thursday 30 October 2014

ColdFusion bug challenge: how quickly can one find a bug in ColdFusion 11?

G'day:
Yesterday at lunchtime, I set myself a challenge: how quickly could I find a new, unreported bug in ColdFusion 11.


During my initial testing of ColdFusion 11 (when it was beta, and continuing through until I got sick of doing Adobe's testing for them), it was hardly even a sport finding bugs in ColdFusion 11. All one had to do was to look at it, and bugs would crawl out going "hi!". It really was that bloody simple. Their prerelease testing  and QA was a shambles for CF11. Until they release a substantial service pack for it, I do not believe it is fit for purpose. Unfortunately a substantial service pack will no-doubt bring a raft of bugs of its own, so I don't know when I'd actually consider it fit for purpose. Some people have gone live with it, that said. Good luck to them.

Anyway, I think the low-hanging fruit have been plucked, bug-wise, so figured it might be harder to find a new bug these days. Although I did kinda think the one hour I gave myself at lunchtime would still be OK.

I opened the "New in ColdFusion" page from the docs, and scanned down for quick things to test: I only had an hour after all.

I dismissed the entire "End-to-end mobile development" section, because - as Andy alluded to on Twitter yesterday:


This is something I whole-heartedly agree with. The whole concept counts as a planning / design / implementation bug, and we already know about it.

I also dismissed "A new lightweight edition", as looking for bugs in that is kinda devops shit, which I detest, so amn't interested.

I settled on the "Language Enhancements" section. This covers a lot, so thought there'd be some scraps around to find.

Scanning down the topics I figured the Elvis operator would be a candidate for a bug, because it's not entirely the most straight forward thing ever, so the ColdFusion Team would probably balls it up.

Indeed they have, but I've raised it already, dammit: "Null coalescing operator not quite implemented correctly". Predictably ColdFusion doesn't quite get how "nulls" work.

Another thing I guessed would be wrong is that ?: should be short-circuited, but it probably hasn't been implemented that way:

// nullcoalescingShortCircuit.cfm
function defaulter(){
    writeOutput("defaulter() was called<br>");
    return "default";
}

result = URL.param ?: defaulter();
writeOutput("Value of result: [#result#]<br>");

If I run this without URL.param set I get this:

defaulter() was called
Value of result: [default]


This is correct.

However if I call it and pass param=set on the URL, I get this:

defaulter() was called
Value of result: [set]


This is less than ideal, as the default option should only be called if necessary. This is a bug (FYI: Railo gets this right). However... it's already been raised: "Elvis operator executes RHS (right hand side) when it doesn't need to."

I moved on from the Elvis operator section. Next was queryExecute(), but I skipped that as I've already given it a bit of a test thrashing, and didn't expect to be able to find a new bug there within the 50min I had left.

JSON serialisation was after that, but I skipped that too as it takes some messing around to create test CFCs and serialisers and stuff. Plus I've looked at these too, so probably wouldn't be able to find anything new quickly enough.

I breezed through some of the member functions looking for things that were likely to be messed up, but nothing leapt out at me. I tried a few things but everything worked to spec. Inconvenient for my challenge, but good for ColdFusion.

I guessed that they'd probably mess up listEach(), so had a quick look there. Perhaps they'd mishandle empty list items or different delimiters (although I had a suspicion I'd already looked at this):

// listEach.cfm
oddNumbers = "tahi||toru||";

oddNumbers.each(function(e){
    writeOutput(e & "<br>");
}, "|", true);

However this works just fine:

tahi

toru



Dammit.

Actually at this point I skirted very close to finding a bug. In fact I identified one (by simply guessing it'd be buggy), but incorrectly "remembered" that I had already raised it. So I didn't check it. I'll come back to this.

I was about 20min into the hour now.

Next I figured queryGetRow() would probably not be able to cope with non-standard query columns, so I tried to break that:

// queryGetRow.cfm
json = '{"COLUMNS":["ID","COL with ## in name"],"DATA":[[1,"tahi"],[2,"rua"],[3,"toru"],[4,"wha"]]}';
records = deserializeJson(json, false);
writeDump(records);

row = queryGetRow(records, 1);
writeDump(row);

"Sadly" this worked fine:

query
COL with # in nameID
1tahi1
2rua2
3toru3
4wha4
struct
COL with # in nametahi
ID1

I also tested getting an invalid row, and queryGetRow() dealt with that OK too:

The row number 6 is out of bounds.

The error occurred in queryGetRow.cfm: line 7
5 : writeDump(records);
6 : 
7 : row = queryGetRow(records, 6);
8 : writeDump(row);
9 : </cfscript>


Actually in playing around with some things now, I just found a tiny bug. Just the grammar in an error message:

Parameter validation error for the QUERYGETROW function.

The function takes 2 parameter.
The error occurred inC:/apps/adobe/ColdFusion/11/express/ColdFusion/cfusion/wwwroot/shared/github-scratch/blogExamples/datatypes/queries/queryGetRow.cfm: line 7
5 : writeDump(records);
6 : 
7 : row = queryGetRow(records, 1, true);
8 : writeDump(row);
9 : </cfscript>

(It should be "parameters")

That's not interesting enough to worry about, although I guess I'll raise it anyhow (later).

I was at about the 30min mark now. Still no bug.

Hmmm.... I thought I'd tested built-in functions being first-class now, but I thought of something I might not have tested to try:

// passing.cfm
function stringTransformer(required string text, required function transformer){
    return transformer(text);
}

string = "G'day World!";
handler = ucase;
transformed = stringtransformer(string, handler);

writeDump([string, transformed]);

I'd tested passing built-in functions as argument values, but wasn't too sure had I tested assigning them to variables first. But it works OK anyhow:

array
1G'day World!
2G'DAY WORLD!

35min.

Clicking around the "what's new" doc, and feeling pressure to come up with something, I spotted the "Security Enhancements (ColdFusion 11)" page, and figured this'd be easy. Adobe are crap at security (like in CFAdmin) so I'd find something.

I scrolled down and spotted the "Changes in Administrator API" subsection and reference to the "isAllowCuncurrentAdminLogin()" function. I was so hoping that spelling mistake had made it into the product I thought I had better check. This would not qualify as a reasonable enough bug to raise, but I figured I was running shy of time so probably wouldn't now find one anyhow.

First things first, I needed to be able to login to the thing. I always disable the need for a password on my CFAdmin because it's a waste of time on a dev machine (and, indeed, any well-configured prod machine), so I figured I'd need an account with a password to use on the API.

I went into CFAdmin, into the Security section, and user manager.
I added a user apiuser, and when trying to set it for "Allow Administrative Access", I got a pop-up thus:



Sigh, yeah, fair enough. So I dismissed that, also selected "API Access Only", then saved the user and moved back to the Administrator screen as suggested.

On this screen I selected "Separate user name [etc]..." and submitted the changes. And was confronted with this:



Dammit. Not being one to give up immediately, I just changed the radio button again and resubmitted:


It's getting worse! I tried another time and the message was the same.

So I sighed and wondered if it was getting confused because I had already created another user, or something, so went back to the "User Manager", and got this:




And no amount of reattempts to load that page worked. But that, to me, is a bug. Indeed it's twofold: a) simply using the UI should not... well.. break it; b) have you heard of error trapping, Adobe? Blimey. you shouldn't just let errors throw up on the screen: catch 'em and present something less amateur-looking than just a raw error message.

At this point I had been looking for 44min, and I'd found a bug. Job done.

I restarted CF and the error cleared itself, and I was then able to make my setting change, and off I went. And I hasten to add I could not replicate the issue again, on that machine. However I have just tried on a second machine, and got exactly the same thing. And on a third machine: same thing.

That was good enough for my mission, but I wanted to find a sort of code bug, so after the restart I was able to create an AdminAPI user so would be able to authenticate to the API and run some code:

// securityObjectBug.cfm
administrator = new CFIDE.adminapi.administrator();
administrator.login("12345678", "apiuser");
try {
    writeOutput("isAdminUser() according to administrator object: ");
    writeOutput("#administrator.isAdminUser()#<br>");
} catch (any e){
    writeDump([e.type,e.message,e.detail]);
}
security = new CFIDE.adminapi.Security();
writeOutput("security object exists: " & structKeyExists(variables, "security") & "<br>");
writeOutput("isAdminUser() method in security object exists: " & structKeyExists(security, "isAdminUser") & "<br>");
writeOutput("isAdminUser() method in security object is a function: " & isCustomFunction(security.isAdminUser) & "<br>");
writeDump(var=getMetadata(security.isAdminUser), label="metadata for security.isAdminUser");

writeOutput("isAdminUser() according to security object: ");
writeOutput("#security.isAdminUser()#<br>");

I arrived at this code because I was trying to login then run some method (I can't remember which now), but it was erroring about security, so decided to verify that I was actually logged in and as a user authorised to call the method. This is as far as I got before I just threw my hands in the air saying "well, yeah, I knew it would not take long to find shittily-written CFML in the admin API, because "shittily-written" is the only sort of CFML the ColdFusion Team are capable of writing".

This is the output of that code, btw:

isAdminUser() according to administrator object: true
security object exists: YES
isAdminUser() method in security object exists: YES
isAdminUser() method in security object is a function: YES

metadata for security.isAdminUser - struct
ACCESSpublic
HINTCheck to see if user is authenticated
NAMEisAdminUser
OUTPUTfalse
PARAMETERS
metadata for security.isAdminUser - array
1
metadata for security.isAdminUser - struct
DEFAULT[empty string]
HINTList of required roles.
NAMErequiredRoles
REQUIREDNo
isAdminUser() according to security object:
The web site you are accessing has experienced an unexpected error.
Please contact the website administrator. 


The following information is meant for the website developer for debugging purposes.
Error Occurred While Processing Request

Variable SECURITY is undefined.

The error occurred in base.cfc: line 27
-1 : Unable to display error's location in a CFML template.

Note that this is not a reference to my code erroring. It's the code in the AdminAPI erroring. And naturally I cannot go and find out why, because Adobe precompile the AdminAPI files and don't ship the source. Groan.

So I was fed up with CF11 now, and - this was after about 1.5 hours - I'd found two bugs.



What about the bug I guessed was there at the 20min mark?

I guessed that even if listEach() handled the delimiters and empty elements properly, that it wouldn't be passing those arguments into the callback. Although it needs to, because whilst the callback gets passed the element, index and the whole list, the whole list is unusable unless you know the delimiters too. And whether to respect empty elements. Here's some code which demonstrates the difference between what listEach()'s and listMap()'s callbacks receive:

// listEach.cfm
oddNumbers = "tahi||toru";

writeOutput("<h3>each()</h3>");
oddNumbers.each(function(){
    writeDump(arguments);
}, "|", true);

writeOutput("<h3>map()</h3>");
oddNumbers.map(function(){
    writeDump(arguments);
}, "|", true);

Output:

each()

struct
1tahi
21
3tahi||toru
struct
1[empty string]
22
3tahi||toru
struct
1toru
23
3tahi||toru

map()

struct
1tahi
21
3tahi||toru
4|
5YES
struct
1[empty string]
22
3tahi||toru
4|
5YES
struct
1toru
23
3tahi||toru
4|
5YES


Notice how listMap() correctly receives the delimiter and empty-elements setting? But listEach() does not. This is a bug in the implementation of listEach().

Unfortunately for my little challenge I misremembered raising this with Adobe already, but I only kind of had. I raised it for listMap(): "listMap() doesn't pass correct args to callback". I even wrote an article about it!! ("ColdFusion 11: .map() and .reduce()"). The annoying thing here is that bug entry only mentions listMap() and in-passing listReduce()... so listMap() and listReduce() are the only ones they addressed. The Adobe ColdFusion engineer addressing this didn't have the nous for it to occur to them that they also need to do the same thing for all the other list iteration functions. Either that, or they're just mind-boggling lazy and didn't bother. Neither would surprise me.

I didn't repro this until I was writing this article, so it can't really count for my bug-finding exercise. But combining the initial exercise of trying to find a bug, and the follow-up writing the article, I've found a total of four new bugs:
  • The broken behaviour in CFAdmin (3845476);
  • The broken behavious in the CFAdmin API (3845479);
  • The wee spelling mistake in that error message (3845475);
  • This fail in listEach() (3713043).

The only one I care about here is the last one, TBH, but still.

I also think I found another bug in ColdFusion 9 today too. But I haven't got a repro for that yet, so cannot file it.

Bug-spotting in ColdFusion is way too easy.

--
Adam