Skip to main content

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

  • From: Yiteng Zhang < >
  • To:
  • Cc: Erik Trauschke < >
  • Subject: [pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e
  • Date: Wed, 30 Oct 2013 10:40:39 -0700

On 10/29/13 01:22 PM, Erik Trauschke wrote:
On 10/25/13 01:31 PM, Yiteng Zhang wrote:
Ok, a new webrev can be seen at
https://ips.java.net/webrev/yitezhan/15507548_4

Sorry that I dont add a comment to the new webrev.

Please let me know your comments.

Much better.
Looking at this it seems a cleaner approach would be to collect the exceptions in a simple list first and then only raise the exception if anything is in this list. Look at how InvalidOptionErrors is used, basically you can do the same thing here.

This way you don't need an additional variable to keep track if you actually had at least one exception and you only create the ExpiredCertificates exception when needed. That's a small change.


src/modules/client/api_errors.py:

2379++
relying on the fact that publishers are always in the correct sequence is sketchy. It's probably better to create a dictionary (publisher: [uri1, uri2, ...]) and then cycle through all the publishers in this list.

2382++
There is some work needed here to have these messages properly localized. The problem is that the length of the fields is not fixed in a way that we can format this message appropriately since the field names might have different lengths in different languages. The localization folks don't want to have any space formatting in the message strings for that reason so I suggest we do the following:

Publisher: pkg5-nightly
  Repository:
    https://pkg.oracle.com/pkg5-nightly
  Certificate:
    /var/pkg/ssl/cd98ea9966fb673235725ba169b2119e0d787fd5
  Key:
    /var/pkg/ssl/ab98ea9966fb673235725ba169b2119e0d787fd6

  Repository:
    https://my.local.repo/pkg5-nightly
  Certificate:
    /var/pkg/ssl/cd98ea9966fb673235725ba169b2119e0d787fd7
  Key:
    /var/pkg/ssl/ab98ea9966fb673235725ba169b2119e0d787fd8

You should be able to generate this like that:
msg = "%s\n" % publisher
msg += "  %s:\n" % _("Origin URI")
msg += "    %s\n" % uri
...

src/modules/client/image.py:
1783
    Please break the lines like this:

misc.validate_ssl_cert(
    uri.ssl_cert,
    prefix=p.prefix,
    uri=uri)

Erik




Ok, code has been modified and please take a look on it. Thanks.

https://ips.java.net/webrev/yitezhan/15507548_5/

Yiteng



[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Yiteng Zhang 10/21/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Erik Trauschke 10/21/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Yiteng Zhang 10/21/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Erik Trauschke 10/21/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Erik Trauschke 10/21/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Yiteng Zhang 10/23/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Xiaobo Shen 10/24/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Erik Trauschke 10/24/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Yiteng Zhang 10/25/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Erik Trauschke 10/29/2013

[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e

Yiteng Zhang 10/30/2013
 
 
Close
loading
Please Confirm
Close