Tag: Web Development

Top Code Review Issues

I figured I would blog about what I’ve been seeing during code reviews lately. Maybe someone else has been seeing too, or seeing other issues. I’ve omitted problems that deal with our shop’s internal best practices, and tried to stick with things that would apply more globally. If I over applied something let me know. It’s a long one, so more after the jump.

5. Improper implementation of Application.cfc inheritance

Application Inheritnace is very useful for application and sub applications development. However there is a gotcha to remember here. If you are going to override application variables that you set in onApplicationStart, they you need to give the application.cfc its own this.name. Why?

  • Imagine this:
  • Application.cfc
    • This.name=parent
    • OnApplicationStart
      • <cfset application.title = “Parent” />
  • Child/application.cfc extends=”../application”
    • OnApplicationStart
      • <cfset application.title = “Child” />

In this case, assuming the application is not started, going to a page under the child will trigger onApplicationStart. But since they have the same application name through inheritance, going to any parent page will not trigger OnApplicationStart, and they will be stuck with the application title of “child.”

The easy fix here is to always give children a different application name, only omitting it when you have a specific reason not to.

4. Improper Access on CFC methods

The four access types of CFC methods tend to cause people some confusion especially when they are learning the whole CFC thing. They each have different implication for application security.

I’ve pasted a more in depth guide below, but for the most part you probably shouldn’t set access=remote. Even if you are writing a webservice, only the exact method you are calling needs to be set to remote, so don’t set all of the methods called from your webservice to remote.

  • Private
    • Only accessible by other functions in CFC
    • Used for sub functions only relevant to CFC
  • Package
    • Only accessible by code in same folder
    • Any sort of organization makes this almost useless
  • Public
    • Accessible by any code on the server
    • Sounds scary, but relatively safe on a dedicate server
    • Use with caution on a hosted box.
  • Remote
    • Used for webservices.
    • If building webservices these should be accompanied by some security or logging.

3. Inappropriate use of a Query of Queries

Query of Queries have many legitimate uses. They include but are not limited to:

  • Performing SQL syntax queries on query objects returned from non-SQL operations. (Ex. <CFLDAP> <CFDIRECTORY>)
  • Joining the results of two queries made from two different sources (Ex. Joining MS SQL and Oracle data, or SQL and LDAP data.)
  • Providing multiple views to the same data on the same page request. (Ex. Showing a chart of data sorted multiple ways on the same page.)

However, where they get deservedly shunned is when they are reproducing functionality that can be completely handled in native SQL. In my experience, this is often (but not always) done by developers who know Cold Fusion very well, but not advanced SQL, therefore when they can’t get the problem solved in SQL they fall back on Cold Fusion to do what they need to get done. (At least that’s what I was doing when I got yelled at for it.)

In many cases, it takes less time to post the SQL request to the database server and get the response back then it does for Cold Fusion to process the request on an in-memory query. It’s counter-intuitive, but such is the power of indexing and SQL-based set operations. Case in point, when I was designing a dictionary check for a password change page, it made sense to load the entire dictionary into an application variable, and then do a query of queries on it. The process took 3000ms, or 3 seconds, which is slower than a request really should take. The same request against the database server took half that. Ultimately through indexing and query tuning, I got the process down to 100ms.

Your mileage may vary, especially depending on the size of the dataset, but the point remains that you should let SQL be SQL and Cold Fusion be Cold Fusion.

And by the way, I’m not being original here, other people have said this:

ColdFusion is Not a DBMS

2. Fake Email addresses in error or reporting pages

If your application sends email, those emails should be sent from a legitimate, addressable email address. (Whether or not the email goes to a list, a mailbox, or some other place is not a factor.) There are a couple of reasons for doing this:

  • It is not inconceivable in the future that your organization, hosting provider, or your user’s email host will start rejecting messages from invalid addresses due to spam considerations.
  • Any bounces generated from these messages will never get back to you.
  • Users responding to these messages will get ungraceful bounce responses.

Of these reasons, 1 is probably the most important as spam or virus concerns have forced mailing hosts to make quick decisions like this without consulting others, due to the severity of the issue. But this is one of those easy to fix low hanging fruit issue.

1. Lack of Var scoping in CFC methods

This is a pretty familiar issue to everyone. I don’t want to belabor it. As a matter of fact I’ll just link to a bunch of posts about it:

