zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (ZOOKEEPER-3007) Potential NPE in ReferenceCountedACLCache#deserialize
Date Tue, 27 Mar 2018 08:27:00 GMT

    [ 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)

Mime
View raw message