Skip to content

Commit

Permalink
Merge pull request #1339 from anarkiwi/master
Browse files Browse the repository at this point in the history
Fix crash parsing non UTF-8 config file.
  • Loading branch information
anarkiwi committed Dec 3, 2017
2 parents d606e9b + 0ea3298 commit 7340074
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 38 deletions.
4 changes: 2 additions & 2 deletions faucet/config_parser_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ def read_config(config_file, logname):
try:
with open(config_file, 'r') as stream:
conf = yaml.safe_load(stream)
except yaml.YAMLError as ex:
logger.error('Error in file %s (%s)', config_file, str(ex))
except (yaml.YAMLError, UnicodeDecodeError) as err:
logger.error('Error in file %s (%s)', config_file, str(err))
return None
return conf

Expand Down
79 changes: 43 additions & 36 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@ def tearDown(self):
def create_config_file(self, config):
"""Returns file path to file containing the config parameter."""
conf_file_name = os.path.join(self.tmpdir, 'faucet.yaml')
with open(conf_file_name, 'w') as conf_file:
conf_file.write(config)
with open(conf_file_name, 'wb') as conf_file:
if isinstance(config, bytes):
conf_file.write(config)
else:
conf_file.write(config.encode('utf-8'))
return conf_file_name

def run_function_with_config(self, config, function):
Expand Down Expand Up @@ -91,7 +94,7 @@ def test_config_only_empty_array(self):

def test_unconfigured_acl(self):
"""Test that config is invalid when there are unconfigured acls"""
acl_config = """
config = """
vlans:
office:
vid: 100
Expand All @@ -103,11 +106,11 @@ def test_unconfigured_acl(self):
acl_in: access-port-protect
tagged_vlans: [office]
"""
self.check_config_failure(acl_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_unconfigured_vlan_acl(self):
"""Test that config is invalid when only there are unconfigured acls"""
acl_config = """
config = """
vlans:
office:
vid: 100
Expand All @@ -119,7 +122,7 @@ def test_unconfigured_vlan_acl(self):
1:
tagged_vlans: [office]
"""
self.check_config_failure(acl_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_config_routes_are_empty(self):
"""Test that config is invalid when vlan routes are empty"""
Expand Down Expand Up @@ -217,7 +220,7 @@ def test_config_dps_is_empty(self):

def test_including_invalid_files(self):
"""Test that config is rejected when including invalid files"""
include_config = """
config = """
include: [-, False, 1967-06-07, 5.5, [5], {'5': 5}, testing]
vlans:
office:
Expand All @@ -229,7 +232,7 @@ def test_including_invalid_files(self):
5:
tagged_vlans: [office]
"""
self.check_config_failure(include_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_config_vlans_on_stack(self):
"""Test that config is rejected vlans on a stack interface."""
Expand Down Expand Up @@ -298,7 +301,7 @@ def test_config_stack(self):

def test_port_number(self):
"""Test port number is valid."""
port_config = """
config = """
vlans:
office:
vid: 100
Expand All @@ -309,11 +312,11 @@ def test_port_number(self):
testing:
native_vlan: office
"""
self.check_config_failure(port_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_one_port_dp(self):
"""Test port number is valid."""
port_config = """
config = """
vlans:
office:
vid: 100
Expand All @@ -325,11 +328,11 @@ def test_one_port_dp(self):
number: 1
native_vlan: office
"""
self.check_config_success(port_config, cp.dp_parser)
self.check_config_success(config, cp.dp_parser)

def test_dp_id_too_big(self):
"""Test DP ID is valid."""
toobig_dp_id_config = """
config = """
vlans:
office:
vid: 100
Expand All @@ -340,11 +343,11 @@ def test_dp_id_too_big(self):
1:
native_vlan: office
"""
self.check_config_failure(toobig_dp_id_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_invalid_vid(self):
"""Test VID is valid."""
vlan_config = """
config = """
vlans:
office:
vid: 10000
Expand All @@ -355,11 +358,11 @@ def test_invalid_vid(self):
1:
native_vlan: office
"""
self.check_config_failure(vlan_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_routers_empty(self):
"""Test with empty router config."""
router_config = """
config = """
routers:
router-1:
vlans:
Expand All @@ -372,11 +375,11 @@ def test_routers_empty(self):
1:
native_vlan: office
"""
self.check_config_failure(router_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_valid_mac(self):
"""Test with valid MAC."""
mac_config = """
config = """
vlans:
office:
vid: 100
Expand All @@ -388,11 +391,11 @@ def test_valid_mac(self):
1:
native_vlan: office
"""
self.check_config_success(mac_config, cp.dp_parser)
self.check_config_success(config, cp.dp_parser)

def test_invalid_mac(self):
"""Test with invalid MAC."""
mac_config = """
config = """
vlans:
office:
vid: 100
Expand All @@ -404,11 +407,11 @@ def test_invalid_mac(self):
1:
native_vlan: office
"""
self.check_config_failure(mac_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_empty_mac(self):
"""Test with empty MAC."""
mac_config = """
config = """
vlans:
office:
vid: 100
Expand All @@ -420,11 +423,11 @@ def test_empty_mac(self):
1:
native_vlan: office
"""
self.check_config_failure(mac_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_empty_vid(self):
"""Test empty VID."""
vlan_config = """
config = """
vlans:
office:
vid:
Expand All @@ -435,23 +438,23 @@ def test_empty_vid(self):
1:
native_vlan: office
"""
self.check_config_failure(vlan_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_empty_interfaces(self):
"""Test empty interfaces."""
interfaces_config = """
config = """
vlans:
office:
vid:
dps:
sw1:
dp_id: 0x1
"""
self.check_config_failure(interfaces_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_invalid_interfaces(self):
"""Test invalid interfaces."""
interfaces_config = """
config = """
vlans:
office:
vid: 100
Expand All @@ -460,10 +463,10 @@ def test_invalid_interfaces(self):
dp_id: 0x1
interfaces: {'5': 5}
"""
self.check_config_failure(interfaces_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_unresolved_mirror_ports(self):
unresolved_mirror_port_config = """
config = """
vlans:
office:
vid: 100
Expand All @@ -481,10 +484,10 @@ def test_unresolved_mirror_ports(self):
mirror: UNRESOLVED
allow: 1
"""
self.check_config_failure(unresolved_mirror_port_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_unresolved_output_ports(self):
unresolved_output_port_config = """
config = """
vlans:
office:
vid: 100
Expand All @@ -503,10 +506,10 @@ def test_unresolved_output_ports(self):
port: UNRESOLVED
allow: 1
"""
self.check_config_failure(unresolved_output_port_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_unknown_output_ports(self):
unknown_output_port_config = """
config = """
vlans:
office:
vid: 100
Expand All @@ -525,7 +528,7 @@ def test_unknown_output_ports(self):
port: 2
allow: 1
"""
self.check_config_failure(unknown_output_port_config, cp.dp_parser)
self.check_config_failure(config, cp.dp_parser)

def test_port_range_valid_config(self):
"""Test if port range config applied correctly"""
Expand Down Expand Up @@ -698,6 +701,10 @@ def test_acl_invalid_rule_name(self):
"""
self.check_config_failure(config, cp.dp_parser)

def test_invalid_char(self):
config = b'\x63\xe1'
self.check_config_failure(config, cp.dp_parser)


if __name__ == "__main__":
unittest.main()

0 comments on commit 7340074

Please sign in to comment.