fix: Ensure str header values in connection.py #142
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* add reproducible test case * cover both sync and async
Ousret
approved these changes
Aug 15, 2024
There was a problem hiding this comment.
Do not hesitate to share the complete stack trace when an exception occurs.
The initial proposal made a specific test fail, so I had to make adjustments.
I added tests to ensure this never repeat again.
The reason this occurred is that since urllib3 v2, headers were typed to be string only (k,v). but current usage shows that bytes are still used as values.
Fortunately, the rest of the code already take that into account. The issue was specifically located in the "content-length" x "charset" transparency addition in urllib3-future.
moreover, it seems that no test available in botocore or boto3 cover that specific case, another reason why we missed it.
we'll push the fix so that no one else will suffer this blocker.
regards,
Ousret
added a commit
that referenced
this pull request
Aug 15, 2024
- Removed opinionated OpenSSL version constraint that forbid any version lower than 1.1.1. The reasoning behind this is that some companies expressed (to us) the need to upgrade urllib3 to urllib3-future in (very) old Python 3.7 built against patched OpenSSL 1.0.2 or 1.0.8 and collaborative testing showed us that this constraint is overly protective. Those build often lack TLS 1.3 support and may contain major vulnerabilities, but we have to be optimistic on their awareness. TLS 1.3 / QUIC is also an option for them as it works out of the box on those old distributions. Effective immediately, we added a dedicated pipeline in our CI to verify that urllib3-future works with the oldest Python 3.7 build we found out there. Blindly removing support for those libraries when supporting Python 3.7 ... 3.9 is as we "partially" support this range and end-users have no to little clues for why it's rejected when it clearly works. The only issue that can appear is for users that have Python built against a SSL library that does not support either TLS 1.2 or 1.3, they will encounter errors for sure. - Changed to submodule http2 to subpackage http2. Purely upstream sync. Still no use for us. - Changed minimum (C)Python interpreter version for qh3 automatic pickup to 3.7.11 as it bundle pip 21.2.4 and is the minimum version to pick an appropriate (abi3) pre-built wheel. You may still install ``qh3`` manually by first upgrading your pip installation by running ``python -m pip install -U pip``. - Fixed an issue where a server is yielding an invalid/malformed ``Alt-Svc`` header and urllib3-future may crash upon it. - Fixed an issue where sending a ``str`` body using a ``bytes`` value for Content-Type would induce a crash. This was due to our unicode transparency policy. See #142
Merged
Ousret
added a commit
that referenced
this pull request
Aug 15, 2024
- Removed opinionated OpenSSL version constraint that forbid any version lower than 1.1.1. The reasoning behind this is that some companies expressed (to us) the need to upgrade urllib3 to urllib3-future in (very) old Python 3.7 built against patched OpenSSL 1.0.2 or 1.0.8 and collaborative testing showed us that this constraint is overly protective. Those build often lack TLS 1.3 support and may contain major vulnerabilities, but we have to be optimistic on their awareness. TLS 1.3 / QUIC is also an option for them as it works out of the box on those old distributions. Effective immediately, we added a dedicated pipeline in our CI to verify that urllib3-future works with the oldest Python 3.7 build we found out there. Blindly removing support for those libraries when supporting Python 3.7 ... 3.9 is as we "partially" support this range and end-users have no to little clues for why it's rejected when it clearly works. The only issue that can appear is for users that have Python built against a SSL library that does not support either TLS 1.2 or 1.3, they will encounter errors for sure. - Changed to submodule http2 to subpackage http2. Purely upstream sync. Still no use for us. - Changed minimum (C)Python interpreter version for qh3 automatic pickup to 3.7.11 as it bundle pip 21.2.4 and is the minimum version to pick an appropriate (abi3) pre-built wheel. You may still install ``qh3`` manually by first upgrading your pip installation by running ``python -m pip install -U pip``. - Fixed an issue where a server is yielding an invalid/malformed ``Alt-Svc`` header and urllib3-future may crash upon it. - Fixed an issue where sending a ``str`` body using a ``bytes`` value for Content-Type would induce a crash. This was due to our unicode transparency policy. See #142
Ousret
added a commit
that referenced
this pull request
Aug 15, 2024
- Removed opinionated OpenSSL version constraint that forbid any version lower than 1.1.1. The reasoning behind this is that some companies expressed (to us) the need to upgrade urllib3 to urllib3-future in (very) old Python 3.7 built against patched OpenSSL 1.0.2 or 1.0.8 and collaborative testing showed us that this constraint is overly protective. Those build often lack TLS 1.3 support and may contain major vulnerabilities, but we have to be optimistic on their awareness. TLS 1.3 / QUIC is also an option for them as it works out of the box on those old distributions. Effective immediately, we added a dedicated pipeline in our CI to verify that urllib3-future works with the oldest Python 3.7 build we found out there. Blindly removing support for those libraries when supporting Python 3.7 ... 3.9 is as we "partially" support this range and end-users have no to little clues for why it's rejected when it clearly works. The only issue that can appear is for users that have Python built against a SSL library that does not support either TLS 1.2 or 1.3, they will encounter errors for sure. - Changed to submodule http2 to subpackage http2. Purely upstream sync. Still no use for us. - Changed minimum (C)Python interpreter version for qh3 automatic pickup to 3.7.11 as it bundle pip 21.2.4 and is the minimum version to pick an appropriate (abi3) pre-built wheel. You may still install ``qh3`` manually by first upgrading your pip installation by running ``python -m pip install -U pip``. - Fixed an issue where a server is yielding an invalid/malformed ``Alt-Svc`` header and urllib3-future may crash upon it. - Fixed an issue where sending a ``str`` body using a ``bytes`` value for Content-Type would induce a crash. This was due to our unicode transparency policy. See #142
Ousret
added a commit
that referenced
this pull request
Aug 15, 2024
- Removed opinionated OpenSSL version constraint that forbid any version lower than 1.1.1. The reasoning behind this is that some companies expressed (to us) the need to upgrade urllib3 to urllib3-future in (very) old Python 3.7 built against patched OpenSSL 1.0.2 or 1.0.8 and collaborative testing showed us that this constraint is overly protective. Those build often lack TLS 1.3 support and may contain major vulnerabilities, but we have to be optimistic on their awareness. TLS 1.3 / QUIC is also an option for them as it works out of the box on those old distributions. Effective immediately, we added a dedicated pipeline in our CI to verify that urllib3-future works with the oldest Python 3.7 build we found out there. Blindly removing support for those libraries when supporting Python 3.7 ... 3.9 is as we "partially" support this range and end-users have no to little clues for why it's rejected when it clearly works. The only issue that can appear is for users that have Python built against a SSL library that does not support either TLS 1.2 or 1.3, they will encounter errors for sure. - Changed to submodule http2 to subpackage http2. Purely upstream sync. Still no use for us. - Changed minimum (C)Python interpreter version for qh3 automatic pickup to 3.7.11 as it bundle pip 21.2.4 and is the minimum version to pick an appropriate (abi3) pre-built wheel. You may still install ``qh3`` manually by first upgrading your pip installation by running ``python -m pip install -U pip``. - Fixed an issue where a server is yielding an invalid/malformed ``Alt-Svc`` header and urllib3-future may crash upon it. - Fixed an issue where sending a ``str`` body using a ``bytes`` value for Content-Type would induce a crash. This was due to our unicode transparency policy. See #142
Ousret
added a commit
that referenced
this pull request
Aug 15, 2024
- Removed opinionated OpenSSL version constraint that forbid any version lower than 1.1.1. The reasoning behind this is that some companies expressed (to us) the need to upgrade urllib3 to urllib3-future in (very) old Python 3.7 built against patched OpenSSL 1.0.2 or 1.0.8 and collaborative testing showed us that this constraint is overly protective. Those build often lack TLS 1.3 support and may contain major vulnerabilities, but we have to be optimistic on their awareness. TLS 1.3 / QUIC is also an option for them as it works out of the box on those old distributions. Effective immediately, we added a dedicated pipeline in our CI to verify that urllib3-future works with the oldest Python 3.7 build we found out there. Blindly removing support for those libraries when supporting Python 3.7 ... 3.9 is as we "partially" support this range and end-users have no to little clues for why it's rejected when it clearly works. The only issue that can appear is for users that have Python built against a SSL library that does not support either TLS 1.2 or 1.3, they will encounter errors for sure. - Changed to submodule http2 to subpackage http2. Purely upstream sync. Still no use for us. - Changed minimum (C)Python interpreter version for qh3 automatic pickup to 3.7.11 as it bundle pip 21.2.4 and is the minimum version to pick an appropriate (abi3) pre-built wheel. You may still install ``qh3`` manually by first upgrading your pip installation by running ``python -m pip install -U pip``. - Fixed an issue where a server is yielding an invalid/malformed ``Alt-Svc`` header and urllib3-future may crash upon it. - Fixed an issue where sending a ``str`` body using a ``bytes`` value for Content-Type would induce a crash. This was due to our unicode transparency policy. See #142
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims to fix a bug I encountered when using a library that depends on urllib3.future in the same venv as boto3. This is I believe the same issue raised in #133.
Please let me know if there's a more robust change we could make than just converting to string in-place.