Scanning For Invalid Property Access in JSP EL Expressions V1

In applications that use JSP we often find ourselves using an object instance and accessing properties in EL expressions. For example:

<input value="${address.line1}" ... />

The problem is that if address doesn't have a getLine1() method this fails at runtime. If someone decides to rename a method our JSP may break without us knowing until someone runs the app and sees an error, particularly if the renaming was done in a library project. More commonly, someone renames something and doesn't quite find all the places it was referenced in our gloriously unchecked JSPs. Accordingly it would be really nice if we could find a way to be automatically notified of property file misuse.

A fancier version will ultimately be built but our for our first proof-of-concept version we want to crank out a quick and dirty JUnit test that will fail if we use a non-existent property. Our first problem to overcome is that even if we scan for EL expressions that look like object field accesses we can't tell what type is being used. Our super-simple solution will be to simply have this explicitly expressed in our JSPs: we'll use a server-side comment that provides this information to our scanner.

<%-- TYPECHECK(myVar,com.example.package.MyType) --%>

The first draft of the scanner will simply search our source tree for TYPECHECK blocks using a regular expression. To find the TYPECHECK blocks we can use an expression similar to:

private static final Pattern checkDecl = Pattern.compile("(?m)^\\s*<%--\\s+TYPECHECK\\s*\\(\\s*(\\w+)\\s*,([\\w.]+)\\s*\\)\\s*--%>\\s*$");

For the record, Java-escaped regular expressions make babies weep and programmers wish for a verbatim-string mechanism, ala C#. Note that we establish capturing groups for varname (group(1)) and type (group(2)) to help pull values out of our Matcher.

In the source tree we want to scan we happen to know that we can find our source dir offset from the / resource. We can then scan *.jsp for TYPECHECK declarations and scan the files using TYPECHECK for accesses to varname that reference non-existent properties. The following listing does just that:

package com.example.validate.jsp;

import static org.junit.Assert.assertEquals;

import java.io.File;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.apache.commons.io.FileUtils;
import org.apache.commons.lang.StringUtils;
import org.junit.BeforeClass;
import org.junit.Test;

import ...internal package....ReflectionHelper;

public class JspTypeCheckTestCase {
 private static final Pattern checkDecl = Pattern.compile("(?m)^\\s*<%--\\s+TYPECHECK\\s*\\(\\s*(\\w+)\\s*,([\\w.]+)\\s*\\)\\s*--%>\\s*$");
 private static final Pattern elExpr = Pattern.compile("\\$\\{([^}])+\\}");
 private static final String useExprTemplate = "%1$s\\.(\\w+)";
 private static File srcDir;
 
 @BeforeClass
 public static void beforeClass() throws Exception {
  srcDir = new File(new File(JspTypeCheckTestCase.class.getResource("/").toURI()), "../../src/main/webapp").getCanonicalFile();
 }
 
 /**
  * Look for JSP files declaring <%-- TYPECHECK(varname,qualifiedJavaType) --%>; for all found validate that for all 
  * expressions of the form varname.something something exists on qualifiedJavaType as a JavaBean's style accessor 
  * (eg a public T isSomething() or public T getSomething() method exists) 
  * @throws Exception
  */
 @Test
 public void doTypeCheck() throws Exception {
  int numFailures = 0;
  //examine src/main/webapp/**/*.jsp files
  @SuppressWarnings("unchecked")
  Collection<File> filesToExamine = (Collection<File>) FileUtils.listFiles(srcDir, new String[] { "jsp" }, true);
  for (File fileToExamine : filesToExamine) {
   String content = FileUtils.readFileToString(fileToExamine);
   Matcher matcher = checkDecl.matcher(content);
   while (matcher.find()) {
    String fullTypeCheck = matcher.group();    
    String varName = matcher.group(1);
    String typeName = matcher.group(2);
    
    Class<?> type = null;
    try {
     type = Class.forName(typeName);
    } catch (Exception e) {
     System.out.println("Bad typecheck in " + fileToExamine + ": " + fullTypeCheck
       + " (" + e.getClass().getName() + " - " + e.getMessage() + ")");
     numFailures++;
    }
    
    List<String> expressions = new ArrayList<String>();
    Matcher exprMatcher = elExpr.matcher(content);
    while (exprMatcher.find()) {
     expressions.add(exprMatcher.group(0));
    }
    
    if (null != type) {
     for (String expression : expressions) {
      numFailures += checkUseOfType(fileToExamine, type, varName, expression);
     }
    }
   }
  }
  assertEquals(numFailures + " typecheck failures in JSP files", 0, numFailures);  
 }

 private int checkUseOfType(File file, Class<?> type, String varName, String expression) {
  int numFailures = 0;
  
  Pattern usagePattern = Pattern.compile(String.format(useExprTemplate, varName));
  Matcher m = usagePattern.matcher(expression);
  while (m.find()) {
   String pptyName = m.group(1);
   String methodName = StringUtils.capitalize(pptyName);
   if (null == ReflectionHelper.findMethod(type, "^(is|get)" + methodName + "$", new Class<?>[] {}, null)) {
    numFailures ++;
    System.out.println(file.getName() + ": " + type.getName() + " has no accessor for " + methodName
     + "; bad expression: " + expression);
   }
  }
  
  return numFailures;
 }
}

A couple of notes about the source:

• It's very much a quick/dirty/non-optimized test to see if this idea is viable and more importantly if we find having such a scan helpful day to day. It sounded good on paper but lots of things meet that criteria ;)
• ReflectionHelper.findMethod is an internal API that looks for a method with a given name, arguments, and return type. In our case we are simply scanning for an instance method named getBlah or isBlah if we find use of varname.blah in our jsp
• If the test fails it will print the offending expressions to stdout similar to: myfile.jsp: com.example.package.MyType has no accessor for A; bad expression ${myvar.A or myvar.B}
• The test will fail if there are > 0 bad accesses with an assertion error similar to java.lang.AssertionError: 2 typecheck failures in JSP files expected:<0> but was:<2>
• The test is typically run on Java 1.6, with commons-lang 2.4 and commons-io 1.4 on classpath.

If the scan proves helpful we speculate that it may evolve to be run directly in Eclipse using a custom builder. This would give us "live" reports of bad accesses in JSPs in the problems view. Heck, maybe we could even run the EL parser on our expressions and see if our EL is valid too...?