> I think you need more safety checks in build_cert().  I'd put the file()
> and read() in a try block rather than doing the exists check, which should
> check for more problems than simple non-existence.  Also, What happens if
> the file is readable, but not a proper PEM file?  Does load_certificate()
> raise any exceptions if certdata is garbage, or does it just return None
> silently?

Fair enough.  The garbage cert case was something that I forgot to test.
I'll add some additional checks here.

> Line 1529: why is this here?

Removed.  Not sure why it was there.  Thanks.

> Line 1651: too computery a message.  Perhaps say that the certificate isn't
> valid/effective until nbdt (or perhaps whatever the local time equivalent is)?

I changed this to "Certificate effective date is in the future"

> I'm also a bit concerned that we're piling this directly into client.py,
> rather than extracting it out into separate code that the gui could reuse.
> As it is, this will just lead to more code copying.  Fixing that would
> require more insight into how the cert-handling code will evolve than I
> have, so I'm not sure how to guide you on that.

We discussed this offline, but for everyone else's edification, it does
seem like image.py might be a better home.  I had also initially considered
misc.py, but Danek observed that the cert_validity() routine operated on
an image.  This makes image.py a better candidate for the code.

I'll send out a new webrev once I finish the re-organization and error
checking.

Thanks for looking at this,

-j

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to