Additionally there is a tool you can use to help programmatically attack this problem:

VarScoper

Running a ColdFusion Shop Part 4

I received an email today, that reminded me of a topic I wanted to throw up on my Running A ColdFusion Shop series.

The email:

Subject: Mail Queues Need Attention

Body: Hoth Spool could be backed up.

query – Top 1 of 1 Rows
DATELASTMODIFIED DIRECTORY NAME SIZE
04/17/2007 05:43:54 PM C:CFusionMXMailSpool Mail22186.cfmail 719

To unblock spool:
Drain stop node(wlbs drainstop)
Restart Cold Fusion Service.
Restore node (wlbs start)

Make sure before doing this that the node is actually backed up.
One or two messages in the queue does not a blockage make.
But depending on the number of items and length of their stay, you can make the determination.

What this email reminded me to say was: DRY isn’t just for code.

We had this problem a while back. Every once in awhile, the mail queues on a random one of our ColdFusion servers would back up. The mail would remain stuck until the server was restarted. Developers and users started getting pissed about their application email being delayed.

The short term solution was to check the queues every once in awhile. Once the problem stopped occurring, we stopped checking… until it happened again. We looked for a hotfix, to no avail. We did this a couple times, and each time we got burned when we stopped being vigilant.

Finally I said “Screw it; I’m scripting a solution to it.” I started checking to see if files were in the spool directory of all of our servers. Then I had to make sure that the files had sat there for a little bit instead of just having been written there. Finally I had to send an alert with the needed fix.

No big deal, it’s not advanced programming, it’s not even particularly good code.

The point here is that it was running in a scheduled task… since I wrote it in 2005 (and restarted it earlier this year.) We had this problem again today, and nobody outside of my team noticed.

So what’s the point of this besides a little bragging? Don’t do things by hand that you can automate and forget about, or DRY isn’t just for code.

Running a ColdFusion Shop 3

For Reference: Part 1 and Part2.

It’s been awhile, but I’ve finally got more to say about all this.

Backup

In the last one of these, I told you to back up your system. Maybe not directly, but it was there. Okay, so I didn’t say it. I’m saying it now. Backup your machines. Do a tape backup, export to an external drive, do anything. Just backup your machines.

Backup Configuration Files Separately

Independent of a backup solution that takes care of your whole system, I would also recommend backing up configurations to separate location, in their current format. Meaning, don’t put them on a tape, don’t ship them off site. Just keep them somewhere you can access them. Why? Coldfusion is pretty light. CFML sites are comprised of pretty small files. Odds are, if you have a systems failure, it very well may be faster to rebuild a system from scratch then it will be to get a system backup restored and working. Rebuild your server, reinstall your webserver, and reinstall ColdFusion, all while getting the CFML files from backup. Then use configuration files you’ve stored elsewhere to restore both your webserver and Coldfusion to its old state. By this point, the CFML files should be out from a restore.

There are other uses for these files. You can use this method to clone your boxes, for clustering for example. Additionally, the ways they are broken up make it easy to share certain areas of configuration between machines. The most useful of these is neo-query.xml, which has all of your datasources on it.

Anyway, you probably want to know where all of these are:

  • ColdFusion configuration information is stored in xml files, in [CfusionRoot]lib.
  • IIS XML configurations can be exported from the IIS Manager
    • Right Click website
    • Choose “All Tasks”
    • Choose “Save Configuration to file.”

I’m sure that Apache has something to handle this too. But I don’t have any experience with it.

Backup Certificates

If you are using SSL certificates, back them up. They are a pain in the ass to manage. If you lose one without a backup you have to depend on the Certificate Authority to get your SSL service back up. Better to just have them ready to go in case of a problem.

Again I can only speak authoritatively about IIS, but:

  • Go to IIS Manager
  • Right click your SSL secured site
  • Choose “Properties”
  • Go to “Directory Security”
  • Under “Secure Communication” hit the “View Certificate” button
  • Choose “Details” tab
  • At the bottom “Copy To File”

That will bring up the “Certificate Export Wizard.” I usually choose the default options, with one exception. I choose “Yes export the private key.” It’s less secure, so you want to make sure you store them somewhere safe, but you’ll be able to do this again with a certification created from a backup with the private key.

