Error cleanly if the public and private keys to an ECDH key exchange …

[alex]

…are on different curves

[reaperhulk]

This fails py3's pep8 with E721 do not compare types, use 'isinstance()'

[alex]

Yeah, I don't know what that means. What's teh right way to check if two curves rae equivilant.

[ronf]

I know this was already fixed another way, but for future reference you could do it as a type check by saying:

    if not isinstance(peer_public_key.curve, type(self.curve)):

[alex]

Yeah, I wonder if we should just have an __eq__ on curves.

On Thu, Oct 29, 2015 at 9:37 AM, Ron Frederick notifications@github.com
wrote:

In src/cryptography/hazmat/backends/openssl/ec.py
#2455 (comment):

@@ -182,6 +182,11 @@ def exchange(self, algorithm, peer_public_key):
_Reasons.UNSUPPORTED_EXCHANGE_ALGORITHM
)

•    if type(peer_public_key.curve) is not type(self.curve):
  

I know this was already fixed another way, but for future reference you
could do it as a type check by saying:

if not isinstance(peer_public_key.curve, type(self.curve)):

—
Reply to this email directly or view it on GitHub
https://github.com/pyca/cryptography/pull/2455/files#r43384597.

"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero
GPG Key fingerprint: 125F 5C67 DFE9 4084

[ronf]

Yeah, that would probably look cleaner. When writing an eq for classes that have other members to compare, part of it is usually involves an isinstance() check of this sort before comparing the members. In this case, if these classes have no other state, the isinstance() by itself could be enough.

That said, is there any reason these are classes instead of instances that could just be compared directly with "is"? Is it just for consistency with other parts of the API where arguments might be passed when constructing such an object?

One possibility here would be to make these singletons, so you still construct them from classes, but the constructor always returns the same instance.

[codecov-io]

Current coverage is 99.98%

Merging #2455 into master will not affect coverage as of 70b5003

@@            master   #2455   diff @@
======================================
  Files          122     122       
  Stmts        12720   12725     +5
  Branches      1399    1401     +2
  Methods          0       0       
======================================
+ Hit          12718   12723     +5
  Partial          2       2       
  Missed           0       0       

Review entire Coverage Diff as of 70b5003

Powered by Codecov. Updated on successful CI builds.

[coveralls]

Changes Unknown when pulling e245345 on alex:different-curves into ** on pyca:master**.