Преглед изворни кода

ZOOKEEPER-3009: Potential NPE in NIOServerCnxnFactory

We have developed a static analysis tool [NPEDetector](https://github.com/lujiefsi/NPEDetector) to find some potential NPE. Our analysis shows that NPE reason can be simple:some callees may return null directly in corner case(e.g. node crash , IO exception), some of their callers have  !=null check but some do not have.
### Bug:
Callee getSocketAddress can return null, may caused by node crash, network exception....
<pre>
    public InetAddress getSocketAddress() {
        if (sock.isOpen() == false) {
            return null;
        }
        return sock.socket().getInetAddress();
    }
</pre>
getSocketAddress has two callers:  removeCnxn and removeCnxn

caller removeCnxn has null check
<pre>
 public boolean removeCnxn(NIOServerCnxn cnxn) {
        //other code
        InetAddress addr = cnxn.getSocketAddress();
        if (addr != null) {
            Set<NIOServerCnxn> set = ipMap.get(addr);
            //other code
        }
     //other code
    }
</pre>

but caller addCnxn has the similar code, but don't have the null check:
<pre>
     private void addCnxn(NIOServerCnxn cnxn) {
         InetAddress addr = cnxn.getSocketAddress();
         Set<NIOServerCnxn> set = ipMap.get(addr);
        //other code
      }
</pre>
I commit a patch to fix it: adding an null checker in addCnxn, and throws a semantics IOException rather than the ugly NPE.  I think the IOException  is ok, because the caller of addCnxn has the handler code:
<pre>
  private void processAcceptedConnections() {
               //other code
               try {
                    addCnxn(cnxn);
               } catch (IOException e) {
                    // register, createConnection
                    cleanupSelectionKey(key);
                    fastCloseSock(accepted);
               }
             }
</pre>
Maybe i am wrong, please point out my error.

Author: LJ1043041006 <1239497420@qq.com>

Reviewers: Patrick Hunt <phunt@apache.org>, Allan Lyu <fangmin@apache.org>, Michael Han <hanm@apache.org>

Closes #525 from lujiefsi/ZOOKEEPER-3009
LJ1043041006 пре 7 година
родитељ
комит
13dd5d0db7
1 измењених фајлова са 4 додато и 1 уклоњено
  1. 4 1
      src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java

+ 4 - 1
src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java

@@ -815,8 +815,11 @@ public class NIOServerCnxnFactory extends ServerCnxnFactory {
         cnxnExpiryQueue.update(cnxn, cnxn.getSessionTimeout());
     }
 
-    private void addCnxn(NIOServerCnxn cnxn) {
+    private void addCnxn(NIOServerCnxn cnxn) throws IOException {
         InetAddress addr = cnxn.getSocketAddress();
+        if (addr == null) {
+            throw new IOException("Socket of " + cnxn + " has been closed");
+        }
         Set<NIOServerCnxn> set = ipMap.get(addr);
         if (set == null) {
             // in general we will see 1 connection from each