Skip to content

Commit a3cb530

Browse files
committed
fix: quote XPath string literals via _xpath_safe helper
Add a private _xpath_safe() helper in panos.base that wraps a value as a valid XPath 1.0 string literal: single quotes by default, double quotes when the value contains a single quote, and concat() when it contains both. Use it everywhere the SDK interpolates a value into an XPath predicate. Templates that previously embedded the surrounding quotes (e.g. "[@name='%s']") drop them; the helper now supplies the correct quoting. For inputs without quote characters the rendered XPath is byte-identical to the previous output, so existing tests asserting exact xpath strings remain green. Touched sites: - panos/base.py: ENTRY/MEMBER constants, _root_xpath_vsys, SUFFIX consumer, delete_similar joiner, vsys-dict import delete, _TEMPLATE_VSYS_XPATH, both _get_param_specific_info overloads, VersionedPanObject.XPATH, delete_import, and the three re.sub 'entry varname' sites - panos/firewall.py, panos/panorama.py, panos/userid.py, panos/network.py, panos/predefined.py: all variable interpolations into [@name=...], [@ip=...], and text()=... predicates Hardcoded literals such as entry[@name='localhost.localdomain' are unchanged.
1 parent 6c6c406 commit a3cb530

6 files changed

Lines changed: 82 additions & 40 deletions

File tree

panos/base.py

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,31 @@
3737

3838
import panos
3939
import panos.errors as err
40+
41+
# Defined before sub-module imports below: panos.userid imports _xpath_safe
42+
# from this module, so the symbol must exist by the time `from panos import
43+
# userid` triggers userid's module body.
44+
SELF = "/%s"
45+
ENTRY = "/entry[@name=%s]"
46+
MEMBER = "/member[text()=%s]"
47+
48+
49+
def _xpath_safe(val):
50+
"""Return val as an XPath 1.0 string literal, safe to inject into a predicate.
51+
52+
XPath 1.0 has no escape syntax for quotes inside string literals, so a value
53+
containing quotes must be wrapped in the opposite quote, or split into a
54+
concat() expression when both quote types are present.
55+
"""
56+
val = "" if val is None else str(val)
57+
if "'" not in val:
58+
return "'" + val + "'"
59+
if '"' not in val:
60+
return '"' + val + '"'
61+
parts = val.split("'")
62+
return "concat('" + "', \"'\", '".join(parts) + "')"
63+
64+
4065
from panos import (
4166
chunk_instances_for_delete_similar,
4267
isstring,
@@ -49,9 +74,6 @@
4974
logger = panos.getlogger(__name__)
5075

5176
Root = panos.enum("DEVICE", "VSYS", "MGTCONFIG", "PANORAMA", "PANORAMA_VSYS")
52-
SELF = "/%s"
53-
ENTRY = "/entry[@name='%s']"
54-
MEMBER = "/member[text()='%s']"
5577

5678

5779
# PanObject type
@@ -341,7 +363,7 @@ def xpath(self, root=None):
341363
# xpath was asked for.
342364
addon = p.XPATH
343365
if p.SUFFIX is not None:
344-
addon += p.SUFFIX % (p.uid,)
366+
addon += p.SUFFIX % (_xpath_safe(p.uid),)
345367
path.insert(0, addon)
346368
if p.__class__.__name__ == "Firewall" and p.parent is not None:
347369
if p.parent.__class__.__name__ == "DeviceGroup":
@@ -413,7 +435,7 @@ def _root_xpath_vsys(self, vsys, label="vsys"):
413435
xpath = "/config/shared"
414436
else:
415437
xpath = "/config/devices/entry[@name='localhost.localdomain']"
416-
xpath += "/{0}/entry[@name='{1}']".format(label, vsys or "vsys1")
438+
xpath += "/{0}/entry[@name={1}]".format(label, _xpath_safe(vsys or "vsys1"))
417439

418440
return xpath
419441

@@ -477,7 +499,7 @@ def element(self, with_children=True, comparable=False):
477499
regex,
478500
matchedvar.path
479501
+ "/"
480-
+ "entry[@name='%s']" % entry_value[0],
502+
+ "entry[@name=%s]" % _xpath_safe(entry_value[0]),
481503
section,
482504
)
483505
entryvar = matchedvar
@@ -861,7 +883,9 @@ def _get_param_specific_info(self, variable):
861883
entry_value = panos.string_or_list(getattr(self, matchedvar.variable))
862884
varpath = re.sub(
863885
regex,
864-
matchedvar.path + "/" + "entry[@name='%s']" % entry_value[0],
886+
matchedvar.path
887+
+ "/"
888+
+ "entry[@name=%s]" % _xpath_safe(entry_value[0]),
865889
varpath,
866890
)
867891
else:
@@ -1449,7 +1473,9 @@ def _parse_xml(cls, xml, variables=None):
14491473
replacement = replacement[0]
14501474
path = re.sub(
14511475
regex,
1452-
matchedvar.path + "/" + "entry[@name='%s']" % replacement,
1476+
matchedvar.path
1477+
+ "/"
1478+
+ "entry[@name=%s]" % _xpath_safe(replacement),
14531479
path,
14541480
)
14551481
else:
@@ -1984,10 +2010,10 @@ def delete_similar(self):
19842010
prefix = ""
19852011
xpath = self.xpath_nosuffix()
19862012
if self.SUFFIX == ENTRY:
1987-
joiner = "@name='{0}'"
2013+
joiner = "@name={0}"
19882014
prefix = "entry"
19892015
elif self.SUFFIX == MEMBER:
1990-
joiner = "text()='{0}'"
2016+
joiner = "text()={0}"
19912017
prefix = "member"
19922018

19932019
# After some testing, PAN-OS seems to be able to handle a DELETE API call
@@ -2000,7 +2026,7 @@ def delete_similar(self):
20002026
"{0}/{1}[{2}]".format(
20012027
xpath,
20022028
prefix,
2003-
" or ".join(joiner.format(x.uid) for x in chunk),
2029+
" or ".join(joiner.format(_xpath_safe(x.uid)) for x in chunk),
20042030
),
20052031
retry_on_peer=self.HA_SYNC,
20062032
)
@@ -2037,7 +2063,9 @@ def _perform_vsys_dict_import_delete(self, dev, vsys_dict):
20372063
"""Iterates over a vsys_dict, deleting the import for all instances."""
20382064
for vsys_spec in vsys_dict.values():
20392065
for objs in vsys_spec.values():
2040-
members = " or ".join("text()='{0}'".format(x.uid) for x in objs)
2066+
members = " or ".join(
2067+
"text()={0}".format(_xpath_safe(x.uid)) for x in objs
2068+
)
20412069
xpath = "{0}/member[{1}]".format(objs[0].xpath_import_base(), members)
20422070
# API complains if you try to do this in one delete statement,
20432071
# so do one delete per vsys per path, just like when we set the
@@ -2432,7 +2460,7 @@ class VersionedPanObject(PanObject):
24322460

