Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Begin making examples Python 3 compatible #245

Merged
merged 14 commits into from
May 6, 2015
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions examples/certgen.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def createCertRequest(pkey, digest="sha256", **name):
Create a certificate request.

Arguments: pkey - The key to associate with the request
digest - Digestion method to use for signing, default is md5
digest - Digestion method to use for signing, default is sha256
**name - The name of the subject of the request, possible
arguments are:
C - Country name
Expand All @@ -45,14 +45,14 @@ def createCertRequest(pkey, digest="sha256", **name):
req = crypto.X509Req()
subj = req.get_subject()

for (key,value) in name.items():
for (key,value) in list(name.items()):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you convert them into a list? Please don’t. :)

Also let’s just write key, value instead of (key,value).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 473fe6a

setattr(subj, key, value)

req.set_pubkey(pkey)
req.sign(pkey, digest)
return req

def createCertificate(req, (issuerCert, issuerKey), serial, (notBefore, notAfter), digest="md5"):
def createCertificate(req, issuerCertKey, serial, validityPeriod, digest="sha256"):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to PEP 3113 tuple unpacking is deprecated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it’s actually even removed in Python 3 (which kind of sucks when doing Twisted)

"""
Generate a certificate given a certificate request.

Expand All @@ -64,9 +64,11 @@ def createCertificate(req, (issuerCert, issuerKey), serial, (notBefore, notAfter
starts being valid
notAfter - Timestamp (relative to now) when the certificate
stops being valid
digest - Digest method to use for signing, default is md5
digest - Digest method to use for signing, default is sha256
Returns: The signed certificate in an X509 object
"""
(issuerCert, issuerKey) = issuerCertKey
(notBefore, notAfter) = validityPeriod
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, no need for parens

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 473fe6a

cert = crypto.X509()
cert.set_serial_number(serial)
cert.gmtime_adj_notBefore(notBefore)
Expand Down
11 changes: 7 additions & 4 deletions examples/mk_simple_certs.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,18 @@
cakey = createKeyPair(TYPE_RSA, 2048)
careq = createCertRequest(cakey, CN='Certificate Authority')
cacert = createCertificate(careq, (careq, cakey), 0, (0, 60*60*24*365*5)) # five years

