[ISSUE] update_segment sends empty port-range lists for the port keys the caller did not pass, clearing them
Environment
|
|
| SDK version |
zscaler-sdk-python 1.9.44 (editable install of master @ e7f5f7ef) |
| Python |
3.11.15 |
| OS |
macOS (arm64) |
| Affected product / resource |
ZPA / application_segment |
Found by reading the SDK source and confirmed with the offline reproduction below (a stub
request executor, so no credentials and no network are involved). All identifiers used are
synthetic.
Description
ApplicationSegmentAPI.update_segment builds the PUT body directly from kwargs with no
merge against the segment's current state:
# zscaler/zpa/application_segment.py:396
body = kwargs
and then, for each of the four port keys, explicitly writes an empty list when the caller
did not supply one:
# zscaler/zpa/application_segment.py:435-453
if "tcp_port_ranges" in body:
body["tcpPortRanges"] = body.pop("tcp_port_ranges")
else:
body["tcpPortRanges"] = []
if "tcp_port_range" in body:
body["tcpPortRange"] = [{"from": pr["from"], "to": pr["to"]} for pr in body.pop("tcp_port_range")]
else:
body["tcpPortRange"] = []
if "udp_port_ranges" in body:
body["udpPortRanges"] = body.pop("udp_port_ranges")
else:
body["udpPortRanges"] = [] # Explicitly clear if not provided
if "udp_port_range" in body:
body["udpPortRange"] = [{"from": pr["from"], "to": pr["to"]} for pr in body.pop("udp_port_range")]
else:
body["udpPortRange"] = [] # Explicitly clear if not provided
So a caller who only wants to change, say, the description ends up sending a body that also
zeroes all four port keys. The affected keys are exactly these four — tcpPortRanges, tcpPortRange, udpPortRanges
and udpPortRange. No other key in the method is set to [] on the omitted-key path.
The remaining fields (name, domainNames, segmentGroupId, serverGroups, …) are not
explicitly cleared, but because body = kwargs they are simply absent from the PUT, so
whether they survive depends entirely on the server's PUT semantics. The four port keys are
different in that they are cleared unconditionally, regardless of those semantics.
Two things in the codebase suggest the explicit-[] behaviour is an oversight rather than a
deliberate contract:
update_access_rule in the same service filters unset values out of its PUT payload —
payload = {k: v for k, v in payload.items() if v is not None}, with the comment
"Filter out None values if you prefer not to send them" (zscaler/zpa/policies.py:710).
zscaler/ztw/location_template.py does the same.
APIClient.format_request_body (zscaler/api_client.py:81-94) skips None values by
design (if val is None: continue). It is present in the base class but does not appear to
be referenced anywhere outside its own recursive call, so the ZPA update path never picks up
that convention.
add_segment (zscaler/zpa/application_segment.py:283-291) handles the same four keys with
if/elif and no clearing branch, so an omitted key is simply not sent. The asymmetry is
only in update_segment.
Possibly related: #571 raises the same read-modify-write concern for policy rules, including
"appConnectorGroups": [...] or [] surviving the None filter for the same reason.
Reproduction
Fully offline — no credentials, no network.
import json
from zscaler.zpa.application_segment import ApplicationSegmentAPI
SEGMENT_ID = "72058304855090128"
CONFIG = {"client": {"customerId": "72058304855015000"}}
class StubResponse:
def __init__(self, body):
self._body = body
def get_body(self):
return self._body
def get_results(self):
return []
def get_status(self):
return 200
class StubExecutor:
"""Stands in for RequestExecutor: records requests, replies with an empty body."""
def __init__(self):
self.requests = []
def create_request(self, method, endpoint, body=None, headers=None, params=None,
use_raw_data_for_body=False):
req = {"method": method, "endpoint": endpoint.strip(),
"body": {} if body is None else body, "params": params or {}}
self.requests.append(req)
return req, None
def execute(self, request, response_type=None, return_raw_response=False):
return StubResponse({}), None
ex = StubExecutor()
seg, resp, err = ApplicationSegmentAPI(ex, CONFIG).update_segment(
SEGMENT_ID, description="new description")
put = [r for r in ex.requests if r["method"] == "PUT"][0]
print("PUT body actually sent:")
print(json.dumps(put["body"], indent=2, sort_keys=True))
print("err =", err)
The equivalent live call, on a segment that has TCP ports configured:
client.zpa.application_segment.update_segment(
segment_id="72058304855090128",
description="new description",
)
Expected behaviour
A caller who does not mention the port keys should not have them modified. Either the keys
should be omitted from the request body, or the method should merge them from the segment's
current state, or the destructive behaviour should be documented on the method so it is a
choice rather than a surprise.
Actual behaviour
PUT body actually sent:
{
"description": "new description",
"tcpPortRange": [],
"tcpPortRanges": [],
"udpPortRange": [],
"udpPortRanges": []
}
err = None
Root cause
| Location |
Code |
zscaler/zpa/application_segment.py:396 |
body = kwargs — no merge with current state |
zscaler/zpa/application_segment.py:438 |
body["tcpPortRanges"] = [] when omitted |
zscaler/zpa/application_segment.py:443 |
body["tcpPortRange"] = [] when omitted |
zscaler/zpa/application_segment.py:448 |
body["udpPortRanges"] = [] when omitted |
zscaler/zpa/application_segment.py:453 |
body["udpPortRange"] = [] when omitted |
Suggested direction
Without knowing whether update_segment is meant to be a full-replacement PUT or a partial
update, I'd rather ask than propose a patch. The options I can see:
- Drop the four
else branches so unmentioned keys are simply not sent. Smallest diff, but it
removes any way to clear port ranges, unless passing [] explicitly is treated as "clear"
(which would work, since the presence check would then be meaningful).
- Reuse the
{k: v for k, v in ... if v is not None} filter already used by
update_access_rule, or wire up APIClient.format_request_body, so the whole body follows
one convention rather than two.
- Keep the current behaviour and document
update_segment as a full replacement, so callers
know they must read-modify-write.
I'm happy to open a PR for whichever of these you'd prefer — the first is a four-line change,
but I don't want to guess at the intended contract.
[ISSUE] update_segment sends empty port-range lists for the port keys the caller did not pass, clearing them
Environment
zscaler-sdk-python1.9.44 (editable install ofmaster@e7f5f7ef)application_segmentFound by reading the SDK source and confirmed with the offline reproduction below (a stub
request executor, so no credentials and no network are involved). All identifiers used are
synthetic.
Description
ApplicationSegmentAPI.update_segmentbuilds thePUTbody directly fromkwargswith nomerge against the segment's current state:
and then, for each of the four port keys, explicitly writes an empty list when the caller
did not supply one:
So a caller who only wants to change, say, the description ends up sending a body that also
zeroes all four port keys. The affected keys are exactly these four —
tcpPortRanges,tcpPortRange,udpPortRangesand
udpPortRange. No other key in the method is set to[]on the omitted-key path.The remaining fields (
name,domainNames,segmentGroupId,serverGroups, …) are notexplicitly cleared, but because
body = kwargsthey are simply absent from thePUT, sowhether they survive depends entirely on the server's
PUTsemantics. The four port keys aredifferent in that they are cleared unconditionally, regardless of those semantics.
Two things in the codebase suggest the explicit-
[]behaviour is an oversight rather than adeliberate contract:
update_access_rulein the same service filters unset values out of itsPUTpayload —payload = {k: v for k, v in payload.items() if v is not None}, with the comment"Filter out None values if you prefer not to send them" (
zscaler/zpa/policies.py:710).zscaler/ztw/location_template.pydoes the same.APIClient.format_request_body(zscaler/api_client.py:81-94) skipsNonevalues bydesign (
if val is None: continue). It is present in the base class but does not appear tobe referenced anywhere outside its own recursive call, so the ZPA update path never picks up
that convention.
add_segment(zscaler/zpa/application_segment.py:283-291) handles the same four keys withif/elifand no clearing branch, so an omitted key is simply not sent. The asymmetry isonly in
update_segment.Possibly related: #571 raises the same read-modify-write concern for policy rules, including
"appConnectorGroups": [...] or []surviving theNonefilter for the same reason.Reproduction
Fully offline — no credentials, no network.
The equivalent live call, on a segment that has TCP ports configured:
Expected behaviour
A caller who does not mention the port keys should not have them modified. Either the keys
should be omitted from the request body, or the method should merge them from the segment's
current state, or the destructive behaviour should be documented on the method so it is a
choice rather than a surprise.
Actual behaviour
Root cause
zscaler/zpa/application_segment.py:396body = kwargs— no merge with current statezscaler/zpa/application_segment.py:438body["tcpPortRanges"] = []when omittedzscaler/zpa/application_segment.py:443body["tcpPortRange"] = []when omittedzscaler/zpa/application_segment.py:448body["udpPortRanges"] = []when omittedzscaler/zpa/application_segment.py:453body["udpPortRange"] = []when omittedSuggested direction
Without knowing whether
update_segmentis meant to be a full-replacementPUTor a partialupdate, I'd rather ask than propose a patch. The options I can see:
elsebranches so unmentioned keys are simply not sent. Smallest diff, but itremoves any way to clear port ranges, unless passing
[]explicitly is treated as "clear"(which would work, since the presence check would then be meaningful).
{k: v for k, v in ... if v is not None}filter already used byupdate_access_rule, or wire upAPIClient.format_request_body, so the whole body followsone convention rather than two.
update_segmentas a full replacement, so callersknow they must read-modify-write.
I'm happy to open a PR for whichever of these you'd prefer — the first is a four-line change,
but I don't want to guess at the intended contract.