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

[Gábor STEFANIK]

>


--------------------------------------------------------------------------
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 7:46 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 12:07, Gábor STEFANIK <Gabor.STEFANIK at nng.com>
> wrote:
> >
> > -----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)
> >>
> >> 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.
>
> I have a hard time imagining how another module named "certifi" without a
> where() method would show up on any sane system.
>
> As Greg said, bare `except:` is banned in Mercurial. Catch the exceptions you
> expect might happen, none others.

Would "except Exception:" be acceptable? that one doesn't catch KeyboardInterrupt and other problematic exceptions.

>
> pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
> Kevin R. Bullock