Fixed Improper Method Call: __eq__ and __ne__

[fazledyn-or]

Details

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

In file: lnodes.py, class: LNode, there is a special method __eq__ that raises a NotImplementedError. If a special method supporting a binary operation is not implemented it should return NotImplemented. On the other hand, NotImplementedError should be raised from abstract methods inside user defined base classes to indicate that derived classes should override those methods. iCR suggested that the special method __eq__ should return NotImplemented instead of raising an exception. An example of how NotImplemented helps the interpreter support a binary operation is here.

Proof of Concept / Explanation

Let's consider the following test code -

class Node(object):
    def __init__(self) -> None:
        pass

a = Node()
b = Node()
c = a
print(f"a == a, a != a : {a == a, a != a}")
print(f"a == b, a != b : {a == b, a != b}")
print(f"a == c, a != c : {a == c, a != c}")

Output:

(env) ataf@openrefactory:~/BFAS/Source/ffcx$ python3 _test.py 
a == a, a != a : (True, False)
a == b, a != b : (False, True)
a == c, a != c : (True, False)

Here, we can see that even the most minimal implementation of a class can compare the objects.

However, if we try executing the following block of code -

class LNode(object):
    """Base class for all AST nodes."""
    def __eq__(self, other):
        name = self.__class__.__name__
        raise NotImplementedError("Missing implementation of __eq__ in " + name)

    def __ne__(self, other):
        return not self.__eq__(other)

a = LNode()
b = LNode()
c = a
print(f"a == a, a != a : {a == a, a != a}")
print(f"a == b, a != b : {a == b, a != b}")
print(f"a == c, a != c : {a == c, a != c}")

Output:

(env) ataf@openrefactory:~/BFAS/Source/ffcx$ python3 _test.py 
Traceback (most recent call last):
  File "_test.py", line 51, in <module>
    print(f"a == a, a != a : {a == a, a != a}")
  File "_test.py", line 40, in __eq__
    raise NotImplementedError("Missing implementation of __eq__ in " + name)
NotImplementedError: Missing implementation of __eq__ in LNode

The fallback mechanism of Python's object comparison gets overridden by the __eq__ method implementation. Let's try change the code as below and try to execute -

class LNode(object):
    """Base class for all AST nodes."""
    def __eq__(self, other):
        return NotImplemented

    def __ne__(self, other):
        return not self.__eq__(other)

a = LNode()
b = LNode()
c = a
print(f"a == a, a != a : {a == a, a != a}")
print(f"a == b, a != b : {a == b, a != b}")
print(f"a == c, a != c : {a == c, a != c}")

Output:

(env) ataf@openrefactory:~/BFAS/Source/ffcx$ python3 _test.py 
a == a, a != a : (True, False)
a == b, a != b : (False, False)
a == c, a != c : (True, False)

But, there's a problem now. The statement a == b, a != b is returning the value False, False which shouldn't have been the case. Since the __ne__ method depends on __eq__ method, we'll have to modify it too.

class LNode(object):
    """Base class for all AST nodes."""
    def __eq__(self, other):
        return NotImplemented

    def __ne__(self, other):
        return NotImplemented

a = LNode()
b = LNode()
c = a
print(f"a == a, a != a : {a == a, a != a}")
print(f"a == b, a != b : {a == b, a != b}")
print(f"a == c, a != c : {a == c, a != c}")

Output:

(env) ataf@openrefactory:~/BFAS/Source/ffcx$ python3 _test.py
a == a, a != a : (True, False)
a == b, a != b : (False, True)
a == c, a != c : (True, False)

After fixing both methods, our code works as expected.

Related Documentation

• docs.python.org - NotImplemented
• docs.python.org - NotImplementedError
• docs.python.org - Implementing Arithmetic Operators

Changes

• Modified both __eq__ and __ne__ methods

Previously Found & Fixed

• https://www.github.com/SciTools/iris/pull/5544
• https://www.github.com/cupy/cupy/pull/7900
• https://www.github.com/ethereum/web3.py/pull/3080

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.