That’s all I got this time around. Sorry it’s all backup related. Nothing happened; I was just collating tips for building a new Coldfusion box in our environment, and realized that these were all pretty important to doing that

CFUse CFTry CFCatch

I was in a code review earlier last week, and someone said that they didn’t like using <cftry> and <cfcatch>. When we as a group investigated further it was revealed that they had used it “incorrectly,” experienced problems for doing so, and labeled the try catch model as bad, instead of the implementation. So I figured I would quickly go over my opinions of what you should be using them for.

To start with, the incorrect usage of <CFTry> was wrapping it around a troublesome page, so instead of visibly erroring, the page would just silently fail. To add insult to injury, there was an error in the cfcatch block which caused the whole thing to throw an application level error anyway. This was, to put it mildly, a flawed use of <cftry> and <cfcatch>.

In a nutshell, the <cftry> and <cfcatch> model in ColdFusion is for handling expected errors, as opposed to handling unexpected errors. ColdFusion has a pretty good system for handling unexpected errors – a combination of <cferror> tags, and use of onError in the application.cfc. So wrapping an entire page in <cftry> to catch any error that may come up is not only the wrong thing to do, it’s also pretty redundant.

In my opinion <cftry> and <cfcatch> should be used to handle errors around small pieces of code that have the potential to error in predictable ways. For example here is a line of code from one of my applications:

cfset Wsdl = “<webservice URL>” />

<cfset displayBrainstorm = FALSE />

<cftry>

    <cfinvoke timeout=“1” webservice=“#Wsdl#” method=“AuthorInfo” returnvariable=“BrainstormStruct”>
        <cfinvokeargument name=“username” value=“#url.username#” />
    </cfinvoke>

    <cfset displayBrainstorm = BrainstormStruct.IsAuthor />
    <cfcatch type=“any”>
        <cfif FindNoCase(“stub objects”, cfcatch.Message)>
            <cfset createObject(“java”,“coldfusion.server.ServiceFactory”) .XmlRpcService.refreshWebService(Wsdl) />

            <cftry>
                <cfinvoke timeout=“1” webservice=“#Wsdl#” method=“AuthorInfo” returnvariable=“BrainstormStruct”>
                    <cfinvokeargument name=“username” value=“#url.username#” />
                </cfinvoke>

                <cfset displayBrainstorm = BrainstormStruct.IsAuthor />

                <cfcatch type=“any”>
                    <cfrethrow />
                </cfcatch>

            </cftry>

        <cfelse>
            <cfrethrow />
        </cfif>

    </cfcatch>

</cftry>


In it you’ll notice I’m doing a webservice call. In my experience webservices can have issues with the skeleton ColdFusion creates to consume them, especially if details about the webservice change. However, my application has little control over this webservice. So I wrap the call to the webservice in a <cftry> block, and if my expected error condition occurs, (something with to do the skeleton, or “stub objects”) I reset the webservice and try again.

You’ll notice that if it isn’t a “stub objects” problem, it assumes that it is some other issue, and rethrows the error so the application error handlers can tackle the problem. Why, because it is an unexpected problem, not an expected one.

In conclusion, I’m not speaking for all programming languages, but at least in ColdFusion <cftry> and <cfcatch> should be used to handle error that you can predict. The shouldn’t be your default error handling technique, and like a <Cflock> or a <cftransaction> they should surround as little code as possible.

Whitespace – What’s the big deal?

This came up in a code review today. There were a couple misunderstandings about it, so I figured I would share the explanation publicly.

First, there are two types of whitespace what are in play here. Whitespace in your code for the purpose of properly formatting, and extra whitespace in your ultimate HTML output that comes from the various loops and other mysterious sources in ColdFusion. In simple terms, the first is good and the second is bad. However, you shouldn’t try and get rid of the first in order to get rid of the second.

First, to see a demonstration of what I am talking about, go to Wharton Computing’s Brainstorm. Once you’re there, “View Source” from your browser. At the top of the page, you’ll see a few lines of empty space before the DocType declaration. Select them with your cursor. You’ll see they’re not just page breaks, but spaces and tabs.

Now, go to the Wharton Wireless Troubleshooter (WWT). Do the same thing. You’ll notice a lot more whitespace at the top.

(I’m not picking on some random developer; I did both of these applications.)

That’s the easiest way to show you what I’m talking about. That whitespace makes the output HTML page bigger, as each piece of whitespace is a character that you are sending to you user.

