[ZOOKEEPER-3009] Potential NPE in NIOServerCnxnFactory

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 3.6.0, 3.4.12
• Fix Version/s: 3.6.0, 3.5.5, 3.4.14
• Labels: pull-request-available
• Assignee: lujie
• Reporter: lujie
• Created: 26/Mar/18 07:46
• Updated: 04/Oct/19 14:55
• Resolved: 15/Jun/18 06:03

Description

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.

Bug description:

 class NIOServerCnxn has three method :getSocketAddress,getRemoteSocketAddress can return null just like :

// code placeholder
if (sock.isOpen() == false) {
  return null;
}

some of their caller give null checker, some(total 3 list in below) are not. 

// ServerCnxn#getConnectionInfo
Map<String, Object> info = new LinkedHashMap<String, Object>();
info.put("remote_socket_address", getRemoteSocketAddress());// Map.put will throw NPE if parameter is null

//IPAuthenticationProvider#handleAuthentication
tring id = cnxn.getRemoteSocketAddress().getAddress().getHostAddress();
cnxn.addAuthInfo(new Id(getScheme(), id));// finally call Set.add(it will throw NPE if parameter is null )

//NIOServerCnxnFactory#addCnxn
InetAddress addr = cnxn.getSocketAddress();
Set<NIOServerCnxn> set = ipMap.get(addr);// Map.get will throw NPE if parameter is null

I think we should add null check in above three caller .

 

Issue Links

• links to: GitHub Pull Request #525
• links to: GitHub Pull Request #544