----- Original Message -----
From: "Martin Kolman" mkolman@redhat.com To: anaconda-patches@lists.fedorahosted.org Sent: Monday, September 8, 2014 1:49:42 PM Subject: [PATCH 1/2] Validate network interface name when parsing the kickstart (#1081982)
Resolves: rhbz#1081982 Signed-off-by: Martin Kolman mkolman@redhat.com
pykickstart/commands/network.py | 55 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+)
diff --git a/pykickstart/commands/network.py b/pykickstart/commands/network.py index 7b1341c..44ddb8d 100644 --- a/pykickstart/commands/network.py +++ b/pykickstart/commands/network.py @@ -517,9 +517,64 @@ class RHEL6_Network(F9_Network): op.add_option("--bondopts", dest="bondopts") return op
+def validate_network_interface_name(name):
- """Check if the given network interface name is valid, return an error
message
- if an error is found or None if no errors are found
Could you give a link here to the specification of the network interface name format or else specify that format here?
- :param str name: name to validate
- :returns: error message or None if no error is found
- :rtype: str or None
This is very pedantic, but None is a value and _NoneType_ is its type.
- """
- # (for reference see the NetworkManager source code:
- # NetworkManager/src/settings/plugins/ifcfg-rh/reader.c
- # and the make_vlan_setting function)
- min_vlan_id = 0
- max_vlan_id = 4095
The above are constants, so should be capitalized.
- vlan_id = None
For the condition below, partition is probably better:
(prefix, sep, vlan_id) = name.partition(".") if sep: if prefix == "vlan": return ... try: vlan_id = int(vlan_id) except ...: ...
That way, you have to do less indexing and you don't have to worry about len and max splits for split() function.
- # if it contains '.', vlan id should follow (eg 'ens7.171', 'mydev.171')
- split_name = name.split(".", 1)
- if len(split_name) > 1:
# 'vlan' can't be followed by a '.'
if split_name[0] == "vlan":
return _("When using the <prefix>.<vlan id> interface name
notation, <prefix> can't be equal to 'vlan'.")
try:
vlan_id = int(split_name[1])
except ValueError:
return _("If network --interfacename contains a '.', valid vlan
id should follow.")
Is vlang.123 an acceptable id? If so, then the block below should be an elif. If not, then the block below should probably be the first block, so that vlang.123 gets thrown out at first check. Right now it gets accepted by first block but rejected by second block.
Should use partition below as well as strip() is not really what you want.
Something like:
(empty, sep, vlan_id) = name.partition("vlan") if empty == "": try: vlan_id = int(vlan_id) except .... ....
That way you avoid accepting "vlan123lvan" by bad coincidence.
- # if it starts with 'vlan', vlan id should follow ('vlan171')
- if name.startswith("vlan"):
try:
vlan_id = int(name.strip("vlan"))
except ValueError:
return _("If network --interfacename starts with 'vlan', valid
vlan id should follow.")
- # check if the vlan id is in range
- if vlan_id is not None:
if not(min_vlan_id <= vlan_id <= max_vlan_id):
return _("The vlan id out of the %d-%d vlan id range." %
(min_vlan_id, max_vlan_id))
- # network interface name seems to be valid (no error found)
- return None
class RHEL7_Network(F20_Network): def _getParser(self): op = F20_Network._getParser(self) op.add_option("--interfacename", dest="interfacename", action="store", default="") return op
- def parse(self, args):
# call the overridden command to do it's job first
retval = F20_Network.parse(self, args)
# validate the network interface name
error_message =
validate_network_interface_name(retval.interfacename)
# something is wrong with the interface name
if error_message is not None:
raise
KickstartValueError(formatErrorMsg(self.lineno,msg=error_message))
return retval
-- 1.9.3
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
Thanks for writing the tests to go along with, they make it a lot easier for me to think about the code.
- mulhern