[PATCH STABLE] sslutil: guard against broken certifi installations (issue5406)

Gregory Szorc gregory.szorc at gmail.com
Wed Oct 19 13:28:04 EDT 2016


On Wed, Oct 19, 2016 at 10:07 AM, Gábor STEFANIK <Gabor.STEFANIK at nng.com>
wrote:

> >
>
>
> --------------------------------------------------------------------------
> This message, including its attachments, is confidential. For more
> information please read NNG's email policy here:
> http://www.nng.com/emailpolicy/
> By responding to this email you accept the email policy.
>
>
> -----Original Message-----
> > From: Kevin Bullock [mailto:kbullock+mercurial at ringworld.org]
> > Sent: Wednesday, October 19, 2016 6:18 PM
> > To: Gábor STEFANIK <Gabor.STEFANIK at nng.com>
> > Cc: mercurial-devel at mercurial-scm.org
> > Subject: Re: [PATCH STABLE] sslutil: guard against broken certifi
> installations
> > (issue5406)
> >
> > > On Oct 19, 2016, at 11:07, Gábor Stefanik <gabor.stefanik at nng.com>
> > wrote:
> > >
> > > # HG changeset patch
> > > # User Gábor Stefanik <gabor.stefanik at nng.com> # Date 1476893174 -7200
> > > #      Wed Oct 19 18:06:14 2016 +0200
> > > # Branch stable
> > > # Node ID 77e20e2892a869717db636f56ab1b9664fc8b285
> > > # Parent  e478f11e418288b8308457303d3ddf6a23f874f8
> > > sslutil: guard against broken certifi installations (issue5406)
> > >
> > > Certifi is currently incompatible with py2exe; the Python code for
> > > certifi gets included in library.zip, but not the cacert.pem file -
> > > and even if it were included, SSLContext can't load a cacert.pem file
> from
> > library.zip.
> > > This currently makes it impossible to build a standalone Windows
> > > version of Mercurial.
> > >
> > > Guard against this, and possibly other situations where a module with
> > > the name "certifi" exists, but is not usable.
> > >
> > > diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
> > > --- a/mercurial/sslutil.py
> > > +++ b/mercurial/sslutil.py
> > > @@ -695,9 +695,10 @@
> > >     try:
> > >         import certifi
> > >         certs = certifi.where()
> > > -        ui.debug('using ca certificates from certifi\n')
> > > -        return certs
> > > -    except ImportError:
> > > +        if os.path.exists(certs):
> > > +            ui.debug('using ca certificates from certifi\n')
> > > +            return certs
> > > +    except:
> >
> > You've gone from catching an ImportError to swallowing all exceptions.
>
> Intentional. ImportError is not the only thing that can be thrown here;
> e.g. if "certifi" is actually some unrelated module with no "where()"
> method.
>
> No reason to let certifi crash Hg under any circumstances.
>
>
Bareword "except" is evil because it catches all exceptions, including
SystemExit and KeyboardInterrupt. That means that if the process is killed
or the user hits ^C because certifi import is being slow because I/O or
some other badness, Python ignores that signal.

"except:" should be banned by the code checker if it isn't already.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20161019/92271c26/attachment.html>


More information about the Mercurial-devel mailing list