countQuery() returns System.QueryException: unexpected token: ':'

Question

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;
}

Answer 1

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).