Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove urllib3.disable_warnings #75

Closed
wants to merge 2 commits into from
Closed

Remove urllib3.disable_warnings #75

wants to merge 2 commits into from

Conversation

ninoseki
Copy link

@ninoseki ninoseki commented Apr 1, 2020

Remove urllib3.disable_warnings and use ca_certs provided by certifi because using an insecure connection is a bad idea in terms of security.

Mattwmaster58 and others added 2 commits March 23, 2020 15:41
Remove urllib3.disable_warnings() and use ca_certs provided by certifi
@Mattwmaster58
Copy link
Member

Mattwmaster58 commented Apr 1, 2020

I believe this is actually not required anymore because Python >= 3.7.4 has patched the root issue that was causing this problem. I'm not sure what versions of 3.6 are affected/fixed. I'll see if I can find a reference for that.

@Mattwmaster58 Mattwmaster58 added the enhancement New feature or request label Apr 1, 2020
@Mattwmaster58
Copy link
Member

Here's the reference: urllib3/urllib3#1682 (comment)
It mentions that 3.6 cannot be fixed, so this PR may have some merit there. @ninoseki Could you base your work off the pup2.1.1 branch? That way we won't have to mess around when it comes to merging pup2.1.1 to dev.

@ninoseki ninoseki changed the base branch from dev to pup2.1.1 April 1, 2020 23:23
@ninoseki
Copy link
Author

ninoseki commented Apr 2, 2020

Sorry for lack of my research and thank you for your comments.
I will make a PR to pup2.1.1 branch to use certifi in browser_fetcher.py.

@Mattwmaster58
Copy link
Member

Mattwmaster58 commented Apr 2, 2020

No problem! The only reason I had the knowledge is that I had to thoroughly investigate a very similar issue recently.

Thank you for your willingness to base your work on the pup2.1.1 branch.

Feel free to request a review from me when you're done.

@Mattwmaster58
Copy link
Member

Superceded by #76

@ninoseki ninoseki deleted the disable-urlib3-disable_warnings branch April 3, 2020 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants