Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[servicebus, eventhubs] Potential bug because of modifying the list during iteration #26765

Closed
bebound opened this issue Jun 27, 2023 · 6 comments
Assignees
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Event Hubs az eventhubs Service Attention This issue is responsible by Azure service team. Service Bus az servicebus

Comments

@bebound
Copy link
Contributor

bebound commented Jun 27, 2023

Describe the bug

In https://github.com/azure/azure-cli/blob/ae4a96ad73e6e6b4cd27b29ade5cbe65e62dadf0/src/azure-cli/azure/cli/command_modules/eventhubs/operations/network_rule_set.py#L64-L67

   for i in ip_rule:
        for j in ip_rule_list:
            if i['ip-address'] == j["ip_mask"]:
                ip_rule_list.remove(j)

ip_rule_list is not fully iterated if j is removed. For example:

a = [1,2,3,4]
for i in a:
    print(i)
    a.remove(i)
>>> 1
>>> 3

There are four parts contains similar logic.

Related PR: #26685, found by modified-iterating-list rule.

c:\users\kk\developer\azure-cli\src\azure-cli\azure\cli\command_modules\eventhubs\operations\network_rule_set.py:67:16: W4701: Iterated list 'ip_rule_list' is being modified inside for loop body, consider iterating through a copy of it instead. (modified-iterating-list)
c:\users\kk\developer\azure-cli\src\azure-cli\azure\cli\command_modules\eventhubs\operations\network_rule_set.py:126:16: W4701: Iterated list 'virtual_network_rule_list' is being modified inside for loop body, consider iterating through a copy of it instead. (modified-iterating-list)
c:\users\kk\developer\azure-cli\src\azure-cli\azure\cli\command_modules\servicebus\operations\network_rule_set.py:64:16: W4701: Iterated list 'ip_rule_list' is being modified inside for loop body, consider iterating through a copy of it instead. (modified-iterating-list)
c:\users\kk\developer\azure-cli\src\azure-cli\azure\cli\command_modules\servicebus\operations\network_rule_set.py:124:16: W4701: Iterated list 'virtual_network_rule_list' is being modified inside for loop body, consider iterating through a copy of it instead. (modified-iterating-list)

@bebound bebound added the bug This issue requires a change to an existing behavior in the product in order to be resolved. label Jun 27, 2023
@yonzhan
Copy link
Collaborator

yonzhan commented Jun 27, 2023

Thank you for opening this issue, we will look into it.

@bebound
Copy link
Contributor Author

bebound commented Jun 27, 2023

The code is introduced by #25792
@schaudhari6254888 Could you please take a look?

@yonzhan yonzhan added Service Bus az servicebus Event Hubs az eventhubs Service Attention This issue is responsible by Azure service team. labels Jun 27, 2023
@ghost
Copy link

ghost commented Jun 27, 2023

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @Saglodha.

Issue Details

Describe the bug

In https://github.com/azure/azure-cli/blob/ae4a96ad73e6e6b4cd27b29ade5cbe65e62dadf0/src/azure-cli/azure/cli/command_modules/eventhubs/operations/network_rule_set.py#L64-L67

   for i in ip_rule:
        for j in ip_rule_list:
            if i['ip-address'] == j["ip_mask"]:
                ip_rule_list.remove(j)

ip_rule_list is not fully iterated if j is removed. For example:

a = [1,2,3,4]
for i in a:
    print(i)
    a.remove(i)
>>> 1
>>> 3

There are four parts contains similar logic.

Related PR: #26685, found by modified-iterating-list rule.

c:\users\kk\developer\azure-cli\src\azure-cli\azure\cli\command_modules\eventhubs\operations\network_rule_set.py:67:16: W4701: Iterated list 'ip_rule_list' is being modified inside for loop body, consider iterating through a copy of it instead. (modified-iterating-list)
c:\users\kk\developer\azure-cli\src\azure-cli\azure\cli\command_modules\eventhubs\operations\network_rule_set.py:126:16: W4701: Iterated list 'virtual_network_rule_list' is being modified inside for loop body, consider iterating through a copy of it instead. (modified-iterating-list)
c:\users\kk\developer\azure-cli\src\azure-cli\azure\cli\command_modules\servicebus\operations\network_rule_set.py:64:16: W4701: Iterated list 'ip_rule_list' is being modified inside for loop body, consider iterating through a copy of it instead. (modified-iterating-list)
c:\users\kk\developer\azure-cli\src\azure-cli\azure\cli\command_modules\servicebus\operations\network_rule_set.py:124:16: W4701: Iterated list 'virtual_network_rule_list' is being modified inside for loop body, consider iterating through a copy of it instead. (modified-iterating-list)

Author: bebound
Assignees: -
Labels:

bug, Service Attention, Service Bus, Event Hubs

Milestone: -

@bebound bebound changed the title [servicebus, eventhubs] Potential bug because of modify the list during iteration [servicebus, eventhubs] Potential bug because of modifying the list during iteration Jun 27, 2023
@schaudhari6254888
Copy link
Contributor

The code is introduced by #25792 @schaudhari6254888 Could you please take a look?

Hey @bebound , Since IP address in Ip-rule is unique. So Suppose if it matches it will remove that ip address in ongoing iteration. even if skip the next index it doesn't harm bcs unique ip is already remove. So next iteration again it starts from 0th index of IP_RULE_list.

@bebound
Copy link
Contributor Author

bebound commented Jun 27, 2023

Thanks for your confirmation.
Would you mind if I use a copy when iteration, so I don't need to disable modified-iterating-list lint rule?
for j in ip_rule_list[:]:

@schaudhari6254888
Copy link
Contributor

schaudhari6254888 commented Jun 28, 2023

Thanks for your confirmation. Would you mind if I use a copy when iteration, so I don't need to disable modified-iterating-list lint rule? for j in ip_rule_list[:]:

yes, you can use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Event Hubs az eventhubs Service Attention This issue is responsible by Azure service team. Service Bus az servicebus
Projects
None yet
Development

No branches or pull requests

3 participants