search

sdgathman / pyspf

Other
49 stars 26 forks source link

Update spf.py #8

Closed jahlives closed 5 years ago

jahlives commented 5 years ago

only import ipaddress module if running python3 else ensure that ipaddr is imported. At least in python 2.7.5 on Centos7 you get ugly exceptions with ipaddress module and unicode. Fixes issue #7

import spf
spf.check2('XX.YY.ZZ.YY', 'user@example.com', 'example.com')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.7/site-packages/spf.py", line 297, in check2
    receiver=receiver,timeout=timeout,verbose=verbose,querytime=querytime).check()
  File "/usr/lib/python2.7/site-packages/spf.py", line 378, in __init__
    self.set_ip(i)
  File "/usr/lib/python2.7/site-packages/spf.py", line 405, in set_ip
    self.ipaddr = ipaddress.ip_address(i)
  File "/usr/lib/python2.7/site-packages/ipaddress.py", line 163, in ip_address
    ' a unicode object?' % address)
ipaddress.AddressValueError: 'XX.YY.ZZ.YY' does not appear to be an IPv4 or IPv6 address. Did you pass in a bytes (str in Python 2) instead of a unicode object?
sdgathman commented 5 years ago

ipaddress works when passing unicode. I'd rather fix the relatively few calls to ipaddress in pyspf than use the older and less supported library. Plus we want to keep moving toward python3.

kitterma commented 5 years ago

I think making explicit calls for Unicode will make Python 3 harder, not easier. Unicode is the default in Python 3. You have to explicitly specify Unicode for Python 2 and then it's implicit in Python3. I'm not aware if any issues using ipaddr (and it is still maintained upstream - they recently did a new release), so I think the approach of using ipaddress on Python3 and ipaddr on Python2 is the right one. I've seen nothing but trouble with ipaddress on Python 2.

My recommendation is to merge the PR as is. It solves the problem without adding complexity to the code. It changes code I wrote and matches my original intent (the Python 2 ipaddress backport didn't exist then).

sdgathman commented 5 years ago

Hmm. I had turned pyspf-2.0-branch into master on my local repo, but couldn't figure out how to push to github. So I'll have to do this on 2.0 also. I made a pyspf-2.1 branch with a copy of master. I hate getting interrupted...