[
https://issues.apache.org/jira/browse/ZOOKEEPER-3007?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16415220#comment-16415220
]
ASF GitHub Bot commented on ZOOKEEPER-3007:
-------------------------------------------
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/495
@brettKK Thanks for the fix. It'd be nice to add at least a unit test to
cover the issue.
I think adding the check alone is not enough here. Looking at the
`serialize()` method, if `map` field is greater than 0, both `long` and `acls`
fields must also be present.
In other words, in `deserialize()` if (i>0) then both `long` and `acls` are
mandatory. As a consequence the else branch of the check should also be
implemented and an exception should be thrown indicating that the archive
cannot be deserialise, because the format is incorrect.
Does it make sense?
> Potential NPE in ReferenceCountedACLCache#deserialize
> ------------------------------------------------------
>
> Key: ZOOKEEPER-3007
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3007
> Project: ZooKeeper
> Issue Type: Bug
> Affects Versions: 3.6.0
> Reporter: lujie
> Priority: Major
>
> Inspired by ZK-3006 , I develop a simple static analysis tool to find other
> Potential NPE like ZK-3006. This bug is found by this tool ,and I have
> carefully studied it. But i am a newbie at here so i may be wrong, hope
> someone could confirm it and help me improve this tool.
> h3. Bug describtion:
> callee BinaryInputArchive#startVector will return null:
> {code:java}
> // code placeholder
> public Index startVector(String tag) throws IOException {
> int len = readInt(tag);
> if (len == -1) {
> return null;
> }
> {code}
> and caller ReferenceCountedACLCache#deserialize call it without null check
> {code:java}
> // code placeholder
> Index j = ia.startVector("acls");
> while (!j.done()) {
> ACL acl = new ACL();
> acl.deserialize(ia, "acl");
> }{code}
> but all the other 14 caller of BinaryInputArchive#startVector performs null
> checker like:
> {code:java}
> // code placeholder
> Index vidx1 = a_.startVector("acl");
> if (vidx1!= null)
> for (; !vidx1.done(); vidx1.incr()){
> .....
> }
> }
> }
> {code}
> so i think we also need add null check in caller
> ReferenceCountedACLCache#deserialize just like other 14 caller
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
