Skip to content

Commit baf6aa6

Browse files
prakash-kalwaniyamn-ram
authored andcommitted
[fix] Prevent _get_common_name() from mutating device name in memory
The method was directly truncating device.name on the live object, corrupting in-memory state and causing auto-created certificates to store a truncated display name instead of the original.
1 parent f3c99c4 commit baf6aa6

File tree

2 files changed

+42
-3
lines changed

2 files changed

+42
-3
lines changed

openwisp_controller/config/base/vpn.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -892,12 +892,16 @@ def _get_common_name(self):
892892
"""
893893
d = self.config.device
894894
end = 63 - len(d.mac_address)
895-
d.name = d.name[:end]
895+
# Use a local variable to avoid mutating the device object in memory.
896+
# Mutating d.name would corrupt in-memory state for the rest of
897+
# the request and cause the certificate to store a truncated name.
898+
truncated_name = d.name[:end]
896899
unique_slug = shortuuid.ShortUUID().random(length=8)
897900
cn_format = app_settings.COMMON_NAME_FORMAT
898-
if cn_format == "{mac_address}-{name}" and d.name == d.mac_address:
901+
if cn_format == "{mac_address}-{name}" and truncated_name == d.mac_address:
899902
cn_format = "{mac_address}"
900-
common_name = cn_format.format(**d.__dict__)[:55]
903+
format_kwargs = {**d.__dict__, "name": truncated_name}
904+
common_name = cn_format.format(**format_kwargs)[:55]
901905
common_name = f"{common_name}-{unique_slug}"
902906
return common_name
903907

openwisp_controller/config/tests/test_vpn.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,22 @@ def test_vpn_client_get_common_name(self):
298298
d.name = d.mac_address
299299
self.assertIn(d.mac_address, client._get_common_name())
300300

301+
def test_get_common_name_does_not_mutate_device_name(self):
302+
"""Regression test: _get_common_name() must not mutate the
303+
device object's name in memory. A long device name should be
304+
truncated only for the common_name, not on the device itself."""
305+
long_name = "a" * 80
306+
vpn = self._create_vpn()
307+
d = self._create_device(name=long_name)
308+
c = self._create_config(device=d)
309+
client = VpnClient(vpn=vpn, config=c, auto_cert=True)
310+
client._get_common_name()
311+
self.assertEqual(
312+
d.name,
313+
long_name,
314+
"_get_common_name() must not truncate device.name in memory",
315+
)
316+
301317
def test_get_auto_context_keys(self):
302318
vpn = self._create_vpn()
303319
keys = vpn._get_auto_context_keys()
@@ -402,6 +418,25 @@ def test_vpn_and_cert_different_organization(self):
402418
else:
403419
self.fail("ValidationError not raised")
404420

421+
def test_auto_create_cert_preserves_full_device_name(self):
422+
"""Regression test: when a device has a very long name, the
423+
auto-created certificate's display name should be the original
424+
(full) device name, not the truncated common_name version."""
425+
long_name = "a" * 80 # exceeds 63 - len(mac_address) limit
426+
org = self._create_org(name="org1")
427+
vpn = self._create_vpn(organization=org)
428+
d = self._create_device(organization=org, name=long_name)
429+
c = self._create_config(device=d)
430+
template = self._create_template()
431+
client = VpnClient(vpn=vpn, config=c, auto_cert=True, template=template)
432+
client.full_clean()
433+
client.save()
434+
# The cert's name field should be the full original device name
435+
cert = Cert.objects.filter(organization=org, name=long_name)
436+
self.assertEqual(cert.count(), 1)
437+
# Device name must not be mutated
438+
self.assertEqual(d.name, long_name)
439+
405440
def test_auto_create_cert_with_long_device_name(self):
406441
device_name = "abcdifghijklmnopqrstuvwxyz12345678901234567890"
407442
org = self._create_org(name="org1")

0 commit comments

Comments
 (0)