Well how much of a problem is that little bit of whitespace? Well frankly not much, but if you understand how it gets created, you realize it can become a very big deal.

It gets created in loops that, in the case of the WWT, are being called before the header is rendered. (It can happen anywhere in a page, but that’s where it’s happening in this application.) So in the case of the WWT a few loops are generating say 1 character of whitespace per iteration. But suppose for a moment that you were doing hundreds or thousands of iterations… In multiple loops… with multiple characters per iteration… in multiple included files.

Like everything in coding, it’s not that one little thing is so bad, it’s that the little bad things tend to aggregate together, form large bad things, gain sentience and run amuck!

So what can you do to prevent whitespace? I could tell you. But Ray Camden does such a better job of it in his article about whitespace.

I would just add two things.

  1. Ray mentioned a whole bunch of techniques you can use to reduce whitespace, but he doesn’t say what not to do to reduce it. (Awkward sentence, but bear with me.) You should not start formatting your source code to reduce the number of lines you use, the number of tabs, or the number of spaces. Readability of your code is still extremely important. Don’t change your code formatting to fix your html formatting. Find the spots that generate the whitespace. Then utilize the techniques he mentions to treat the problem topically. (My favorite method is to insert the <yo /> tag randomly throughout the source.)
  2. If you’re not sure if you have this problem with your page, or if you’re not sure how big an impact the whitespace is having, use the tools at Web Site Optimization to see how big your output page is. Alternately this can be accessed via the Firefox Web Developer Toolbar.

Jupiter Rising

I have found reason 531 to be glad I switched to developing in CFEclipse. Its name is Jupiter. It’s a code review tool for Eclipse. I find myself in complete disbelief, but I think I love a code review tool.

Why? What does do that makes me love it?

  1. It links annotations directly to the code via line numbers.
  2. Because of that linkage, you can open the code directly from the code comments.
  3. It allows several levels of severity and categories of comments.
  4. It allows for teams to collaborate on code reviews.
  5. It saves notes in an easily parsed XML file for external reporting.

Dave and I are experimenting with it. It looks like it will make all of the code reviews we have to do a little easier.

I have to thank Kola Oyedeji who made some observations about a previous post of mine regarding code reviews.

Squidhead Swims On

I’ve done three updates to Squidhead since the last time I blogged about it. Two of them were pretty major so I figured I would squawk some more about it.

First, I haven’t forgotten about adding support for other DBMS’s other than Microsoft SQL. I haven’t done it, but I have re-architected Squidhead slightly to make it a bit more possible to do so down the road. There’s one more step I need to take to open that up, which is focusing on primary key’s rather than identity. I know it should work, but I’m just not ready to pull the trigger on it.

Second, I’ve added quite a few features around the main unique feature of Squidhead, stored procedures. The whole focus of Squidhead is pulling in user created stored procedures and creating the ColdFusion code to support them. The recent changes I made allow Squidhead to properly handle store procedures that return multiple result sets. Alternately it can handle multiple output parameters, from the stored procedures.

Third, I’ve got a Build a Blog in 15 minutes presentation in the works. It’s a respectful homage, to Joe Rinehart’s
YouTube presentation on ModelGlue. I figure it’s effective in getting the feature set across, and appropriate since Joe’s presentation got me into code generation and frameworks in the first place.

Finally, I have a pretty good idea of what’s coming down the pike for Squidhead. Before I add support for other DBMS, I’m going to try and flush out the feature set, so as to avoid writing multiple versions in tandem. I prefer to port over the full version to other databases. My current list of updates includes support for foreign and primary keys. I’m also looking to add some form validation. That being said, if anyone has any thoughts, I’m very open to them.

JVM Update and LDAP SSL

I had a problem this week due to recent updates for DST. LDAP over SSL stopped working. It took me a fair amount of time to track down the answer, so I figured I would share.

Basically, the secure certificates store is tied to the JVM, not to ColdFusion or even JRUN. So when you update the JVM you lose a bunch of Certificate Authorities in your keystore (I assume since the file is so much smaller) and you lose any custom Certificate Authorities you may have added.

I found the solution at via a Google search at sagewire.org (JVM upgrade breaks secure CFLDAP connections), but basically the solution is to copy your former copy of the cacerts files to the new JVM’s location, then restart ColdFusion.