Tuesday 24 September 2013

Things I dislike: jobsworths

G'day:

Update:
For non-UK-English-speakers, from Wikipedia:
A jobsworth is a person who uses their job description in a deliberately uncooperative way, or who seemingly delights in acting in an obstructive or unhelpful manner.


I started to have a quick look at ColdFusion 10's new ESAPI functions this morning...  I'd give you a link for those, but they don't seem to be categorised in the docs (and I can't annotate the docs accordingly, cos the site is still broken since it was "upgraded" to use the new wiki)... and quickly got deviated onto the bug tracker.


Ron Stewart, one of the bods I talk to on Twitter was asking some interesting questions, about how these functions have been implemented, and in the course of not being much bloody help to him at all, I landed on the ESAPI4CF page on github (at Ron's suggestion).

I was interested in what the status of the project was now that ColdFusion 10 has inbuilt functions covering similar ground, and read down the page to find out. I landed on this annotation relating to ColdFusion 10 support:

ColdFusion 10.x (ESAPI4J v2.0.1) not supported due to Coldfusion 10 Bug #3222889. Adobe lists this as won't fix - help me get Adobe to fix this!
Obviously this is kinda red-rag-to-a-bull to me, so I followed the link. And was reminded I had been there before.

Here's a summary of salient comments:

Rupesh, Sep 4, 2013:
Is there any reason why you cannot use the Form.username directly and would like to use this method? GetPageContext().getRequest().getParameter('username') has never been documented and it is not supported.

Me, Sep 4, 2013:
Rupesh, both getParameter() and getParameterMap() are part of the PageContext spec (well: the ServletRequest spec, which is what PageContext().getRequest() returns.

So it's a bug that you don't implement them, surely?

Rupesh, Sep 10, 2013:
we don't have the implementation for PageContext or servletRequest.That comes from the underlying application server. You can see it for yourself by checking the cfdump of GetPageContext().getRequest()

Damon Miller, Sep 20, 2013:
This is the method ESAPI4CF uses for authentication. This bug means ESAPI4CF cannot support CF10. Using form.username/password is not an option. Referencing it directly would break encapsulation and username/password cannot be passed as arguments due to logging and that ESAPI4CF uses the same method to persist a user as it does to login a user.

Fixing this is a must in order for ESAPI4CF to support CF10.

Damon Miller, Sep 20, 2013:
Can I ask why this is marked as NeverFix, ThirdParty? Last I checked the GetPageContext function is supported and documented in the CF docs. What this function returns should also be considered supported. This worked in CF8, CF9, and works in Railo.

Me, Sep 24, 2013:
What's that supposed to mean, Rupesh? The very code you pointed us to DEMONSTRATES that these methods are a) there; b) supposed to work!

Here is the link to the docs in the implementation of PageContext you are using: http://tomcat.apache.org/tomcat-6.0-doc/api/org/apache/catalina/connector/RequestFacade.html#getParameter(java.lang.String)

Now... what's more likely to be the case here? Apache have screwed up the implementation of the underlying class (and this has gone unreported)? Or you have screwed up your integration of it?

Whilst getting cross about Rupesh's attitude here, I knocked out some code to try to work out what was going on.

Here's an example of using these methods upon the form scope:

<form method="post">
    <input type="text" name="txt1" value="">
    <input type="submit" name="btnSubmit" value="Submit">
</form>

<cfscript>
if (structKeyExists(form, "txt1")) {
    writeOutput("<h3>FORM</h3>");
    writeDump(var=form, label="form");

    pc = getPageContext();
    req = pc.getRequest();

    writeOutput("<h3>getParameter() via getParameterNames()</h3>");
    params =req.getParameterNames();
    writeOutput("params.hasMoreElements(): " & params.hasMoreElements());

    while (params.hasMoreElements()){
        thisParam = params.nextElement();
        writeDump(var=[thisParam, req.getParameter(thisParam)], label=thisParam);
    }


    writeOutput("<h3>getParameter() via form fields</h3>");
    for (field in form){
        writeDump(var=[field, req.getParameter(field)], label=field);
    }
}
</cfscript>

And the output (after submitting the form) on CF9:

FORM

form - struct
BTNSUBMITSubmit
FIELDNAMESTXT1,BTNSUBMIT
TXT1adam

getParameter() via getParameterNames()

params.hasMoreElements(): YES
btnSubmit - array
1btnSubmit
2Submit
txt1 - array
1txt1
2adam

getParameter() via form fields

FIELDNAMES - array
1FIELDNAMES
2[undefined array element] Element 2 is undefined in a Java object of type class coldfusion.runtime.Array.
BTNSUBMIT - array
1BTNSUBMIT
2[undefined array element] Element 2 is undefined in a Java object of type class coldfusion.runtime.Array.
TXT1 - array
1TXT1
2[undefined array element] Element 2 is undefined in a Java object of type class coldfusion.runtime.Array.

So we see that on CF9 getParameter() works fine. One just needs to be mindful of the casing of the parameter name being passed into the function, which means using the FORM scope is no use here as CF (stupidly) converts all the form field names to upper case. Still: if one uses the correct casing, it all works.

On ColdFusion 10, we get this (just the relevant bit):

params.hasMoreElements(): NO

The getParameternames() method returns an empty enumeration.

And on Railo 4.1, it's the same too! I'll follow that up separately.

Given Railo doesn't return anything here either,  I thought there was the slightest of chances there was something up with the way parameters were handled might be bung on Tomcat. But it seemed very odd that something so fundamental didn't work on Tomcat. It's a pretty solid product, after all.

On a whim, I decided to test what happened with URL variables instead of FORM scoped ones. I doctored that code to look at stuff passed on the URL instead, and got rather different results on ColdFusion 10 and Railo:

ColdFusion 10:

URL

URL - struct
foobar
moocow

getParameter() via getParameterNames()

params.hasMoreElements(): YES
moo - array
1moo
2cow
foo - array
1foo
2bar

getParameter() via URL keys

moo - array
1moo
2cow
foo - array
1foo
2bar

Railo:

URL

URL
Scope
foo
stringbar
moo
stringcow

getParameter() via getParameterNames()

params.hasMoreElements(): true
moo
Array
1
stringmoo
2
stringcow
foo
Array
1
stringfoo
2
stringbar

getParameter() via URL keys

foo
Array
1
stringfoo
2
stringbar
moo
Array
1
stringmoo
2
stringcow

So... getParameter() does actually work fine. As does getParameterNames(). It's just that both ColdFusion 10 and Railo seem to be doing something odd in their handling of posted parameters. And I wonder if it's something to do with this caveat right there in the docs:

If the parameter data was sent in the request body, such as occurs with an HTTP POST request, then reading the body directly via ServletRequest.getInputStream() or ServletRequest.getReader() can interfere with the execution of this method.
This sounds reasonable, but I'm not going to poke around in their codebase to work it out.

Here's the thing that really annoys me though, and hence the title of this article. Rupesh made no attempt to get to the bottom of this, he just spend his time making excuses along the lines of "not my job mate", whilst not even making a reasonable effort to explain why it's "not his job" (which, mate, it is): the docs he pointed us to demonstrated that the underlying code that he was blaming for this not working actually works exactly the way we expect it to. It's ColdFusion 10 not using it properly.

In contrast, I just know that if I raised this with Micha from Railo, he'd simply scratch his head, go "yeah, it's cos of this thing we did..." and from there a discussion would ensue as to how significant an issue it would be to fix: from both perspectives of the amount of work involved to do so, and how important it really is to fix, if there are other ways to arrive at the same end.

Another annoying thing about this is it's not even as if this is the first time this sort of attempt at blameshifting sleight of hand has gone on when I've had interaction from some quarters of the Adobe ColdFusion team regarding bugs. Now I do not mean to tar the whole team with the same brush - most of the bods are very stand-up, helpful and honest about what's going on. But the team is let down by a bit of a jobsworth attitude that crops up occasionally (but often enough that I am always watchful for it).

I think there's a regression bug here that has broken a community project, and Adobe should stop pitching excuses as to why it's not their fault, and perhaps instead have a look into why it is their fault. Then assess how tricky it would be to fix it, and if not so much of a problem: fix it. Or maybe they could work with the peeps from ESAPI4CF and understand why they are doing what they're doing and help them with a work-around. And stop with this "it's not our fault, it's someone else's fault" attitude. That's not community (or professional) spirit.

I'll go mention this lot to Micha to see what he thinks about this from a Railo perspective...

Cheers.

--
Adam