24332461
_DEFAULT_NAME = None
24342462
_TEMPLATE_DEVICE_XPATH = "/config/devices/entry[@name='localhost.localdomain']"
2435-
_TEMPLATE_VSYS_XPATH = _TEMPLATE_DEVICE_XPATH + "/vsys/entry[@name='{vsys}']"
2463+
_TEMPLATE_VSYS_XPATH = _TEMPLATE_DEVICE_XPATH + "/vsys/entry[@name={vsys}]"
24362464
_TEMPLATE_MGTCONFIG_XPATH = "/config/mgt-config"
24372465

24382466
def __init__(self, *args, **kwargs):
@@ -2639,7 +2667,7 @@ def element(self, with_children=True, comparable=False):
26392667
if ap.startswith("entry "):
26402668
junk, var_to_use = ap.split()
26412669
sol_value = panos.string_or_list(settings[var_to_use])[0]
2642-
finder = "entry[@name='{0}']".format(sol_value)
2670+
finder = "entry[@name={0}]".format(_xpath_safe(sol_value))
26432671
tag = "entry"
26442672
attribs["name"] = sol_value
26452673
elif ap == "entry[@name='localhost.localdomain']":
@@ -2726,8 +2754,8 @@ def _get_param_specific_info(self, param):
27262754
p = None
27272755
if token.startswith("entry "):
27282756
junk, var_to_use = token.split()
2729-
p = "entry[name='{0}']".format(
2730-
*(x for x in self._value_as_list(settings[var_to_use]))
2757+
p = "entry[name={0}]".format(
2758+
*(_xpath_safe(x) for x in self._value_as_list(settings[var_to_use]))
27312759
)
27322760
else:
27332761
p = None
@@ -2836,7 +2864,7 @@ def XPATH(self):
28362864
"""Returns the version specific xpath of this object."""
28372865
panos_version = self.retrieve_panos_version()
28382866
val = self._xpaths._get_versioned_value(panos_version, self.parent)
2839-
return val.format(vsys=self.vsys or "vsys1")
2867+
return val.format(vsys=_xpath_safe(self.vsys or "vsys1"))
28402868

28412869

28422870
class VersionedParamPath(VersioningSupport):
@@ -3199,7 +3227,7 @@ def parse_xml(self, xml, settings, possibilities):
31993227
return
32003228
settings[entry_var] = ans.attrib["name"]
32013229
sol_val = panos.string_or_list(settings[entry_var])[0]
3202-
path_str = "entry[@name='{0}']".format(sol_val)
3230+
path_str = "entry[@name={0}]".format(_xpath_safe(sol_val))
32033231
else:
32043232
# Standard path part
32053233
try:
@@ -3421,8 +3449,8 @@ def delete_import(self, vsys=None):
34213449
p = p.parent
34223450

34233451
if vsys != "shared" and vsys is not None and self.XPATH_IMPORT is not None:
3424-
xpath = "{0}/member[text()='{1}']".format(
3425-
self.xpath_import_base(vsys), self.uid
3452+
xpath = "{0}/member[text()={1}]".format(
3453+
self.xpath_import_base(vsys), _xpath_safe(self.uid)
34263454
)
34273455
device = self.nearest_pandevice()
34283456
device.active().xapi.delete(xpath, retry_on_peer=True)

panos/firewall.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
from panos import device, getlogger, yesno
2828
from panos.base import ENTRY, PanDevice, Root
2929
from panos.base import VarPath as Var
30+
from panos.base import _xpath_safe
3031

3132
logger = getlogger(__name__)
3233

@@ -333,7 +334,7 @@ def delete(self):
333334
devices_xpath = self.devicegroup().xpath() + self.XPATH
334335
devices_xml = panorama.xapi.get(devices_xpath)
335336
dg_vsys = devices_xml.findall(
336-
"result/devices/entry[@name='%s']/vsys/entry" % self.serial
337+
"result/devices/entry[@name=%s]/vsys/entry" % _xpath_safe(self.serial)
337338
)
338339
if dg_vsys:
339340
if len(dg_vsys) == 1:
@@ -344,7 +345,7 @@ def delete(self):
344345
# It's not the only vsys, just delete the vsys
345346
panorama.set_config_changed()
346347
panorama.xapi.delete(
347-
self.xpath() + "/vsys/entry[@name='%s']" % self.vsys
348+
self.xpath() + "/vsys/entry[@name=%s]" % _xpath_safe(self.vsys)
348349
)
349350
else:
350351
# This is a firewall under a panorama
@@ -392,7 +393,7 @@ def refreshall_from_xml(self, xml, refresh_children=False, variables=None):
392393
)
393394
# Add system settings to firewall instances
394395
for fw in firewall_instances:
395-
entry = xml.find("entry[@name='%s']" % fw.serial)
396+
entry = xml.find("entry[@name=%s]" % _xpath_safe(fw.serial))
396397
system = fw.find_or_create(None, device.SystemSettings)
397398
system.hostname = entry.findtext("hostname")
398399
system.ip_address = entry.findtext("ip-address")

panos/network.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,12 @@
2626
from panos import device, getlogger, string_or_list
2727
from panos.base import ENTRY, MEMBER, PanObject, Root
2828
from panos.base import VarPath as Var
29-
from panos.base import VersionedPanObject, VersionedParamPath, VsysOperations
29+
from panos.base import (
30+
VersionedPanObject,
31+
VersionedParamPath,
32+
VsysOperations,
33+
_xpath_safe,
34+
)
3035

3136
logger = getlogger(__name__)
3237

@@ -741,7 +746,7 @@ def XPATH(self):
741746
if self._BASE_INTERFACE_NAME in path:
742747
base = self.uid.split(".")[0]
743748
path = path.replace(
744-
self._BASE_INTERFACE_NAME, "entry[@name='{0}']".format(base)
749+
self._BASE_INTERFACE_NAME, "entry[@name={0}]".format(_xpath_safe(base))
745750
)
746751

747752
return path

panos/panorama.py

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
from panos import base, firewall, getlogger, policies, yesno
2929
from panos.base import ENTRY, MEMBER, OpState, PanObject, Root
3030
from panos.base import VarPath as Var
31-
from panos.base import VersionedPanObject, VersionedParamPath
31+
from panos.base import VersionedPanObject, VersionedParamPath, _xpath_safe
3232

3333
logger = getlogger(__name__)
3434

@@ -643,7 +643,7 @@ def refresh_devices(
643643
serial = str(device)
644644
if serial is None:
645645
continue
646-
entry = devices_xml.find("entry[@name='%s']" % serial)
646+
entry = devices_xml.find("entry[@name=%s]" % _xpath_safe(serial))
647647
if entry is None:
648648
if only_connected:
649649
raise err.PanNotConnectedOnPanorama(
@@ -661,7 +661,10 @@ def refresh_devices(
661661
except AttributeError:
662662
continue
663663
# Create entry if needed
664-
if filtered_devices_xml.find("entry[@name='%s']" % serial) is None:
664+
if (
665+
filtered_devices_xml.find("entry[@name=%s]" % _xpath_safe(serial))
666+
is None
667+
):
665668
entry_copy = deepcopy(entry)
666669
# If looking for specific vsys, erase all vsys in filtered entry
667670
if vsys != "shared" and vsys is not None:
@@ -670,15 +673,15 @@ def refresh_devices(
670673
filtered_devices_xml.append(entry_copy)
671674
# Get specific vsys
672675
if vsys != "shared" and vsys is not None:
673-
vsys_entry = entry.find("vsys/entry[@name='%s']" % vsys)
676+
vsys_entry = entry.find("vsys/entry[@name=%s]" % _xpath_safe(vsys))
674677
if vsys_entry is None:
675678
raise err.PanNotAttachedOnPanorama(
676679
"Can't find device with serial %s and"
677680
" vsys %s attached to Panorama at %s"
678681
% (serial, vsys, self.id)
679682
)
680683
vsys_section = filtered_devices_xml.find(
681-
"entry[@name='%s']/vsys" % serial
684+
"entry[@name=%s]/vsys" % _xpath_safe(serial)
682685
)
683686
vsys_section.append(vsys_entry)
684687
devices_xml = filtered_devices_xml
@@ -733,7 +736,8 @@ def refresh_devices(
733736
continue
734737
for fw_entry in dg_entry.find("devices"):
735738
fw_entry_op = devicegroup_opxml.find(
736-
"entry/devices/entry[@name='%s']" % fw_entry.get("name")
739+
"entry/devices/entry[@name=%s]"
740+
% _xpath_safe(fw_entry.get("name"))
737741
)
738742
if fw_entry_op is not None:
739743
panos.xml_combine(fw_entry, fw_entry_op)
@@ -748,7 +752,7 @@ def refresh_devices(
748752
dg_serials = [
749753
entry.get("name")
750754
for entry in devicegroup_configxml.findall(
751-
"entry[@name='%s']/devices/entry" % dg.name
755+
"entry[@name=%s]/devices/entry" % _xpath_safe(dg.name)
752756
)
753757
]
754758
# Find firewall with each serial
@@ -759,13 +763,14 @@ def refresh_devices(
759763
all_dg_vsys = [
760764
entry.get("name")
761765
for entry in devicegroup_configxml.findall(
762-
"entry[@name='%s']/devices/entry[@name='%s']/vsys/entry"
763-
% (dg.name, dg_serial)
766+
"entry[@name=%s]/devices/entry[@name=%s]/vsys/entry"
767+
% (_xpath_safe(dg.name), _xpath_safe(dg_serial))
764768
)
765769
]
766770
# Collect the firewall serial entry to get current status information
767771
fw_entry = devicegroup_configxml.find(
768-
"entry[@name='%s']/devices/entry[@name='%s']" % (dg.name, dg_serial)
772+
"entry[@name=%s]/devices/entry[@name=%s]"
773+
% (_xpath_safe(dg.name), _xpath_safe(dg_serial))
769774
)
770775
if not all_dg_vsys:
771776
# This is a single-context firewall, assume vsys1
@@ -807,7 +812,8 @@ def refresh_devices(
807812
shared_policy_status = fw_entry.findtext("shared-policy-status")
808813
if shared_policy_status is None:
809814
shared_policy_status = fw_entry.findtext(
810-
"vsys/entry[@name='%s']/shared-policy-status" % dg_vsys
815+
"vsys/entry[@name=%s]/shared-policy-status"
816+
% _xpath_safe(dg_vsys)
811817
)
812818
fw.state.set_shared_policy_synced(shared_policy_status)
813819

panos/predefined.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import panos.errors as err
2323
from panos import getlogger, objects
24+
from panos.base import _xpath_safe
2425
from panos.updater import PanOSVersion
2526

2627
logger = getlogger(__name__)
@@ -43,7 +44,7 @@ class Predefined(object):
4344

4445
# xpath
4546
XPATH = "/config/predefined"
46-
SINGLE_ENTRY_XPATH = "/entry[@name='{0}']"
47+
SINGLE_ENTRY_XPATH = "/entry[@name={0}]"
4748
ALL_ENTRIES_XPATH = "/entry"
4849
CHILDTYPES = (
4950
"objects.ApplicationContainer",
@@ -76,7 +77,7 @@ def _refresh(self, decisions, name=None):
7677
x.parent = self
7778
xpath = x.xpath_nosuffix()
7879
if name is not None:
79-
xpath += self.SINGLE_ENTRY_XPATH.format(name)
80+
xpath += self.SINGLE_ENTRY_XPATH.format(_xpath_safe(name))
8081
else:
8182
xpath += self.ALL_ENTRIES_XPATH
8283

panos/userid.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import panos.errors as err
2727
from panos import getlogger, string_or_list, string_or_list_or_none
28+
from panos.base import _xpath_safe
2829
from panos.updater import PanOSVersion
2930

3031
logger = getlogger(__name__)
@@ -245,7 +246,7 @@ def register(self, ip, tags, timeout=None):
245246
return
246247
tags = [self.prefix + t for t in tags]
247248
for c_ip in ip:
248-
tagelement = register.find("./entry[@ip='%s']/tag" % c_ip)
249+
tagelement = register.find("./entry[@ip=%s]/tag" % _xpath_safe(c_ip))
249250
if tagelement is None:
250251
entry = ET.SubElement(register, "entry", {"ip": c_ip})
251252
tagelement = ET.SubElement(entry, "tag")
@@ -276,7 +277,7 @@ def unregister(self, ip, tags):
276277
return
277278
tags = [self.prefix + t for t in tags]
278279
for c_ip in ip:
279-
tagelement = unregister.find("./entry[@ip='%s']/tag" % c_ip)
280+
tagelement = unregister.find("./entry[@ip=%s]/tag" % _xpath_safe(c_ip))
280281
if tagelement is None:
281282
entry = ET.SubElement(unregister, "entry", {"ip": c_ip})
282283
tagelement = ET.SubElement(entry, "tag")

0 commit comments

Comments
 (0)