Rename VerifiedHTTPSConnection

[kobayashi]

Fix: #1799
Renamed VerifiedHTTPSConnection to HTTPSConnection. VerifiedHTTPSConnection is backported.

[dopstar]

I think that on the lines 423-428:

if ssl:
    # Make a copy for testing.
    UnverifiedHTTPSConnection = HTTPSConnection
    HTTPSConnection = VerifiedHTTPSConnection
else:
    HTTPSConnection = DummyConnection

We should consider the following:

• The HTTPSConnection class should not be shadowed by reassignment so that it is exported as is (im not sure about the use case of the DummyConnection in the else statement)
• if we are keeping UnverifiedHTTPSConnection = HTTPSConnection then we should also have VerifiedHTTPSConnection = HTTPSConnection and then remove VerifiedHTTPSConnection class definition code.
  • Alternatively, a UnverifiedHTTPSConnection class might need to be defined just like how VerifiedHTTPSConnection is defined.

[kobayashi]

Thanks for your review! Overlooked these lines. Could we delete the if statement? I like the third option you mentioned, so defining UnverifiedHTTPSConnection like VerifiedHTTPSConnection is simpler IMO.
I am not sure the purpose of DummyConnection.
Let me know any thoughts.

[kobayashi]

DummyConnection seems to be used only for here.

urllib3/src/urllib3/connectionpool.py

Lines 962 to 965 in 75d2719

	if not self.ConnectionCls or self.ConnectionCls is DummyConnection:
	raise SSLError(
	"Can't connect to HTTPS URL because the SSL module is not available."
	)

[sethmlarson]

I'm thinking we do something like:

if ssl:
    HTTPSConnection = DummyConnection

VerifiedHTTPSConnection = HTTPSConnection

so we still get the nice error if the user is running Python without an ssl module and always get the name rebound to VerifiedHTTPSConnection. :)

Since the UnverifiedHTTPSConnection class was totally borked for the past ~6 months and we only received one issue on it I think we're good to remove it in the next release. 🎉 Just need to document in the changelog that there's only one HTTPSConnection object now. :)

[kobayashi]

Thanks @sethmlarson, Let me check the if statement. We assign DummyConnection to HTTPSConnection if not ssl, right? So the if statement is like this?

if not ssl:
    HTTPSConnection = DummyConnection

VerifiedHTTPSConnection = HTTPSConnection

[sethmlarson]

You're absolutely right haha, nice catch. :)

[kobayashi]

Thank you! modified the statement and added to CHANGES.rst

[sethmlarson]

Thanks for this! Almost there. :)

One thing we do need is a changelog entry for this change.

[sethmlarson]

I'm thinking we do something like:

if ssl:
    HTTPSConnection = DummyConnection

VerifiedHTTPSConnection = HTTPSConnection

so we still get the nice error if the user is running Python without an ssl module and always get the name rebound to VerifiedHTTPSConnection. :)

Since the UnverifiedHTTPSConnection class was totally borked for the past ~6 months and we only received one issue on it I think we're good to remove it in the next release. 🎉 Just need to document in the changelog that there's only one HTTPSConnection object now. :)

[pquentin]

Thanks! I think we're going to need a # NOQA comment to silence the F811 flake8 warnings: https://travis-ci.org/urllib3/urllib3/jobs/656104926

[sethmlarson]

I've made that change, thanks @pquentin for identifying. Will merge as soon as CI passes. 🎉

[sethmlarson]

Thanks so much @kobayashi for this change! 🚀