I want the feature that was implemented in #1953.

But it's already closed by this message:

Um...this feature seems to discourage the use of ${}, however, the right way to go is to learn/understand how ${} works and use it correctly.

@harawata said:

We can reconsider at any time if this turns out to be a popular request. :)

I posted the comment on the closed #1953 but there's no reaction. As a result, I create a new issue.

Comment From: harawata

Hello @tokuhirom ,

I still believe this should not be a built-in feature. As mentioned in the linked issue, string substitution is a crucial feature and we should not discourage its usage in any way.

If it is super-important to your team, you may be able to write a simple language driver and use it in the development phase.

import org.apache.ibatis.builder.BuilderException;
import org.apache.ibatis.mapping.SqlSource;
import org.apache.ibatis.parsing.XNode;
import org.apache.ibatis.scripting.xmltags.XMLLanguageDriver;

public class NoobLanguageDriver extends XMLLanguageDriver {
  @Override
  public SqlSource createSqlSource(Configuration configuration, XNode script, Class<?> parameterType) {
    doCheck(script.toString());
    return super.createSqlSource(configuration, script, parameterType);
  }

  @Override
  public SqlSource createSqlSource(Configuration configuration, String script, Class<?> parameterType) {
    doCheck(script);
    return super.createSqlSource(configuration, script, parameterType);
  }

  private void doCheck(String script) {
    if (script.contains("${")) {
      throw new BuilderException("Found ${} : " + script);
    }
  }
}

This approach has some advantages over an out-of-box solution, IMHO.

  • It has no impact on production performance whatsoever.
  • It should be possible to allow ${} in particular statements. It's highly likely that people want this capability (cf.).

Comment From: tokuhirom

But so, we may use $ {} in some cases.

For example: SELECT * FROM foo ORDER BY ${column} DESC. It's the correct way to use ${}. It's a very common use case.

If all of the developers can care about the value of the $column, there's no problem. But humans make mistakes. I want a way to use injectionFilter. Since I want to allow ${} in some cases, but I want to deny the illegal value, that taken by the variable.

And performance is not a big deal. In most of cases, the regular expression matching cost is negligible compared to the communication cost with the DB server.

Comment From: harawata

As the language driver receives the entire statement, you should be able to allow particular parameter ...

private static final Pattern pattern = Pattern.compile("\\$\\{(.*?)}");

private void doCheck(String script) {
  Matcher matcher = pattern.matcher(script);
  while(matcher.find()) {
    if (!allowedNames.contains(matcher.group(1))) {
      throw new BuilderException("Found ${} : " + script);
    }
  }
}

... or particular statement.

private void doCheck(String script) {
  if (!script.contains("${")) {
    return;
  }
  try {
    String hash = HexFormat.of().formatHex(
        MessageDigest.getInstance("md5").digest(
            script.getBytes(StandardCharsets.UTF_8)));
    if (!allowedHashes.contains(hash)) {
      throw new BuilderException("Found ${} : " + script);
    }
  } catch (NoSuchAlgorithmException e) {
    e.printStackTrace();
  }
}

(these are just for demonstrating ideas and are not tested)

Comment From: liudongmiao

@tokuhirom I think you may need a custom OGNL to scan static XML or Java Annotation.

We have defined a custom ClassLoader to modify the MyBatis' way. In our cases, we are doing more. Analysis the XML without actual parameter to get SQL, then check SQL grammar, including dollar.

Comment From: mengmengdelaozhao

@liudongmiao Can you share how you solved this problem through a custom class loader? I also encountered this problem recently.