Skip to content

Commit

Permalink
[openvpn] Avoid failing if data is empty
Browse files Browse the repository at this point in the history
  • Loading branch information
nemesifier committed Dec 26, 2017
1 parent 84e97c5 commit 6b72dab
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 3 deletions.
17 changes: 14 additions & 3 deletions netdiff/parsers/openvpn.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import networkx
from openvpn_status import parse_status
from openvpn_status.parser import ParsingError

from .base import BaseParser

Expand All @@ -13,7 +14,10 @@ class OpenvpnParser(BaseParser):
_server_common_name = 'openvpn-server'

def to_python(self, data):
return parse_status(data)
try:
return parse_status(data)
except (AttributeError, ParsingError):
return None

This comment has been minimized.

Copy link
@nemesifier

nemesifier Jan 3, 2018

Author Member

@rohithasrk in retrospective this may not be a good idea, it's probably better to do something like:

if not data:
    return None
try:
    return parse_status(data)
except (AttributeError, ParsingError) as e:
    msg = 'something went wront: {0}'.format(e.message)
    raise ConversionException(msg)

This comment has been minimized.

Copy link
@nemesifier

nemesifier Jan 3, 2018

Author Member

otherwise badly formatted files won't raise any exception


def parse(self, data):
"""
Expand All @@ -25,8 +29,15 @@ def parse(self, data):
server = self._server_common_name
# add server (central node) to graph
graph.add_node(server)
# data may be empty
if data is None:
clients = []
links = []
else:
clients = data.client_list.values()
links = data.routing_table.values()
# add clients in graph as nodes
for client in data.client_list.values():
for client in clients:
client_properties = {
'label': client.common_name,
'real_address': str(client.real_address.host),
Expand All @@ -44,6 +55,6 @@ def parse(self, data):
client_properties['local_addresses'] = local_addresses
graph.add_node(str(client.real_address.host), **client_properties)
# add links in routing table to graph
for link in data.routing_table.values():
for link in links:
graph.add_edge(server, str(link.real_address.host), weight=1)
return graph
7 changes: 7 additions & 0 deletions tests/test_openvpn.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,10 @@ def test_json_dict_tap(self):
self.assertIn('nodeC', labels)
self.assertIn('nodeD', labels)
self.assertIn('nodeE', labels)

def test_empty_string(self):
OpenvpnParser(data='{}')

def test_empty_dict(self):
OpenvpnParser({})

1 comment on commit 6b72dab

@rohithasrk
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Please sign in to comment.