cocagne / pysrp

Python implementation of the Secure Remote Password protocol (SRP)
MIT License
113 stars 42 forks source link

Python 3 Compatible Version #16

Closed masihyeganeh closed 7 years ago

masihyeganeh commented 8 years ago

Hello. I did some changes to C code. It's now compatible with both python 2 and 3. Please review and merge it and update pip version.

Thanks

cocagne commented 8 years ago

So I tried running this on an Arch linux system with Python 3.5.1 and it compiles okay but the _srp module refuses to load when I attempt to run the tests. First complaining about an OpenSSL symbol error (the -lssl flag apparently didn't work as ldd reports the library as not being linked to libssl.so) Even when I force libssl.so to be loaded though, Python complains that the PyInit__srp function is not exported.

All I did was check out your changes and run "python setup.py build; python srp/test_srp.py". Also, I had to convert the use of the "thread" module in test_srp.py to use "threading" instead. I must be missing something here. How do I compile and run your changes with Python 3?

lsmag commented 8 years ago

I didn't read the C code, but I believe there's a simple way to deal with it: distutils.extension.Extension has an optional argument, which ignores a failed extension build if optional=True. So, this works (alongside with the print updates, of course):

ext_modules = [ Extension('srp._srp', ['srp/_srp.c',], libraries = ['ssl',], optional=True) ]

Also, minor nitpick: I've read this patch's updates to print in test_srp.py and I don't think they're quite right. You should be using from __future__ import print_function, as print has a different behavior in Py2 or Py3. For example print ' |', does not translate to print(' |',), but rather print(' |', end=' ').

lsmag commented 8 years ago

Another update: I commited some different things on my fork, but the test doesn't work yet, it still has some specific Python2 code (like import thread). Another issue I found is that the test actually requires the C extension to be present. I presume the srp._srp tests should be skipped if the module is missing, what do you guys think?

cocagne commented 7 years ago

completed by another pull request