fix: copy include list before in-place mutation in get/query (#5857) - #7472
fix: copy include list before in-place mutation in get/query (#5857)#7472rkfshakti wants to merge 1 commit into
Conversation
…core#5857) The _validate_and_prepare_get_request and _validate_and_prepare_query_request methods assigned request_include = include without copying, then mutated it with .append('uris'). This corrupted the caller's list object, causing validation errors on subsequent calls that reused the same list. Fix: use list(include) to create a copy before any mutation.
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
|
Hi maintainers — gentle bump on this fix for #5857. Copies the |
|
Hi maintainers — circling back on this fix for #5857. Curious if the copy-before-mutation approach for the |
|
Friendly ping — this PR has been open for a couple of days with CI passing. Would appreciate a review when time allows. |
|
Hi maintainers — following up again on this fix for #5857. Copies the include list before in-place mutation in get/query to prevent side effects. CI is green. Would appreciate a review when time allows. Thanks! |
Problem
The
includeparameter passed tocollection.get()andcollection.query()is mutated in-place without copying, causing corruption when the same list object is reused across multiple calls.CollectionCommon.pylines 282 and 346:Impact
Reproduction
Fix
Copy the list before any in-place modification:
Applied to both
_validate_and_prepare_get_requestand_validate_and_prepare_query_request.Tests
Added two regression tests:
test_include_not_mutated_by_query— verifies the include list is not mutated across twoquery()callstest_include_not_mutated_by_get— verifies the include list is not mutated across twoget()callsFixes #5857