print('Creating Certificate Authority private key in "simple/CA.pkey"')
open('simple/CA.pkey', 'w').write(crypto.dump_privatekey(crypto.FILETYPE_PEM, cakey))
open('simple/CA.pkey', 'w').write(crypto.dump_privatekey(crypto.FILETYPE_PEM, cakey).decode('utf-8'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be converted into a context manager

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback @hynek. I thought contexts were for creating connections with certs that had already been created. Or am I missing something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

He meant a context manager:

with open(...) as f:
    f.write(...)

On Wed, Apr 29, 2015 at 10:25 PM, elitest notifications@github.com wrote:

In examples/mk_simple_certs.py
#245 (comment):

print('Creating Certificate Authority private key in "simple/CA.pkey"')
-open('simple/CA.pkey', 'w').write(crypto.dump_privatekey(crypto.FILETYPE_PEM, cakey))
+open('simple/CA.pkey', 'w').write(crypto.dump_privatekey(crypto.FILETYPE_PEM, cakey).decode('utf-8'))

Thanks for the feedback @hynek https://github.com/hynek. I thought
contexts were for creating connections with certs that had already been
created. Or am I missing something?


Reply to this email directly or view it on GitHub
https://github.com/pyca/pyopenssl/pull/245/files#r29399782.

"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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in aab9ddd

print('Creating Certificate Authority certificate in "simple/CA.cert"')
open('simple/CA.cert', 'w').write(crypto.dump_certificate(crypto.FILETYPE_PEM, cacert))
open('simple/CA.cert', 'w').write(crypto.dump_certificate(crypto.FILETYPE_PEM, cacert).decode('utf-8'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be converted into a context manager

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in aab9ddd


for (fname, cname) in [('client', 'Simple Client'), ('server', 'Simple Server')]:
pkey = createKeyPair(TYPE_RSA, 2048)
req = createCertRequest(pkey, CN=cname)
cert = createCertificate(req, (cacert, cakey), 1, (0, 60*60*24*365*5)) # five years

print('Creating Certificate %s private key in "simple/%s.pkey"' % (fname, fname))
open('simple/%s.pkey' % (fname,), 'w').write(crypto.dump_privatekey(crypto.FILETYPE_PEM, pkey))
open('simple/%s.pkey' % (fname,), 'w').write(crypto.dump_privatekey(crypto.FILETYPE_PEM, pkey).decode('utf-8'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be converted into a context manager

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in aab9ddd

print('Creating Certificate %s certificate in "simple/%s.cert"' % (fname, fname))
open('simple/%s.cert' % (fname,), 'w').write(crypto.dump_certificate(crypto.FILETYPE_PEM, cert))
open('simple/%s.cert' % (fname,), 'w').write(crypto.dump_certificate(crypto.FILETYPE_PEM, cert).decode('utf-8'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be converted into a context manager.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in aab9ddd

6 changes: 3 additions & 3 deletions examples/simple/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@

def verify_cb(conn, cert, errnum, depth, ok):
# This obviously has to be updated
print 'Got certificate: %s' % cert.get_subject()
print('Got certificate: %s' % cert.get_subject())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please make the % arguments a tuple since you’re touching the line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b2ff5be

return ok

if len(sys.argv) < 3:
print 'Usage: python[2] client.py HOST PORT'
print('Usage: python client.py HOST PORT')
sys.exit(1)

dir = os.path.dirname(sys.argv[0])
Expand All @@ -44,7 +44,7 @@ def verify_cb(conn, cert, errnum, depth, ok):
sys.stdout.write(sock.recv(1024))
sys.stdout.flush()
except SSL.Error:
print 'Connection died unexpectedly'
print('Connection died unexpectedly')
break


Expand Down
24 changes: 12 additions & 12 deletions examples/simple/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@

def verify_cb(conn, cert, errnum, depth, ok):
# This obviously has to be updated
print 'Got certificate: %s' % cert.get_subject()
print('Got certificate: %s' % cert.get_subject())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, tuple please

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b2ff5be

return ok

if len(sys.argv) < 2:
print 'Usage: python[2] server.py PORT'
print('Usage: python server.py PORT')
sys.exit(1)

dir = os.path.dirname(sys.argv[0])
Expand All @@ -44,27 +44,27 @@ def verify_cb(conn, cert, errnum, depth, ok):

def dropClient(cli, errors=None):
if errors:
print 'Client %s left unexpectedly:' % (clients[cli],)
print ' ', errors
print('Client %s left unexpectedly:' % (clients[cli],))
print(' ', errors)
else:
print 'Client %s left politely' % (clients[cli],)
print('Client %s left politely' % (clients[cli],))
del clients[cli]
if writers.has_key(cli):
if cli in writers:
del writers[cli]
if not errors:
cli.shutdown()
cli.close()

while 1:
try:
r,w,_ = select.select([server]+clients.keys(), writers.keys(), [])
r,w,_ = select.select([server]+list(clients.keys()), list(writers.keys()), [])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add missing spaces after , and around +

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lists removed(4852ef4) and then re-added(b2ff5be) as will not work under python3. Spacing fixed in those commits.

except:
break

for cli in r:
if cli == server:
cli,addr = server.accept()
print 'Connection from %s' % (addr,)
print('Connection from %s' % (addr,))
clients[cli] = addr

else:
Expand All @@ -74,10 +74,10 @@ def dropClient(cli, errors=None):
pass
except SSL.ZeroReturnError:
dropClient(cli)
except SSL.Error, errors:
except SSL.Error as errors:
dropClient(cli, errors)
else:
if not writers.has_key(cli):
if cli not in writers:
writers[cli] = ''
writers[cli] = writers[cli] + ret

Expand All @@ -88,13 +88,13 @@ def dropClient(cli, errors=None):
pass
except SSL.ZeroReturnError:
dropClient(cli)
except SSL.Error, errors:
except SSL.Error as errors:
dropClient(cli, errors)
else:
writers[cli] = writers[cli][ret:]
if writers[cli] == '':
del writers[cli]

for cli in clients.keys():
for cli in list(clients.keys()):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don’t convert it to a list please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 473fe6a

cli.close()
server.close()