SNOW-926289: Why is urllib3 both vendored and marked as a dependency?

[pquentin]

What is the current behavior?

Installing snowflake-connector-python installs urllib3 1.26.

What is the desired behavior?

Since snowflake-connector-python vendors urllib3, I would expect to not also install it from pip.

How would this improve snowflake-connector-python?

It would remove one dependency, while helping the ecosystem move to urllib3 2.0.

References and other background

Disclaimer: I'm an urllib3 maintainer.

[sfc-gh-sfan]

@sfc-gh-mkeller : Are you aware of any reasons? It does feel like we could remove urllib3?

[sfc-gh-yixie]

I'm looking into this.

[EzraBC]

A thing to consider: https://nvd.nist.gov/vuln/detail/CVE-2023-43804 applies to the currently vendored version of urllib3 and snowflake-connector-python could be (is) blocked from use in some organizations.

[sfc-gh-mkeller]

So the reason why we have a vendored urllib3 and requests are because we monkey-patch OCSP into those.
However; this creates a chicken and the egg problem.
How do you do requests for OCSP? OCSP requests shouldn't care about OCSP checks.

Because of this we both depend on requests (and transiently urllib3) in their pure form to be used during OCSP checks and we also vendor them in a modified state for the connector to make its higher level calls through with OCSP checks.

Now where does this dependency come from? I've copied this from our vendored requests's dependencies.
On a second thought we shouldn't need this and we could just declare a less then next major pin on requests and let that deal with its own dependency.

[sfc-gh-mkeller]

A thing to consider: nvd.nist.gov/vuln/detail/CVE-2023-43804 applies to the currently vendored version of urllib3 and snowflake-connector-python could be (is) blocked from use in some organizations.

I want to be transparent that we have always despised the current way we deal with requests (and transiently urllib3) and we have been planning on reworking it for a long time. I'm actually really happy with the direction we are heading in currently and I want to make sure that you are aware that a fix of this is currently in the works, but I'm not comfortable enough to announce specifics just yet.

That being said I'm happy to bump our vendored libraries in the meantime to work around the CVE, we take these very seriously!

[timostrunk]

Now where does this dependency come from? I've copied this from our vendored requests's dependencies. On a second thought we shouldn't need this and we could just declare a less then next major pin on requests and let that deal with its own dependency.

Yes, please do this.

[ACronje]

+1, I have other dependencies that require urllib>2.0

[sfc-gh-mkeller]

For folks reading this after there was a bunch of conversation over on #1793.
Thank you everyone for their input on the matter! We now had our internal conversation, we'll be restricting the urllib3 pin to only affect Python version < 3.10 as explained in this comment similar to what botocore has done.

[mglickVA]

A thing to consider: nvd.nist.gov/vuln/detail/CVE-2023-43804 applies to the currently vendored version of urllib3 and snowflake-connector-python could be (is) blocked from use in some organizations.

I want to be transparent that we have always despised the current way we deal with requests (and transiently urllib3) and we have been planning on reworking it for a long time. I'm actually really happy with the direction we are heading in currently and I want to make sure that you are aware that a fix of this is currently in the works, but I'm not comfortable enough to announce specifics just yet.

That being said I'm happy to bump our vendored libraries in the meantime to work around the CVE, we take these very seriously!

Is there any update on this? It really would be ideal if you didn't have to vendor these libraries.