Fixed Inappropriate Logical Expression

[nydelzaf]

What do these changes do?

• Removed a break statement inside the 'finally' block of a try-catch statement. Using 'break' or 'continue' inside a finally block can suppress the propagation of unhandled exception.

Checklist

• [x] I think the code is well written
• [ ] Unit tests for the changes exist
• [ ] Documentation reflects the changes
• [x] If you provide code modification, please add yourself to CONTRIBUTORS.txt
  • The format is <Name> <Surname>.
  • Please keep alphabetical order, the file is sorted by names.
• [x] Add a new news fragment into the CHANGES folder
  • name it <issue_id>.<type> for example (588.bugfix)
  • if you don't have an issue_id change it to the pr id after creating the pr
  • ensure type is one of the following:
    • .feature: Signifying a new feature.
    • .bugfix: Signifying a bug fix.
    • .doc: Signifying a documentation improvement.
    • .removal: Signifying a deprecation or removal of public API.
    • .misc: A ticket has been closed, but it is not of interest to users.
  • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

Details

While triaging your project, our bug fixing tool generated the following message(s)-

In file: web_protocol.py, method: start, a break statement is used inside a finally block. This will discard the temporarily saved unhandled exception which was raised inside the try, else or except block. iCR suggested that the break statement should not be used inside the finally block.

How It Works

Incorrect Use You can have a look at the example below-

foo = [1, 2, 3, 4, 5]
bar = None
for _ in range(5):
    try:
        # IndexError
        bar = foo[10]
        print("Inside try block")
    except:
        print("Inside except block")
        # raising another exception
        bar = foo[100]
    else:
        print("Inside else block")
        # let's try another exc
        bar = foo[100]
    finally:
        break

Inside except block

Correct Use

foo = [1, 2, 3, 4, 5]
bar = None
for _ in range(5):
    try:
        # IndexError
        bar = foo[10]
        print("Inside try block")
    except:
        print("Inside except block")
        # raising another exception
        bar = foo[100]
    else:
        print("Inside else block")
        # let's try another exc
        bar = foo[100]
    finally:
        pass

Inside except block
Traceback (most recent call last):
  File "try-catch-break.py", line 13, in <module>
    bar = foo[10]
IndexError: list index out of range

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "try-catch-break.py", line 18, in <module>
    bar = foo[100]
IndexError: list index out of range

From these two examples, we can see that if break is used in the finally block, it ignores any exceptions that takes place in except or else. But if pass is used, or finally block isn't used, the exceptions are reported correctly. As a result, the correct use is not to use break or continue inside the finally block.

I've removed the else and break statement from the code since it'll lead to a better understanding of any exceptions that may take place during code execution.

CLA Requirements

This section is only relevant if your project requires contributors to sign a Contributor License Agreement (CLA) for external contributions.

All contributed commits are already automatically signed off.

The meaning of a signoff depends on the project, but it typically certifies that committer has the rights to submit this work under the same license and agrees to a Developer Certificate of Origin (see https://developercertificate.org/ for more information). - Git Commit SignOff documentation

Sponsorship and Support

This work is done by the security researchers from OpenRefactory and is supported by the Open Source Security Foundation (OpenSSF): Project Alpha-Omega. Alpha-Omega is a project partnering with open source software project maintainers to systematically find new, as-yet-undiscovered vulnerabilities in open source code - and get them fixed – to improve global software supply chain security.

The bug is found by running the Intelligent Code Repair (iCR) tool by OpenRefactory and then manually triaging the results.

[webknjaz]

@fazledyn-or is this still on your radar? Unfortunately, we cannot merge this until it stops shuttering the CI...