[SalesForce] countQuery() returns System.QueryException: unexpected token: ‘:’

Scenario:

While updating some code to satisfy SOQL injection issues brought up by security review report, I stumbled across a code that was not brought up. I thought it was similar to the code in the report, so I proceeded to apply the same change. In this case, the recordId originated from a URL parameter while the objName is not. This is the code before:

private static Boolean valId(String recordId, String objName) {
    String id = String.escapeSingleQuotes(recordId);
    if((id.length() == 15 || id.length() == 18) && Pattern.matches('^[a-zA-Z0-9]*$', id)) {
        return (database.countQuery('SELECT count() FROM ' + objName + ' WHERE Id = \'' + id + '\'') > 0) ? true : false;
    }
    return false;
}

This is the code after making the changes:

private static Boolean valId(String recordId, String objName) {
    String id = String.escapeSingleQuotes(recordId);
    if((id.length() == 15 || id.length() == 18) && Pattern.matches('^[a-zA-Z0-9]*$', id)) {
        return (database.countQuery('SELECT count() FROM :objName WHERE Id = :id ') > 0) ? true : false;
    }
    return false;
}

Problem:

I am getting the following error:

FATAL_ERROR System.QueryException: unexpected token: ':'

I get the same error when I simplified the code in an Execute Anonymous Window

String objName = 'User';
system.debug(database.countQuery('SELECT count() FROM :objName'));

Other Observations:

It seems countQuery() returns an error when a colon-prefixed variable is used in the FROM section of the query. I was able to run the query successfully by hard-coding the Object name. And after trying the alternative (see Attempted Fixes), AggregateResult also has the same restriction.

Attempted Fixes:

I've looked at the question posted Return type of count() versus count(fieldName) and the alternative seems to be using an AggregateResult list.

Question:

How do I keep my code dry for this particular method without relying on SOQL injection code practice?

Update:

So here's the workaround I used… sigh

private static Boolean valId(String recordId, String objName) {
    String id = String.escapeSingleQuotes(recordId);
    if((id.length() == 15 || id.length() == 18) && Pattern.matches('^[a-zA-Z0-9]*$', id)) {
        if (objName == 'User') {
            return (database.countQuery('SELECT count() FROM User WHERE Id = :id') > 0) ? true : false;
        } else if (objName == 'Report') {
            return (database.countQuery('SELECT count() FROM Report WHERE Id = :id') > 0) ? true : false;
        }
    }
    return false;
}

Best Answer

There's really no reason that you need to do all that fancy footwork. You can just trust the system to tell you what you need to know. Here's an implementation that I whipped up for you:

public class Utils {
    static Boolean validId(String recordIdString) {
        try {
            Id recordId = (Id)recordIdString;
            String sobjectName = String.valueOf(recordId.getSObjectType());
            return Database.countQuery('SELECT COUNT() FROM '+sobjectName+' WHERE Id = :recordId') > 0;
        } catch(Exception e) {
            return false;
        }
    }
}

I've also posted this as a gist.

The try-catch block will catch the TypeException if it's not a valid Id (from the cast), or a NullPointerException if it's a null Id, and possibly a QueryException if the Id points to an object that can't be queried (rare, but they are there).

I'm pretty sure that even the automated scanner won't give you a red flag for this code, but even if it did, a simple review should clearly identify that no injection is possible as long as it's used in the appropriate sharing context; user-facing code should be "with sharing" to prevent leaking Id values they should't know about.

You can actually do this without a try-catch as well:

static Boolean validId(String recordIdString) {
    return recordIdString instanceOf Id &&
         Id.valueOf(recordIdString).getSObjectType().getDescribe().isQueryable() &&
         0 < Database.countQuery(
             'SELECT COUNT() FROM '+
             String.valueOf(Id.valueOf(recordIdString).getSObjectType())+
             ' WHERE Id = :recordIdString');
    }
}

I find this to be a little less readable, but is Exception-free and probably better for performance.

Note: Id.valueOf is broken in older versions of the API, so make sure you're using a recent version of the API for your class (e.g. version 37.0).

Related Topic