Bug 197164 – webkitpy auto installer should download packages using `curl` instead of python's urllib.

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Dean Johnson]

I think webkitpy's auto installer should use `curl` to download packages instead of Python's urllib2 library. There are many reasons for this:
1. urllib2 can fail with obscure messages due to failures to TLS versions[1] or permissions on certs[2]. These messages could be communicated back to the user, but they generally do not have clear solutions to them.
2. Hashes for expected packages are already stored in Tools/Scripts/webkitpy/thirdparty/__init__.py.

So using `curl`, we could get around TLS version issues and cert permission issues using --no-verify, but also still guarantee we're getting the package we expect by checking the hashes of the downloaded packages against what they are known to be.

I'm not sure if this should be done for Windows as well, but would err on the side of caution and say to use the default behavior on Windows until someone was able to verify the same codepath worked.

[1] via https://bugs.webkit.org/show_bug.cgi?id=197046 - <urlopen error [SSL: TLSV1_ALERT_PROTOCOL_VERSION] tlsv1 alert protocol version (_ssl.c:590)>
[2] via https://bugs.webkit.org/show_bug.cgi?id=197046 - <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:727)>

[Jonathan Bedard]

(In reply to Dean Johnson from comment #0)
> I think webkitpy's auto installer should use `curl` to download packages
> instead of Python's urllib2 library. There are many reasons for this:
> 1. urllib2 can fail with obscure messages due to failures to TLS versions[1]
> or permissions on certs[2]. These messages could be communicated back to the
> user, but they generally do not have clear solutions to them.
> 2. Hashes for expected packages are already stored in
> Tools/Scripts/webkitpy/thirdparty/__init__.py.
> 
> So using `curl`, we could get around TLS version issues and cert permission
> issues using --no-verify, but also still guarantee we're getting the package
> we expect by checking the hashes of the downloaded packages against what
> they are known to be.
> 
> I'm not sure if this should be done for Windows as well, but would err on
> the side of caution and say to use the default behavior on Windows until
> someone was able to verify the same codepath worked.
> 
> [1] via https://bugs.webkit.org/show_bug.cgi?id=197046 - <urlopen error
> [SSL: TLSV1_ALERT_PROTOCOL_VERSION] tlsv1 alert protocol version
> (_ssl.c:590)>
> [2] via https://bugs.webkit.org/show_bug.cgi?id=197046 - <urlopen error
> [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:727)>

I don't know that we'd get the SSL error with curl anyways, since curl definitely relies on the system certs, where Python's libraries are far less consistent in that regard.

[Dean Johnson]

It looks like I was mis-remembering. webkitpy/common/system/autoinstall.py does not appear to have any notion of checking for the hash of the tarball that was downloaded.

So we can't use --no-verify to curl, but according to local testing it looks like even TLS version issues seen using `urllib2` are not a problem using `curl`.

[Dean Johnson]

Submitted comment too early.

...are not a problem using `curl` *even when --no-verify is omitted*.

[Dean Johnson]

Created attachment 367967 [details]
Patch

[Darin Adler]

Comment on attachment 367967 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=367967&action=review

> Tools/ChangeLog:11
> +        * Scripts/webkitpy/common/system/autoinstall.py:
> +        (AutoInstaller._download): Use curl to download packages necessary for webkitpy/thirdparty/autoinstalled
> +        since Python2's urllib2 module sometimes spews useless error messages about TLS versions or certificate access
> +        even if invoking a command to do the same thing with `curl` would work without issue.

No elegant way to quiet the messages?

[Dean Johnson]

(In reply to Darin Adler from comment #5)
> Comment on attachment 367967 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=367967&action=review
> 
> > Tools/ChangeLog:11
> > +        * Scripts/webkitpy/common/system/autoinstall.py:
> > +        (AutoInstaller._download): Use curl to download packages necessary for webkitpy/thirdparty/autoinstalled
> > +        since Python2's urllib2 module sometimes spews useless error messages about TLS versions or certificate access
> > +        even if invoking a command to do the same thing with `curl` would work without issue.
> 
> No elegant way to quiet the messages?

Thanks for the review.

I'll update the ChangeLog to make it more clear, but the issue being addressed is not the spewing of error messages, but rather that using Python2's urllib2 module for this download results in a variety of errors that don't give a clear indication of how they can be fixed. They also are errors that only seem to be shown by urllib2, since curl(1) does the package download without any issues.

This bug was primarily initiated based on the detailed bug report by yevseytsev in https://bugs.webkit.org/show_bug.cgi?id=197046, where the only way he was able to run our tests (after deleting the previous local admin user on macOS *2 years* ago) was to do a complete re-installation of his system.

[Dean Johnson]

Created attachment 367973 [details]
Patch

[Dean Johnson]

Created attachment 367974 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 367974 [details]
Patch

Clearing flags on attachment: 367974

Committed r244513: <https://trac.webkit.org/changeset/244513>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/50109280>