[
https://issues.apache.org/jira/browse/ZOOKEEPER-3504?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16905961#comment-16905961
]
Jürgen Wagner commented on ZOOKEEPER-3504:
------------------------------------------
Generally, either way works. The fact that one part of the code has the
LOG.isDebugEnabled() check, while another doesn't, is something I would place
in the domain of personal styles. The issue you raised does not sound like a
security problem in any case.
It is beneficial to use the check if the enclosed statement would perform some
computation which may not be necessary in case of the debug mode being
disabled. In the easiest case, that's something like the string concatenation
you quoted above. You still see this in the code in some places
[https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java]
However, the preferred method of avoiding the string concatenation is the use
of "{}" parametrized log strings.
PS: The fact that Zookeeper is used in productive systems does not mean you
want "debug" mode switched on there. It also does not tell you how operators of
such instances actually secure their instances and log services. That's an
operational topic.
> An information leakage from FileTxnSnapLog to log:
> --------------------------------------------------
>
> Key: ZOOKEEPER-3504
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3504
> Project: ZooKeeper
> Issue Type: Bug
> Components: security, server
> Affects Versions: 3.4.11, 3.4.12, 3.4.13, 3.5.5, 3.4.14
> Reporter: xiaoqin.fu
> Priority: Major
>
> In org.apache.zookeeper.server.persistence.FileTxnSnapLog, the statement
> LOG.debug don't have LOG controls:
> public void processTransaction(TxnHeader hdr,DataTree dt,
> Map<Long, Integer> sessions, Record txn)
> throws KeeperException.NoNodeException {
> ......
> if (rc.err != Code.OK.intValue()) {
> LOG.debug("Ignoring processTxn failure hdr:" + hdr.getType()
> + ", error: " + rc.err + ", path: " + rc.path);
> }
> ......
> }
> Sensitive information about hdr type or rc path was leaked. The conditional
> statement LOG.isDebugEnabled() should be added:
> public void processTransaction(TxnHeader hdr,DataTree dt,
> Map<Long, Integer> sessions, Record txn)
> throws KeeperException.NoNodeException {
> ......
> if (rc.err != Code.OK.intValue()) {
> if (LOG.isDebugEnabled())
> LOG.debug("Ignoring processTxn failure hdr:" +
> hdr.getType()
> + ", error: " + rc.err + ", path: " + rc.path);
> }
> ......
> }
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
