As you may or may not know, CFML has a bunch of headless functions for encoding strings according to OWASP security recommendations, eg:
These are - for all intents and purposes - wrappers for equivalent methods in OWASP's own Java library: org.owasp.esapi.reference.DefaultEncoder.
You should always use one of these functions when you are going to render untrusted content (user input, data from third-party APIs, databases, etc), eg don't do this:
Kia ora <cfoutput>#form.firstName# #form.firstName#</cfoutput>!
Do this:
Kia ora <cfoutput>#encodeForHtml("#form.firstName# #form.firstName#")#</cfoutput>!
Similarly if the value is going into JS, use
encodeForJavaScript(), or a URL: encodeForUrl().Also note these supercede older functions like
htmlEditFormat() and urlEncodedFormat(), which do not do the job thoroughly, and should be actively removed from your codebase.All good.
One thing that CFML has not had thusfar though are an OO approach to calling these functions, via a method rather than a headless function. It's really no great shakes that it doesn't, but it would be good if Adobe had done a thorough / complete job of implementing OO coverage of CFML's functionality. This is beginning to seem like their approach to CFScript coverage: it took about seven ColdFusion versions for them to complete the job, in the mean time CFML was marginalised as a language due to the play-school-looking code us CFMLers had to write our applications in.
There's a ticket in the bugbase to implement "member function" versions of these encoding functions: "Member functions for encoding". Cheers to Neil for raising that.
Unfortunately the Adobe ColdFusion Team have demonstrated their lack of OO nous in their implementation. Or lack of willingness to engage in language design. Or... really... preparedness to do good work.
The thought process seems to have been "well one passes a string to these functions, therefore they're clearly String member functions. We'll add them to the String class".
Well... no. A String class should have methods which describe the behaviour of a String object. Any String object so its methods should focus on ubiquious / generalised functionality relating to any string. Like finding & replacing sub-strings, comparing strings, etc. You know: the sort of general stuff that java.lang.String provides. Note how java.lang.String isn't a catch-all for every function which happens to take a string as an argument. That's not how OO works.
These encoding functions don't apply to most strings. They only apply to strings being used for a special purpose: a special sort of String, not the general String implementation. But these encoding functions are tricky ones. If there was a class "EncodableString" or something, then they'd be object methods of those. But what's the difference between a String and an EncodableString other than these functions? I don't think there's a decent case for that.
So what was OWASP's Java approach? Well one creates (or gets) an instance of an Encoder, and then passes the String to the encoder via the appropriate method. This makes sense in Java, and fits with the rest of the language. It looks a bit odd in CFML though:
<cfset myEncoder = new Encoder()>
Kia ora <cfoutput>#myEncoder.encodeForHtml("#form.firstName# #form.firstName#")#</cfoutput>!
Well it doesn't look odd, but it would be the first of a precedent of having built-in CFML classes which one creates instances of. Still: this sort of thing is where CFML should have gone with the release of CFMX 6.0: if they're in for a penny with OO, they should have been in for a pound.
So perhaps Adobe could have taken that approach.
However this still doesn't sit well with me. I rather more think that these methods are better suited to be static methods which one calls on the class. For CFML's needs there is no "object" needed here, as the Encoder doesn't have any stateful information. So I rather think the functions should be implemented as static methods:
Kia ora <cfoutput>#Encoder.encodeForHtml("#form.firstName# #form.firstName#")#</cfoutput>!
This follows on from the previous example: there's an Encoder class, and we just call the method on the class itself. I've suggested this in the ticket.
As is increasingly obvious, Adobe too often charges off and does work without public consultation, and then present some half-baked, half-working, design-absent solution. They have done this here too. There was never any community discussion, they simply did the work. And they plan to release it in ColdFusion 2016.
Adobe finally followed-up on the ticket (after a bit of should-be-unnecessary badgering), and seem to concede my suggested alternative makes sense. Cool. Except they also seem to be suggesting they're gonna release this current incorrect implementation in the mean time, and then - in a later ColdFusion release - look at fixing it. This is even more ill-conceived than their initial implementation! Once they release this stuff, it's out in the wild, and people will use it. Why would they release something that's been done wrong (you might think "wrong" is a heavy-handed description, but from a language design perspective, I don't see how it can be considered "right"?)? If they're short on time and/or resource, then they should simply park the work until they have time to do it properly. No-one's in a rush for them to make CFML just that little bit more sh!t, are they? I can only surmise they're unprepared to concede "yeah that work didn't pass muster so should not be released", because that'd be an admission of error.
What I'd like people to do is to give this issue some thought, form yer own opinion, and express it on the ticket. From my perspective I'd like to vote for not releasing this solution until it's done right. I'd rather have no implementation than a bad one. We need to discourage Adobe form actually releasing sub-par work.
So have a vote and a comment if you feel like it (hey, vote and comment even if you don't agree with me; I'm not trying to ballot-stuff here. Well: maybe a bit ;-).
Righto.
--
Adam
