読者です 読者をやめる 読者になる 読者になる

この日記は私的なものであり、所属会社の見解ではありません。 GitHub: takahashikzn

OGNL式の評価中に発生した例外が無視される件

Struts2


いやー、1時間ほどハマりました。


JSP内で、Actionクラスのgetterでプロパティ値を取得し表示するという単純なコードで、
値がなぜか常にnullになるという現象に悩んでいたのですが。

原因は、getter内でRuntimeExceptionが発生していたからでした。

OGNLの罠

では、何でHTTP500エラーにならないの?もしや…
と思っていたら案の定、OGNL(というかxwork)が例外を握り潰していたのでした。


以下、該当箇所をxwork-2.1.2から抜粋。

com.opensymphony.xwork2.ognl.OgnlValueStack

public Object findValue(String expr) {
    try {
        if (expr == null) {
            return null;
        }

        if ((overrides != null) && overrides.containsKey(expr)) {
            expr = (String) overrides.get(expr);
        }

        if (defaultType != null) {
            return findValue(expr, defaultType);
        }

        Object value = ognlUtil.getValue(expr, context, root);
        if (value != null) {
            return value;
        } else {
            checkForInvalidProperties(expr);
            return findInContext(expr);
        }
    } catch (OgnlException e) { // 例外を握りつぶしている個所 その1
        checkForInvalidProperties(expr);
        return findInContext(expr);
    } catch (Exception e) {     // 例外を握りつぶしている個所 その2
        logLookupFailure(expr, e);

        return findInContext(expr);
    } finally {
        ReflectionContextState.clear(context);
    }
}


『先生!例外を握りつぶすのはいけないと思います!』

などと考えていたら、これは2年前に議論済みでした。


どっちの言い分も間違っちゃいないとは思うのですが、それでも例外を握りつぶすのはアカン、というのが僕の意見です。
だって、バグが出ても判りづらいし。今回みたいに。

xwork最新版に期待

ちなみに最新版では例外をスローするかどうかを設定できる模様です。
以下、抜粋。

public Object findValue(String expr, Class asType, boolean throwExceptionOnFailure) { // ← 3つ目の引数に注目!!!
    try {
        if (expr == null) {
            return null;
        }

        if ((overrides != null) && overrides.containsKey(expr)) {
            expr = (String) overrides.get(expr);
        }

        Object value = ognlUtil.getValue(expr, context, root, asType);
        if (value != null) {
            return value;
        } else {
            checkForInvalidProperties(expr, throwExceptionOnFailure, throwExceptionOnFailure);
            return findInContext(expr);
        }
    } catch (OgnlException e) {
        checkForInvalidProperties(expr, throwExceptionOnFailure, throwExceptionOnFailure);
        return findInContext(expr);
    } catch (Exception e) {
        logLookupFailure(expr, e);

        if (throwExceptionOnFailure)
            throw new XWorkException(e);
        
        return findInContext(expr);
    } finally {
        ReflectionContextState.clear(context);
    }
}