Skip to content

Fix shared class-level errors/objects lists in 4 DNS response classes - #2216

Open
Hisernberg wants to merge 2 commits into
apache:trunkfrom
Hisernberg:fix/dns-response-shared-lists
Open

Hisernberg wants to merge 2 commits into
apache:trunkfrom
Hisernberg:fix/dns-response-shared-lists

Conversation

@Hisernberg

@Hisernberg Hisernberg commented Nov 11, 2026 •

Copy link
Copy Markdown

Description

BuddyNSResponse, DNSPodResponse, LuadnsResponse and NsOneResponse declare errors/objects as class-level mutable attributes and append to them from parse_body_and_errors(). The lists are therefore shared by every response object in the process:

  • parsed objects accumulate across requests: libcloud/dns/drivers/nsone.py get_zone() reads response.objects[0] and can receive data from an earlier response;
  • errors leak between requests, so success() can report stale failures.

Reproduced locally against trunk:

from types import SimpleNamespace
from libcloud.common.buddyns import BuddyNSResponse

def raw(body):
    return SimpleNamespace(headers={}, body=body, text=body, status_code=200,
                           reason="OK", request=None, iter_content=lambda *a: iter([]))

r1 = BuddyNSResponse(response=raw('{"zones": [1]}'), connection=None)
r2 = BuddyNSResponse(response=raw('{"zones": [2]}'), connection=None)
print(r2.objects)   # [{'zones': [1]}, {'zones': [2]}]  <- stale first response inside r2

Fix

Initialize fresh per-instance errors/objects lists before super().__init__() (the base Response constructor calls success(), which reads errors), and convert the class attributes to bare annotations. Per-request behavior is otherwise unchanged.

Tests

Regression tests added to all four driver test suites (libcloud/test/dns/test_buddyns.py, test_dnspod.py, test_luadns.py, test_nsone.py):

  • test_objects_do_not_accumulate_across_responses
  • test_errors_do_not_leak_across_responses

With the old code the four suites report 4 failures + 1 error; with this change all 60 tests pass.

AI Generation Disclosure

Developed with AI assistance (agent workflow): the bug was found by an AST scan for mutated class-level attributes, reproduced against trunk, fixed, and verified with the A/B test run above.

BuddyNSResponse, DNSPodResponse, LuadnsResponse and NsOneResponse
declared errors/objects as class-level mutable attributes and appended
to them from parse_body_and_errors(). The lists were shared by every
response object in the process, so parsed objects accumulated across
requests: a get_zone() reading response.objects[0] could receive data
from an earlier response, and errors leaked between requests.

Initialize fresh per-instance lists before super().__init__() (the base
Response constructor calls success(), which reads errors), and convert
the class attributes to bare annotations.

Regression tests added to all four driver test suites: with the old
code they fail (4 failures, 1 error); with this change all 60 tests in
the four suites pass.

@micafer micafer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix.

There is some overlap with PR #2066 in NsOneResponse, but I don't consider that a blocker for this focused fix. We can reconcile the overlapping change when updating #2066 against trunk.

One minor, suggestion: the new test classes are currently placed after the if __name__ == "__main__": block. Moving them above that block would ensure they are also discovered when the test modules are executed directly.

Please also add the entry in the changelog.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.65%. Comparing base (d3bb996) to head (5dc1c7e).

Files with missing lines Patch % Lines
libcloud/test/dns/test_buddyns.py 93.75% 0 Missing and 1 partial ⚠️
libcloud/test/dns/test_dnspod.py 93.75% 0 Missing and 1 partial ⚠️
libcloud/test/dns/test_luadns.py 93.75% 0 Missing and 1 partial ⚠️
libcloud/test/dns/test_nsone.py 93.75% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk    #2216      +/-   ##
==========================================
+ Coverage   83.64%   83.65%   +0.01%     
==========================================
  Files         352      352              
  Lines       82321    82393      +72     
  Branches     8868     8880      +12     
==========================================
+ Hits        68855    68921      +66     
- Misses      10593    10594       +1     
- Partials     2873     2878       +5     
Files with missing lines Coverage Δ
libcloud/common/buddyns.py 100.00% <100.00%> (ø)
libcloud/common/dnspod.py 100.00% <100.00%> (ø)
libcloud/common/luadns.py 100.00% <100.00%> (ø)
libcloud/common/nsone.py 100.00% <100.00%> (ø)
libcloud/test/dns/test_buddyns.py 96.23% <93.75%> (-0.44%) ⬇️
libcloud/test/dns/test_dnspod.py 96.65% <93.75%> (-0.28%) ⬇️
libcloud/test/dns/test_luadns.py 96.00% <93.75%> (-0.20%) ⬇️
libcloud/test/dns/test_nsone.py 94.42% <93.75%> (-0.06%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants